search

tomaka / rouille

Web framework in Rust
Apache License 2.0
1.09k stars 105 forks source link

SSL enabled server hangs on request #201

Open ForgottenBeast opened 5 years ago

ForgottenBeast commented 5 years ago

I seem to be having the exact same issue as described at the end of #90: the server starts OK with a valid cert and key but hangs after receiving a request. The provided gist (https://gist.github.com/CamJN/4369785b198a6dd513f75c8ad24023bd) hangs too.

rustc 1.31.1 on debian rouille version: 3.0.0

jpastuszek commented 5 years ago

Same here, any updates on this? Curl show the cert is OK, my server returns a response but curl never gets the response and keeps waiting.

oherrala commented 5 years ago

There's open issue and PR in tiny_http:

ltearno commented 4 years ago

Same for me. Will check at tiny_http level maybe...

oherrala commented 4 years ago

This has been fixed in tiny_http (https://github.com/tiny-http/tiny-http/pull/151) and released as version 0.7.0: CHANGELOG.

PR #231 updates rouille's tiny_http version to 0.7.0. Merging that and releasing new rouille should fix this issue.

KyGost commented 3 years ago

This was killing me for a while! Seems to be fixed (for me) in latest rouile release v3.1.0.

bradfier commented 3 years ago

Excellent news, I'll close the issue and if it pops up again feel free to reopen.

KyGost commented 3 years ago

I take it back! It seems to have problems when first trying to connect to a site (on Chrome) and then it is fine and acts as normal.

Will try to give more info in a bit.

KyGost commented 3 years ago

Hmm not even #231 seems to work.

Apologies, I don't know any more advanced techniques to say exactly what is going wrong, it seems to just be hangs when returning the response.

If I println for the handler, it can print the request, the browser, however, does not recieve a response; it just hangs.

What makes this especially weird is that my main browser seems to have accepted the site (at first it was having issues too) and now it works entirely fine every time.

KyGost commented 3 years ago

Hmmm!!! I'm in a bit of a rush right now so don't have a chance to investigate fully, when I have a chance I will try to do so (also, apologies for the multi-comment!).

Quick findings are: It seems to maybe sometimes only allow the first connection of a process in? It wasn't letting my firefox session in, I restarted the process, it let it in, now it works fine (even after restarting the process). However trying to do the same for my android chrome didn't work.

I'm a bit lost! I tried another few browsers:

I can't see any real patterns here! It seems to be a weird mess, perhaps it has to do with:

Responding to requests out of order with HTTPS won't work (this could conceivably be fixed with some check-is-ready-to-read to supply more requests if they're available)

But I can't seem to find a pattern nor solution.

Once again, will have a better look when I have a bit more time. If anyone else could also investigate that would be much appreciated. ( Or perhaps just see if experience is different to mine using: a. Latest Rouille b. https://github.com/tomaka/rouille/pull/231 c. https://github.com/tomaka/rouille/pull/231 with tinyhttp set to latest )

bradfier commented 3 years ago

Thanks for continuing to investigate this, based on what you've added so far, I suspect request ordering is at fault, it could be that depending on the browser, it might open one or more connections, and depending on which of those succeeds the page either appears to work or does not.

I'll try the SSL example from the repo on the off chance that reproduces the issue, if not, if you could drop a minimal example here that would be great.

bradfier commented 3 years ago

Unfortunately I'm not able to reproduce this when I wrap examples/hello-world.rs in a server built with new_ssl(), so a snippet that exhibits the issue would be really helpful.

I tested both the Threaded and ThreadPool executors with the same result, which one are you using?

I think your assessment and link to the HTTPS fix in tiny-http is likely to be related, a cursory glance at the change leads me to believe it's a bit of a work around that the browsers may or may not tolerate.

KyGost commented 3 years ago

I'll try to do some more testing, the biggest annoyance to this is that it is unreliable to reproduce and seems to not really be reproducable for a given machine:browser combination once they are working. I'll try to figure some more out.

Not entirely sure how one might change between the executors, I'm using whatever the default is with a loop which .poll()s multiple servers the way I am doing it might be contributing to the issue &| weirdness and if it was reliable to test I would be comparing with a normal .run() now.

bradfier commented 3 years ago

~I'm starting to suspect the changes in https://github.com/tiny-http/tiny-http/pull/151 are related.~

The PR references fixing a deadlock, but at least at first glance it doesn't look like it should have been dead locking there in the first place. The added synchronisation would have the side effect of changing the timings which could mask a deadlock from elsewhere.

I'll keep investigating too.

KyGost commented 3 years ago

Maybe it is working now (?!) I can't reproduce again.

I'll try getting rid of the changes I've added beyond current rouille and report back.

KyGost commented 3 years ago

I've gotten rid of all of the changes I've added and cannot reproduce. Very weird.

Would appreciate others who have had issues with SSL in this thread giving rouille latest a test.

For now, until I find issues, I'll leave it be.

bradfier commented 3 years ago

It turns out it was unfair to blame https://github.com/tiny-http/tiny-http/pull/151! The PR description is inaccurate, it refers to the TLS Socket accept call being that which holds a lock preventing the request from being responded to.

In fact it's the reader side of the accepted SslStream which blocks the writer side, meaning that if the message receive loop tries to read() the next message before the thread handling the previous one has finished, the Writer is blocked from sending the response.

This code is most likely to get exercised by a client that does HTTP request pipelining, maybe that's something you can try playing with while you try and reproduce?

KyGost commented 3 years ago

This code is most likely to get exercised by a client that does HTTP request pipelining, maybe that's something you can try playing with while you try and reproduce?

Honestly I am far from experienced enough in the HTTP space to even understand what "HTTP request pipelining" is, if/when I get some time I'll make an effort to research it, test it, and report back. Until then it seems to be working fine. If someone else who was having issues in this thread in past reports back saying latest is working I wouldn't be opposed to you closing this issue again.

@ltearno @jpastuszek @ForgottenBeast @CamJN

travispaul commented 2 years ago

Not sure if its related to this exact issue but, I was seeing the an ssl enabled server hang for curl, but not for firefox, which oddly enough resolved itself when I forced curl to use IPV4 (curl -4 ...)