upload limit and hot reload of config

[tessus]

There are 2 things I came across that seemed a bit strange to me.

1. The content file size limit is only evaluated AFTER a file has been uploaded.
2. Hot reload of the configuration does not seem to work.

I set the value to 50MB, because sometimes log files even in compressed form can be quite large.
It happend that I had a 710MB compressed log file and forgot that I had the limit in place.
It uploaded the entire 710MB only to tell me that it exceeded the content length limit.
This is just an extreme case and probably won't happen that often, but I think the limit should be evaluated before any transfer happens.
Afaik the content length is part of the header for POST requests so it should be fairly easy to test for length compliance.

As for 2, I am not really using the feature, but I added a config value and changed another and neither was picked up after the "hot reload" refresh time lapsed. I looked at the code, but I didn't fully understand how the original config struct was actually changed/replaced. It only changes a cloned version unless I missed something. I would have to do more tests, but maybe somebody is using the feature and has some additional input.

[tessus]

I did more testing and the middleware reports the Upload rejected due to exceeded limit. warning in the log file right away, but the file is still uploaded by the client (curl, rpaste).
Only after the file has been uploaded, the error message appears on the client side.

Thus something is not ok with the middleware. Unfortunately I don't understand this part of the code.

[tessus]

I can look into this. First I want to get the auth_tokens PR done.

[orhun]

Alright!

[tessus]

There are fixtures that test the content limiter. Strangely enough, when I run these fixtures locally on my macOS, they do not fail. I even tried to add a huge file to that particular test, but curl doesn't hang. (Thinking out loud: Is is because client and server are both on macOS - still, this doesn't make sense.)

My test was macOS client with server on Linux (w/ and w/o reverse proxy, w/ and w/o auth). I am going to test with Linux boxes and see, if I can see the same issue.

[orhun]

I see, I would still say let's add another fixture to testing huge files on GitHub Actions.

[tessus]

The problem is how to detect whether the error message shows up before the file is uploaded or after? And how can I make sure that the file was not in fact uploaded at all.

I have to think about that...

[orhun]

The content file size limit is only evaluated AFTER a file has been uploaded.

This was supposed to be fixed in #53

Hot reload of the configuration does not seem to work.

For some of the values such as upload_path, timeout and max_content_length cannot be hot-reloaded (AKA configured on the fly) because they are supposed to be configured at the server start.

As for 2, I am not really using the feature, but I added a config value and changed another and neither was picked up after the "hot reload" refresh time lapsed. I looked at the code, but I didn't fully understand how the original config struct was actually changed/replaced. It only changes a cloned version unless I missed something. I would have to do more tests, but maybe somebody is using the feature and has some additional input.

Can you set RUST_LOG environment variable to trace and see if there are any logs about the hot-reloader? (e.g. "watcher dropped" etc.)

[tessus]

right, and the server config also holds the auth_tokens. There are 3 options in server_config that would make sense to hot reload:

• expose_version
• auth_tokens
• max_content_length

Everthing else should not be changed. Need to dive into the code. Maybe I can make that happen somehow.

[tessus]

Since server_config is cloned, it is not being updated by the hotwatcher thus the hot-reloaded values do not take effect.

I was looking into the code, but it still makes no sense to me. Yes, server_config is cloned, but the hotwatcher uses these 2 vars:

let config = Data::new(RwLock::new(config)); 
let cloned_config = Data::clone(&config); 

And they reference the entire config struct. So I am not sure, why auth_tokens or expose_version are not hot reloaded.

[tessus]

Actually they are hot reloaded. I am not sure why that didn't happen in my previous tests. The only difference was the refresh interval. Previously I had set it to 1h because I don't think it is necessary to check/reload every second. But even after 3 hours waiting, the changes were not picked up. I will run some more tests with an interval of 1h.

But now I was testing with 1s and changes to auth_tokens and expose_version were picked up and hot reloaded.

[tessus]

I did some more tests with an interval of 1h. But it seems that the config parameters I had in mind are in fact hot reloaded. I don't know why it didn't work when I started this discussion.
Maybe the quantum signature was slightly distored due to quarks being unhappy. It's a mystery to me.

I guess this part of the discussion is resolved.

[orhun]

Sounds like everything is working as expected! (fingers crossed)