Skip to content

Error handling for HPE_HEADER_OVERFLOW in HTTP server does not take into account socket re-use #37685

Closed
@ramesius

Description

@ramesius
  • Version: 12.19.0 and above
  • Platform: Darwin 19.6.0 Darwin Kernel Version 19.6.0: Tue Jan 12 22:13:05 PST 2021; root:xnu-6153.141.16~1/RELEASE_X86_64 x86_64

What steps will reproduce the bug?

Create a HTTP agent with keep-alive set to true, max sockets and max free sockets to 1 (for simplicity).
Send a request to a NodeJS HTTP server, resulting in a successful response.
Send a 2nd request, this time with a payload that will trigger HPE_HEADER_OVERFLOW (send a large header value, larger than max-header-size-limit).
Observe that the 2nd request has the socket destroyed, instead of receiving a HTTP 431.

The above steps are implemented in this gist: https://gist.github.com/ramesius/6d9fbd79d76ca36e555855275cf158c0 which is able to reproduce between node 12.18.4 and 12.19.0. For later versions it might require the example header to be increased in size.

How often does it reproduce? Is there a required condition?

Any socket that is re-used will not be able to respond with a HTTP 431.

What is the expected behavior?

A request that results in HPE_HEADER_OVERFLOW regardless of a one-use socket or a re-used socket should be able to return HTTP 431.

What do you see instead?

Any subsequent request over a re-used socket that triggers HPE_HEADER_OVERFLOW results in the socket being destroyed.
This is particularly troublesome when an application is behind any other service that keeps connections alive (for example a load balancer) for performance, the connections are unexpectedly closed which results in the service incorrectly interpreting what would have been a HTTP 431 as HTTP 503.

Additional information

The expected behavior was present in versions 12.18.4 and below.
Suspected buggy code: https://github.com/nodejs/node/blob/master/lib/_http_server.js#L658
When listening for clientError on the server we can also see that bytesWritten is > 0

This is the commit that introduced the changes

Metadata

Metadata

Assignees

No one assigned

    Labels

    confirmed-bugIssues with confirmed bugs.httpIssues or PRs related to the http subsystem.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions