Skip to content

Commit

Permalink
Fix ABI breakage
Browse files Browse the repository at this point in the history
Fix ABI breakage introduced in commit 7d5c99d ("Support multi-coding
Transfer-Encoding") by undoing the change in `sizeof(http_parser)`.

Restore the size of the `flags` field and shrink the `index` field from
7 to 5 bits. It track strings up to `strlen("Transfer-Encoding")` bytes
so 2^5 == 32 is wide enough.

Fixes: nodejs#502
PR-URL: nodejs#503
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
  • Loading branch information
bnoordhuis committed Mar 24, 2020
1 parent 1c02cb9 commit 714cbb2
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 10 deletions.
11 changes: 7 additions & 4 deletions http_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,7 @@ size_t http_parser_execute (http_parser *parser,
if (ch == CR || ch == LF)
break;
parser->flags = 0;
parser->extra_flags = 0;
parser->content_length = ULLONG_MAX;

if (ch == 'H') {
Expand Down Expand Up @@ -768,6 +769,7 @@ size_t http_parser_execute (http_parser *parser,
if (ch == CR || ch == LF)
break;
parser->flags = 0;
parser->extra_flags = 0;
parser->content_length = ULLONG_MAX;

if (ch == 'H') {
Expand Down Expand Up @@ -925,6 +927,7 @@ size_t http_parser_execute (http_parser *parser,
if (ch == CR || ch == LF)
break;
parser->flags = 0;
parser->extra_flags = 0;
parser->content_length = ULLONG_MAX;

if (UNLIKELY(!IS_ALPHA(ch))) {
Expand Down Expand Up @@ -1338,7 +1341,7 @@ size_t http_parser_execute (http_parser *parser,
parser->header_state = h_general;
} else if (parser->index == sizeof(TRANSFER_ENCODING)-2) {
parser->header_state = h_transfer_encoding;
parser->flags |= F_TRANSFER_ENCODING;
parser->extra_flags |= F_TRANSFER_ENCODING >> 8;
}
break;

Expand Down Expand Up @@ -1800,7 +1803,7 @@ size_t http_parser_execute (http_parser *parser,

/* Cannot us transfer-encoding and a content-length header together
per the HTTP specification. (RFC 7230 Section 3.3.3) */
if ((parser->flags & F_TRANSFER_ENCODING) &&
if ((parser->extra_flags & (F_TRANSFER_ENCODING >> 8)) &&
(parser->flags & F_CONTENTLENGTH)) {
/* Allow it for lenient parsing as long as `Transfer-Encoding` is
* not `chunked`
Expand Down Expand Up @@ -1886,7 +1889,7 @@ size_t http_parser_execute (http_parser *parser,
/* chunked encoding - ignore Content-Length header,
* prepare for a chunk */
UPDATE_STATE(s_chunk_size_start);
} else if (parser->flags & F_TRANSFER_ENCODING) {
} else if (parser->extra_flags & (F_TRANSFER_ENCODING >> 8)) {
if (parser->type == HTTP_REQUEST && !lenient) {
/* RFC 7230 3.3.3 */

Expand Down Expand Up @@ -2162,7 +2165,7 @@ http_message_needs_eof (const http_parser *parser)
}

/* RFC 7230 3.3.3, see `s_headers_almost_done` */
if ((parser->flags & F_TRANSFER_ENCODING) &&
if ((parser->extra_flags & (F_TRANSFER_ENCODING >> 8)) &&
(parser->flags & F_CHUNKED) == 0) {
return 1;
}
Expand Down
13 changes: 7 additions & 6 deletions http_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ enum flags
, F_UPGRADE = 1 << 5
, F_SKIPBODY = 1 << 6
, F_CONTENTLENGTH = 1 << 7
, F_TRANSFER_ENCODING = 1 << 8
, F_TRANSFER_ENCODING = 1 << 8 /* Never set in http_parser.flags */
};


Expand Down Expand Up @@ -272,13 +272,13 @@ enum flags
"unexpected content-length header") \
XX(INVALID_CHUNK_SIZE, \
"invalid character in chunk size header") \
XX(INVALID_TRANSFER_ENCODING, \
"request has invalid transfer-encoding") \
XX(INVALID_CONSTANT, "invalid constant string") \
XX(INVALID_INTERNAL_STATE, "encountered unexpected internal state")\
XX(STRICT, "strict mode assertion failed") \
XX(PAUSED, "parser is paused") \
XX(UNKNOWN, "an unknown error occurred")
XX(UNKNOWN, "an unknown error occurred") \
XX(INVALID_TRANSFER_ENCODING, \
"request has invalid transfer-encoding") \


/* Define HPE_* values for each errno value above */
Expand All @@ -296,11 +296,12 @@ enum http_errno {
struct http_parser {
/** PRIVATE **/
unsigned int type : 2; /* enum http_parser_type */
unsigned int flags : 8; /* F_* values from 'flags' enum; semi-public */
unsigned int state : 7; /* enum state from http_parser.c */
unsigned int header_state : 7; /* enum header_state from http_parser.c */
unsigned int index : 7; /* index into current matcher */
unsigned int index : 5; /* index into current matcher */
unsigned int extra_flags : 2;
unsigned int lenient_http_headers : 1;
unsigned int flags : 16; /* F_* values from 'flags' enum; semi-public */

uint32_t nread; /* # bytes read in various scenarios */
uint64_t content_length; /* # bytes in body (0 if no Content-Length header) */
Expand Down
1 change: 1 addition & 0 deletions test.c
Original file line number Diff line number Diff line change
Expand Up @@ -4221,6 +4221,7 @@ main (void)
printf("http_parser v%u.%u.%u (0x%06lx)\n", major, minor, patch, version);

printf("sizeof(http_parser) = %u\n", (unsigned int)sizeof(http_parser));
assert(sizeof(http_parser) == 4 + 4 + 8 + 2 + 2 + 4 + sizeof(void *));

//// API
test_preserve_data();
Expand Down

0 comments on commit 714cbb2

Please sign in to comment.