Without this change, it is possible to get an assertion to fail by
continuing to call http_parser_execute after it has returned an error.
Specifically, the parser could be called with parser->state ==
s_chunk_size_almost_done and parser->flags & F_CHUNKED set. Then,
F_CHUNKED could have been cleared, and an error could be hit. In this
case, the parser would have returned with F_CHUNKED clear, but
parser->state == s_chunk_size_almost_done, resulting in an assertion
failure on the next call.
There are alternate solutions possible, including just saving all of
the fields (state included) on error.
I didn't add a test case because this is a bit annoying to test, but I
can add one if necesssary.
acceptable_header[x] is always assigned to a variable of type char, so
the 'unsigned' is unnecessary.
The other arrays can be of type int8_t/uint8_t to save space.
Yay valgrind testing
I don't believe that this actually mattered at all, because state was
initialized correctly, and flags would be set to 0 almost immediately
anyways.
This matters because char is signed by default on x86, so bytes with
values above 127 could have theoretically survived a pass through
lowcase (assuming that there was some non-zero data before the lowcase
array).
This also fixes test failures from the previous commit.
It also adds support for the LOCK method, which was previously
missing.
This brings the size of http_parser from 44 bytes to 32 bytes. It
also makes the code substantially shorter, at a slight cost in
craziness.
Currently this test fails, because short method strings do not cause
failures, even if they are unknown methods. However, long unknown
method strings do cause errors.
This saves space in the structure (it is now 28 bytes on x86), and
makes the handling of content_length more consistent between chunked
encoding and non-chunked-encoding.
This fixes a possible issue where a very large body (one that involves
> 80*1024 calls to http_parser_execute) will cause the next request
with that parser to return an error because it believes that this is
an overflow condition.
The *_mark members were actually being used as just boolean values to
the next call of the parser. However, you can calculate if the mark
members should be set or not purely based on the current state, so
they can just be gotten rid of entirely.
This is mostly done by using sized types instead of enums, and
reordering fields to allow better packing.
I also moved the 'upgrade' field out of the PRIVATE section and into
the READ-ONLY section, as I believe that it is supposed to be
non-private.
This does have some slight functional changes in cases where
MAX_FIELD_SIZE is hit, specficially if a URL is made up of many
components, each of which is smaller than MAX_FIELD_SIZE, but the
total together is greater than MAX_FIELD_SIZE, then we now might not
call callbacks for any of the components (even the ones that are
smaller than 80kb). With the old code, it was possible to get a
callback for query_string and never get a callback for the URL (or at
least the end of the URL that is past 80kb), if the callback for the
URL would have been larger than 80kb.
(to be honest, I'm surprised that the MAX_FIELD_SIZE is implemented in
http_parser at all, instead of requiring that callers pay attention to
it, as it feels like it should be the caller's responsibility)
Ryan Dahl
Ewen Cheslack-Postava
Nathan Rajlich
Cliff Frey
Ben Noordhuis
Jeff Terrace
Tim Becker
Santiago Gala