Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

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

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
Collaborator

@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 referenced this pull request from a commit
@kolbyjack kolbyjack src: simple Connection header multi-value parsing
Reviewed-By: Fedor Indutny <fedor@indutny.com>
PR-URL: #100
091ebb8
@indutny
Collaborator

Landed in 091ebb8, sorry for long wait!

@indutny indutny closed this
@KjellSchubert KjellSchubert referenced this pull request from a commit in KjellSchubert/http-parser
@kolbyjack kolbyjack src: simple Connection header multi-value parsing
Reviewed-By: Fedor Indutny <fedor@indutny.com>
PR-URL: joyent#100
7a20c8c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 125 additions and 30 deletions.
  1. +84 −26 http_parser.c
  2. +4 −3 http_parser.h
  3. +37 −1 test.c
View
110 http_parser.c
@@ -330,12 +330,16 @@ enum header_states
, h_upgrade
, h_matching_transfer_encoding_chunked
+ , h_matching_connection_token_start
, h_matching_connection_keep_alive
, h_matching_connection_close
+ , h_matching_connection_upgrade
+ , h_matching_connection_token
, h_transfer_encoding_chunked
, h_connection_keep_alive
, h_connection_close
+ , h_connection_upgrade
};
enum http_host_state
@@ -366,6 +370,7 @@ enum http_host_state
#define IS_USERINFO_CHAR(c) (IS_ALPHANUM(c) || IS_MARK(c) || (c) == '%' || \
(c) == ';' || (c) == ':' || (c) == '&' || (c) == '=' || (c) == '+' || \
(c) == '$' || (c) == ',')
+#define STRICT_TOKEN(c) (tokens[(unsigned char)c])
#if HTTP_PARSER_STRICT
#define TOKEN(c) (tokens[(unsigned char)c])
@@ -1406,8 +1411,10 @@ size_t http_parser_execute (http_parser *parser,
/* looking for 'Connection: close' */
} else if (c == 'c') {
parser->header_state = h_matching_connection_close;
+ } else if (c == 'u') {
+ parser->header_state = h_matching_connection_upgrade;
} else {
- parser->header_state = h_general;
+ parser->header_state = h_matching_connection_token;
}
break;
@@ -1480,12 +1487,26 @@ size_t http_parser_execute (http_parser *parser,
}
break;
+ case h_matching_connection_token_start:
+ /* looking for 'Connection: keep-alive' */
+ if (c == 'k') {
+ parser->header_state = h_matching_connection_keep_alive;
+ /* looking for 'Connection: close' */
+ } else if (c == 'c') {
+ parser->header_state = h_matching_connection_close;
+ } else if (c == 'u') {
+ parser->header_state = h_matching_connection_upgrade;
+ } else if (STRICT_TOKEN(c)) {
+ parser->header_state = h_matching_connection_token;
+ }
+ break;
+
/* looking for 'Connection: keep-alive' */
case h_matching_connection_keep_alive:
parser->index++;
if (parser->index > sizeof(KEEP_ALIVE)-1
|| c != KEEP_ALIVE[parser->index]) {
- parser->header_state = h_general;
+ parser->header_state = h_matching_connection_token;
} else if (parser->index == sizeof(KEEP_ALIVE)-2) {
parser->header_state = h_connection_keep_alive;
}
@@ -1495,16 +1516,50 @@ size_t http_parser_execute (http_parser *parser,
case h_matching_connection_close:
parser->index++;
if (parser->index > sizeof(CLOSE)-1 || c != CLOSE[parser->index]) {
- parser->header_state = h_general;
+ parser->header_state = h_matching_connection_token;
} else if (parser->index == sizeof(CLOSE)-2) {
parser->header_state = h_connection_close;
}
break;
+ /* looking for 'Connection: upgrade' */
+ case h_matching_connection_upgrade:
+ parser->index++;
+ if (parser->index > sizeof(UPGRADE)-1
+ || c != UPGRADE[parser->index]) {
+ parser->header_state = h_matching_connection_token;
+ } else if (parser->index == sizeof(UPGRADE)-2) {
+ parser->header_state = h_connection_upgrade;
+ }
+ break;
+
+ case h_matching_connection_token:
+ if (ch == ',') {
+ parser->header_state = h_matching_connection_token_start;
+ parser->index = 0;
+ }
+ break;
+
case h_transfer_encoding_chunked:
+ if (ch != ' ') parser->header_state = h_general;
+ break;
+
case h_connection_keep_alive:
case h_connection_close:
- if (ch != ' ') parser->header_state = h_general;
+ case h_connection_upgrade:
+ if (ch == ',') {
+ if (parser->header_state == h_connection_keep_alive) {
+ parser->flags |= F_CONNECTION_KEEP_ALIVE;
+ } else if (parser->header_state == h_connection_close) {
+ parser->flags |= F_CONNECTION_CLOSE;
+ } else if (parser->header_state == h_connection_upgrade) {
+ parser->flags |= F_CONNECTION_UPGRADE;
+ }
+ parser->header_state = h_matching_connection_token_start;
+ parser->index = 0;
+ } else if (ch != ' ') {
+ parser->header_state = h_matching_connection_token;
+ }
break;
default:
@@ -1520,34 +1575,35 @@ size_t http_parser_execute (http_parser *parser,
STRICT_CHECK(ch != LF);
parser->state = s_header_value_lws;
-
- switch (parser->header_state) {
- case h_connection_keep_alive:
- parser->flags |= F_CONNECTION_KEEP_ALIVE;
- break;
- case h_connection_close:
- parser->flags |= F_CONNECTION_CLOSE;
- break;
- case h_transfer_encoding_chunked:
- parser->flags |= F_CHUNKED;
- break;
- default:
- break;
- }
-
break;
}
case s_header_value_lws:
{
- if (ch == ' ' || ch == '\t')
- parser->state = s_header_value_start;
- else
- {
+ if (ch == ' ' || ch == '\t') {
+ parser->state = s_header_value;
+ MARK(header_value);
+ } else {
+ switch (parser->header_state) {
+ case h_connection_keep_alive:
+ parser->flags |= F_CONNECTION_KEEP_ALIVE;
+ break;
+ case h_connection_close:
+ parser->flags |= F_CONNECTION_CLOSE;
+ break;
+ case h_connection_upgrade:
+ parser->flags |= F_CONNECTION_UPGRADE;
+ break;
+ case h_transfer_encoding_chunked:
+ parser->flags |= F_CHUNKED;
+ break;
+ default:
+ break;
+ }
+
parser->state = s_header_field_start;
- goto reexecute_byte;
}
- break;
+ goto reexecute_byte;
}
case s_headers_almost_done:
@@ -1565,7 +1621,9 @@ size_t http_parser_execute (http_parser *parser,
/* Set this here so that on_headers_complete() callbacks can see it */
parser->upgrade =
- (parser->flags & F_UPGRADE || parser->method == HTTP_CONNECT);
+ ((parser->flags & (F_UPGRADE | F_CONNECTION_UPGRADE)) ==
+ (F_UPGRADE | F_CONNECTION_UPGRADE) ||
+ parser->method == HTTP_CONNECT);
/* Here we call the headers_complete callback. This is somewhat
* different than other callbacks because if the user returns 1, we
View
7 http_parser.h
@@ -125,9 +125,10 @@ enum flags
{ F_CHUNKED = 1 << 0
, F_CONNECTION_KEEP_ALIVE = 1 << 1
, F_CONNECTION_CLOSE = 1 << 2
- , F_TRAILING = 1 << 3
- , F_UPGRADE = 1 << 4
- , F_SKIPBODY = 1 << 5
+ , F_CONNECTION_UPGRADE = 1 << 3
+ , F_TRAILING = 1 << 4
+ , F_UPGRADE = 1 << 5
+ , F_SKIPBODY = 1 << 6
};
View
38 test.c
@@ -612,7 +612,7 @@ const struct message requests[] =
,.request_path= "/"
,.request_url= "/"
,.num_headers= 2
- ,.headers= { { "Line1", "abcdefghijklmno qrs" }
+ ,.headers= { { "Line1", "abc\tdef ghi\t\tjkl mno \t \tqrs" }
, { "Line2", "line2\t" }
}
,.body= ""
@@ -897,6 +897,42 @@ const struct message requests[] =
,.body= ""
}
+#define CONNECTION_MULTI 34
+, {.name = "multiple connection header values with folding"
+ ,.type= HTTP_REQUEST
+ ,.raw= "GET /demo HTTP/1.1\r\n"
+ "Host: example.com\r\n"
+ "Connection: Something,\r\n"
+ " Upgrade, ,Keep-Alive\r\n"
+ "Sec-WebSocket-Key2: 12998 5 Y3 1 .P00\r\n"
+ "Sec-WebSocket-Protocol: sample\r\n"
+ "Upgrade: WebSocket\r\n"
+ "Sec-WebSocket-Key1: 4 @1 46546xW%0l 1 5\r\n"
+ "Origin: http://example.com\r\n"
+ "\r\n"
+ "Hot diggity dogg"
+ ,.should_keep_alive= TRUE
+ ,.message_complete_on_eof= FALSE
+ ,.http_major= 1
+ ,.http_minor= 1
+ ,.method= HTTP_GET
+ ,.query_string= ""
+ ,.fragment= ""
+ ,.request_path= "/demo"
+ ,.request_url= "/demo"
+ ,.num_headers= 7
+ ,.upgrade="Hot diggity dogg"
+ ,.headers= { { "Host", "example.com" }
+ , { "Connection", "Something, Upgrade, ,Keep-Alive" }
+ , { "Sec-WebSocket-Key2", "12998 5 Y3 1 .P00" }
+ , { "Sec-WebSocket-Protocol", "sample" }
+ , { "Upgrade", "WebSocket" }
+ , { "Sec-WebSocket-Key1", "4 @1 46546xW%0l 1 5" }
+ , { "Origin", "http://example.com" }
+ }
+ ,.body= ""
+ }
+
, {.name= NULL } /* sentinel */
};
Something went wrong with that request. Please try again.