-
Notifications
You must be signed in to change notification settings - Fork 30.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fatal error when attempting to (wrongly) set incoming packet encoding #18118
Labels
http
Issues or PRs related to the http subsystem.
Comments
2 tasks
2 tasks
iSkore
added a commit
to iSkore/node
that referenced
this issue
Apr 3, 2018
added override to socket.setEncoding to not allow encoding changes for incoming HTTP requests added tests to ensure method throws JavaScript error because an HTTP buffer must be in US-ASCII, this function should not be allowed and should throw an Error currently, the process encounters a fatal v8 error and crashes error report detailed in [issue nodejs#18118](nodejs#18118) Fixes: nodejs#18118 Ref: nodejs#18178
iSkore
added a commit
to iSkore/node
that referenced
this issue
Apr 3, 2018
added test to ensure setEncoding inside socket connection would throw an error Fixes: nodejs#18118 Ref: nodejs#18178
iSkore
added a commit
to iSkore/node
that referenced
this issue
Apr 3, 2018
added ERR_HTTP_INCOMING_SOCKET_ENCODING error and error docs throw ERR_HTTP_INCOMING_SOCKET_ENCODING error when incoming request socket encoding is manipulated error report detailed in nodejs#18118 Fixes: nodejs#18118 Ref: nodejs#18178
iSkore
added a commit
to iSkore/node
that referenced
this issue
Apr 3, 2018
added note and link to RFC7230 >A recipient MUST parse an HTTP message as a sequence of octets in an encoding that is a superset of US-ASCII [USASCII]. Ref: https://tools.ietf.org/html/rfc7230#section-3 Ref: nodejs#18118 Ref: nodejs#18178 Ref: nodejs#19344
iSkore
added a commit
to iSkore/node
that referenced
this issue
Apr 3, 2018
internal/errors declaration updated to match new scheme PR-URL: nodejs#19344 Ref: https://tools.ietf.org/html/rfc7230#section-3 Ref: nodejs#18118 Ref: nodejs#18178 Ref: nodejs#19344
iSkore
added a commit
to iSkore/node
that referenced
this issue
Apr 3, 2018
@nodejs/http-parser |
iSkore
added a commit
to iSkore/node
that referenced
this issue
May 14, 2020
applied updates from previous pull-requests to disallow socket.setEncoding before a http connection is parsed. wrapped socket.setEncoding to throw an error. previously resulted in a fatal error Fixes: nodejs#18118 Ref: nodejs#18178 Ref: nodejs#19344
4 tasks
ping @nodejs/http-parser and teammates |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Error occurs in both versions of Node (likely because
http_parser
is the same version)v8.9.4
Output from
process.versions
(v8.9.4)v9.4.0
Output from
process.versions
(v9.4.0)Darwin MacBook-Pro 17.3.0 Darwin Kernel Version 17.3.0: Thu Nov 9 18:09:22 PST 2017; root:xnu-4570.31.3~1/RELEASE_X86_64 x86_64
node_http_parser
Repeatability: constant
Error:
node index.js
Packet submitted to process:
Tested request with
printf "GET / HTTP/1.1\r\nHost: localhost:3000\r\n\r\n" > /dev/tcp/localhost/3000
With the example code (above), the process immediately terminates when any request is made and prints out the fatal V8 error (above)
Since HTTP uses US-ASCII, I completely understand why an incoming packet (encoded differently) would break
node_http_parser
.This fatal error bypasses all my attempts to catch errors on the
process
and crashes.I typically expect a clean error report, stackTrace, error code and all.
But this appears to be a fatal crash due to user error without being caught in the
process
level error handling.I understand that
setEncoding
was meant for the response packet, but I believe this error report could lead to confusion.I recommend blocking (and throwing and error) for any attempts to set encoding on an incoming packet/connection.
Ref: #18178
Ref: #19344
The text was updated successfully, but these errors were encountered: