The bad arithmetic didn't result in wrong or insecure behavior
(there were no out-of-bound accesses and forward progress was
still being made because of the outer loop) but it did regress
the performance of header field parsing rather massively.
Taking the test suite as an example: the inner fast path loop
was skipped over 4.4 million times, falling back to the outer
slow path loop. After this commit that only happens about
2,000 times - a 2,200-fold reduction.
Fixes: https://github.com/nodejs/http-parser/issues/473
PR-URL: https://github.com/nodejs/http-parser/pull/474
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Commit 2a0d106 ("http_parser: revert `memchr()` optimization")
introduced a -Wsign-compare warning that manifests itself with
gcc 7.4.0 but not older versions.
The type of p - q is signed when p and q are pointers and was
compared against an unsigned type. Its actual value is always >= 0
however and can be safely cast to size_t.
Fixes: https://github.com/nodejs/http-parser/issues/471
PR-URL: https://github.com/nodejs/http-parser/pull/472
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
An optimization was introduced in c6097e1d and 0097de58. The crux of
optimization was to skip all characters in header value until either
of CR or LF. Unfortunately, this optimization comes at cost of
inconsistency in header value validation, which might lead to security
issue due to violated expectations in the user code.
Partially revert the optimization, and add additional check to make
general header value parsing consistent.
Fix: #468
PR-URL: https://github.com/nodejs/http-parser/pull/469
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Harvey Tuch <htuch@google.com>
Changes:
* Cache parser->nread in a local variable (same optimization that was
already in place for parser->state).
* Move the cost of the conditional branch for spaces in tokens out of
the fast-tokenizer implementation and into the strict-tokenizer
implementation.
Together, these changes yield a ~15% increase in MB/s and req/s in
the included bench program on x86_64.
PR-URL: https://github.com/nodejs/http-parser/pull/422
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Before this commit `Content-Length: 4 2` was accepted as a valid header
and recorded as `parser->content_length = 42`. Now it is a parse error
that fails with error `HPE_INVALID_CONTENT_LENGTH`.
Downstream users that inspect `parser->content_length` and naively parse
the string value using `strtoul()` might get confused by the discrepancy
between the two values. Resolve that by simply not letting it happen.
Fixes: https://github.com/nodejs-private/security/issues/178
PR-URL: https://github.com/nodejs-private/http-parser-private/pull/1
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
`strtoul` will attempt to lookup the next digit up until it will stumble
upon an invalid one. However, for an unterminated string as an input
value, this results in out-of-bounds read.
Remove `strtoul` call, and replace it with simple loop.
Fix: #408
PR-URL: https://github.com/nodejs/http-parser/pull/409
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Fix symbolic links so they are now relative, and also removes a
dangling symlink that was left there with make uninstall
(libhttp_parser.so was left pointing to a non-existant file previously).
Don't hard-code the library name, match the file names of `make install`.
PR-URL: https://github.com/nodejs/http-parser/pull/391
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Modified the Makefile so that it actually works. As it is now, symlinks
will point to the fully qualified path of wherever your DESTDIR was: so
one would end up with symlinks like:
/usr/local/lib/libhttp_parser.so.2.7
pointing to:
/tmp/http-parser-git/pkg/http-parser-git/usr/local/lib/libhttp_parser.so.2.7.1.
This caused quite a bit of trouble when trying to package it, this
patch makes it work.
PR-URL: https://github.com/nodejs/http-parser/pull/391
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
The boilerplate included attribution to NGINX that created confusion
because NGINX is distributed under a different license (BSD, not MIT.)
To the best of everyone's knowledge, no actual NGINX code remains.
Remove the attribution to clear up the confusion.
Fixes: https://github.com/nodejs/http-parser/issues/389
PR-URL: https://github.com/nodejs/http-parser/pull/390
Reviewed-By: Fedor Indutny <fedor@indutny.com>
can use same switch-lookup for '-' char case
move PROPFIND and PURGE to be next to the other P methods
change IS_ALPHA(ch) to A <= ch <= Z
(very slight optimization, only uppercase will match in switch)
PR-URL: https://github.com/nodejs/http-parser/pull/323
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Raised in issue #356, reduce version number in SONAME to MAJOR.MINOR.
While at it, create a symlink the from SONAME to the library, instead of
the other way around, and add a (standard) unversioned symlink to the
library to aid the ordinary linking process.
PR-URL: https://github.com/nodejs/http-parser/pull/359
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
The include is required for type size_t. stddef.h should be available
on every platform, sys/types.h is not.
PR-URL: https://github.com/nodejs/http-parser/pull/360
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
- original fix is from daeon: https://github.com/daeon/http-parser/
"Tolerate web servers which do not return a status message in the
return response.
I have noticed this usse on several websites such downloads from
mediafire.com"
- original pull request: https://github.com/nodejs/http-parser/pull/254
- i merely added the status_cb_called unit test check, there already
is a test that triggers this without the patch (a 301 without a
reason phrase).
PR-URL: https://github.com/nodejs/http-parser/pull/367
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Only one digit is allowed for the major version and only one is
allowed for the minor version according to RFC 7230.
PR-URL: https://github.com/nodejs/http-parser/pull/366
Reviewed-By: Fedor Indutny <fedor@indutny.com>
This patch provides an enum for the standardized HTTP status codes.
Additionally, the HTTP_STATUS_MAP(XX) can be used for other purposes as
well, such as code-to-name lookups and code-based switch statements.
PR-URL: https://github.com/nodejs/http-parser/pull/337
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Microsoft C++ compiler fails when this assert appears before the
declarations. Moved assert to appear after the declarations.
PR-URL: https://github.com/nodejs/http-parser/pull/293
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Returning `2` from on_headers_complete will tell parser that it
should not expect neither a body nor any futher responses on
this connection. This is useful for handling responses to a
CONNECT request which may not contain `Upgrade` or
`Connection: upgrade` headers.
See: https://github.com/nodejs/node/pull/6198
PR-URL: https://github.com/nodejs/http-parser/pull/299
Reviewed-By: Brian White <mscdex@mscdex.net>
Fixes a header parsing bug for obstext characters (> 0x80)
Adaption of nodejs/node@954a4b4b:
Author: James M Snell <jasnell@gmail.com>
Date: Mon Feb 15 09:40:58 2016 -0800
deps: update to http-parser 2.6.2
Fixes http-parser regression with IS_HEADER_CHAR check
Add test case for obstext characters (> 0x80) is header
PR-URL: https://github.com/nodejs/node/pull/5237
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
PR-URL: https://github.com/nodejs/http-parser/pull/287
Reviewed-By: James M Snell <jasnell@gmail.com>
Ben Noordhuis
Fedor Indutny
Sergey Zolotarev
Olga Batyshkina
cjihrig
Welling Guzmán
dnartz
Matt Klein
Matthias Loy
Brian Pane
Welling Guzman
Stefano Sambi
Fedor Indutny
Jon Moss
ShaRose
Fabian Groffen
Pierce Lopez
André Klitzing
Stephan Gatzka
Thomas 'fake' Jakobi
Brian White
Nathaniel McCallum
Guo Xiao
oldratlee
Jonathan Marler
Johan Bergström