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

Upgrade header which advertises protocol support being treated as an actual upgrading #422

Closed
unarist opened this issue Jul 23, 2017 · 8 comments
Labels

Comments

@unarist
Copy link

unarist commented Jul 23, 2017

from RFC7230 sec. 6.7:

A server MAY send an Upgrade header field in any other response to advertise that it implements support for upgrading to the listed protocols, in order of descending preference, when appropriate for a future request.

However, current http.rb treats it as an actual upgrading, and doesn't process response body.

c.f. nodejs/http-parser#363

@Gargron
Copy link

Gargron commented Jul 23, 2017

👍

@tarcieri
Copy link
Member

tarcieri commented Nov 6, 2018

Picking up the discussion from #508, hey @ixti, is #433 still the path forward on that?

Perhaps we should open an upstream issue about this on http_parser.rb (unless there already is one?)

@janko
Copy link
Member

janko commented Nov 7, 2018

@tarcieri Considering that http_parser.rb doesn't work on JRuby 9.2.0.0, I think the path forward really is switching to http-parser gem, but I also want to hear what @ixti thinks. http-protocol also looks like a good option, if we want pure Ruby.

@tarcieri
Copy link
Member

tarcieri commented Nov 7, 2018

Last time I benchmarked a pure Ruby parser versus the Node parser, it was multiple orders of magnitude difference in performance...

@ixti
Copy link
Member

ixti commented Nov 8, 2018

Yeah. my plan was to switch to http-parser but with shim for it (I don't like it's API).

@tarcieri
Copy link
Member

tarcieri commented Nov 8, 2018

@ixti that's basically what you were working on in #433, right?

@ixti
Copy link
Member

ixti commented Nov 8, 2018

@tarcieri Yup!

@ixti
Copy link
Member

ixti commented Feb 4, 2019

I have cut 5.0.0 pre release version of gem with switch to updated http parser. This should be fixed now.

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

No branches or pull requests

5 participants