Add simple Connection header multi-value parsing #100

Closed
wants to merge 5 commits into
from

5 participants

@kolbyjack

Addresses #99: http-parser doesn't currently parse Connection: Upgrade at all, it only parses the Upgrade: header. This adds Connection: Upgrade parsing as well as correctly parsing multiple values in the Connection header.

@kolbyjack

Actually, this may be more correct as else if (TOKEN(c)) now that I think about it

@cmr

Did this ever get reviewed?

@bnoordhuis
Node.js Foundation member

I don't think so, it slipped under my radar. It's a good feature to have but the PR needs rebasing.

Surprisingly, so far zero node.js users have complained about this. I speculate most WS libraries use the Upgrade header instead of the Connection header.

@cmr

I merged it (a few simple-to-resolve conflicts) in my master branch but tests don't pass. @kolbyjack still interested in this?

@kolbyjack

Apparently I merged with master in a bad way, since it shows all of the changes I pulled in from upstream. The tests pass for me with this update, though

@cmr

The numbering on your tests is off, you have two 31's. Not sure how that'd affect things though

@kolbyjack

I'll move the test to the end, but those defines aren't ever referenced, so it shouldn't matter

@cmr

Yeah, tests pass here from a fresh repo, not sure what was in mine that was nasty.

@cmr

@bnoordhuis Tests pass; anything else?

@markpotts123

Hi,
I have used http-parser successfully for a project and wondered if you have any plans for future upgrades, e.g. SPDY or HTTP/2 support
Mark

@indutny
Node.js Foundation member

@markmontymark hello, definitely no HTTP2/SPDY plans, this project will remain http/1.1/http/1.0 parser.

@markpotts123

Fair enough, just thought I'd ask ;-) Appreciate the project anyway. It works well. Mark

@indutny indutny added a commit that referenced this pull request Dec 5, 2014
@kolbyjack kolbyjack src: simple Connection header multi-value parsing
Reviewed-By: Fedor Indutny <fedor@indutny.com>
PR-URL: #100
091ebb8
@indutny
Node.js Foundation member

Landed in 091ebb8, sorry for long wait!

@indutny indutny closed this Dec 5, 2014
@KjellSchubert KjellSchubert pushed a commit to KjellSchubert/http-parser that referenced this pull request Apr 18, 2015
@kolbyjack kolbyjack src: simple Connection header multi-value parsing
Reviewed-By: Fedor Indutny <fedor@indutny.com>
PR-URL: nodejs#100
7a20c8c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment