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

Update shinatra.sh #16

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Update shinatra.sh #16

wants to merge 2 commits into from

Conversation

jojenki
Copy link

@jojenki jojenki commented Apr 5, 2018

There's no value in keeping it open other than causing some browsers and commands to hang. This will force-close the connection after the response is sent.

There's no value in keeping it open other than causing some browsers and commands to hang. This will force-close the connection after the response is sent.
@benrady
Copy link
Owner

benrady commented Apr 5, 2018

I think you're right, but what is the purpose of adding the -N option? That's not available in BSD netcat.

@jojenki
Copy link
Author

jojenki commented Apr 6, 2018

-N tells netcat to close the stream after it sends the message. On Debian, netcat would hold the connection open, even without the "Connection: keep-alive" header and even with a "Connection: close" header. (The latter is just a heads-up to the client and not something netcat should be expected to handle anyway.)

I see the "-N" in OpenBSD. Which distro are you not seeing it in?

http://man.openbsd.org/nc

I guess this is probably the friendliest way to do this.
@benrady
Copy link
Owner

benrady commented Apr 6, 2018

I'm looking at version 1.105-7 on Ubuntu 16.04.3 from the netcat-openbsd package. Also didn't see it on my Mac's netcat version but I don't remember where that came from.

@benrady
Copy link
Owner

benrady commented Apr 7, 2018

Thanks for the PR. I really have no specific memory of why that header is there, so I want to make sure I understand what's going on before changing anything.

Re-reading the RFC, I don't think it's possible to handle all the requirements without adding a lot of logic to the script (which I'm definitely not willing to do...it takes the punchline out of the joke).

So my first question is, if not all versions of netcat support the -N option, wouldn't you want Connection:keep-alive to give a hint to the clients that they need to close the connection because the server won't? Persistent connections are the default in HTTP 1.1.

Secondly, are there any specific clients/browsers where you've seen requests hang? What Connection headers do they send?

Thanks again!

@bendem
Copy link

bendem commented Aug 20, 2020

wouldn't you want Connection:keep-alive to give a hint to the clients that they need to close the connection because the server won't?

Actually, adding keep-alive tells the client to keep the connection open so they can re-use it, if you want the browser to close the connection because you aren't going to do it yourself, you need to pass close, then -N is not required.

@benrady
Copy link
Owner

benrady commented Aug 20, 2020 via email

@bendem
Copy link

bendem commented Aug 20, 2020

Have you tried that yet? When I tried making that change it didn't seem to make a difference with Chrome or Firefox.

I have not, but I have read the latest RFC, first point of persistent connection, if anybody sets connection to close, the exchange ends with the connection closed: https://tools.ietf.org/html/rfc7230#section-6.3

I get weird behaviour with chrome/firefox and edge, but it's mostly due to the trailing double line return after the response body (edge hangs without it, firefox and chrome hang when it's present).

edthrn added a commit to edthrn/shinatra that referenced this pull request Nov 19, 2020
edthrn added a commit to edthrn/shinatra that referenced this pull request Nov 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants