Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SSL enabled server hangs on request #201

Open
ForgottenBeast opened this issue Jan 2, 2019 · 18 comments
Open

SSL enabled server hangs on request #201

ForgottenBeast opened this issue Jan 2, 2019 · 18 comments
Labels

Comments

@ForgottenBeast
Copy link

ForgottenBeast commented Jan 2, 2019

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

@tomaka tomaka added the bug label Jan 2, 2019
@jpastuszek
Copy link

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
Copy link
Contributor

@ltearno
Copy link

ltearno commented Jan 8, 2020

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

@oherrala
Copy link
Contributor

This has been fixed in tiny_http (tiny-http/tiny-http#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
Copy link

KyGost commented May 1, 2021

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

@bradfier
Copy link
Collaborator

bradfier commented May 3, 2021

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

@bradfier bradfier closed this as completed May 3, 2021
@KyGost
Copy link

KyGost commented May 5, 2021

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
Copy link

KyGost commented May 7, 2021

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
Copy link

KyGost commented May 7, 2021

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:

  • Vivaldi: Same experience as Firefox: not working, restart process, try again, working and continually so.
  • Chromium: Seemed to work straight away.
  • Opera: Worked.
  • Waterfox: Worked.
  • Brave: Same as Firefox and Vivaldi.
  • Chrome and Firefox on another machine: Worked.
  • Restarted my phone: Worked.

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. #231
c. #231 with tinyhttp set to latest
)

@bradfier bradfier reopened this May 7, 2021
@bradfier
Copy link
Collaborator

bradfier commented May 7, 2021

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
Copy link
Collaborator

bradfier commented May 7, 2021

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
Copy link

KyGost commented May 7, 2021

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
Copy link
Collaborator

bradfier commented May 7, 2021

I'm starting to suspect the changes in tiny-http/tiny-http#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
Copy link

KyGost commented May 7, 2021

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
Copy link

KyGost commented May 7, 2021

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
Copy link
Collaborator

bradfier commented May 7, 2021

It turns out it was unfair to blame tiny-http/tiny-http#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
Copy link

KyGost commented May 8, 2021

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
Copy link

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 ...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants