Skip to content
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.

Sign up for GitHub

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

Jump to bottom

HTTP GET Request with a long URI produces a 400 Bad Request error (should be 414) #26296

Closed
RMHonor opened this issue Feb 25, 2019 · 4 comments
Closed

HTTP GET Request with a long URI produces a 400 Bad Request error (should be 414) #26296

RMHonor opened this issue Feb 25, 2019 · 4 comments
Labels
http Issues or PRs related to the http subsystem.

Comments

@RMHonor
Copy link

RMHonor commented Feb 25, 2019

  • Version: v10.13.0 (docker image node:lts-alpine)
  • Platform: Linux 552c097b0775 4.9.125-linuxkit deps: update openssl to 1.0.1j #1 SMP Fri Sep 7 08:20:28 UTC 2018 x86_64 Linux (docker image node:lts-alpine)

Performing a GET request with a URI too long (as specified in https://github.com/nodejs/http-parser/blob/master/http_parser.h#L55) throws a 400 Bad Request error. This should be a 414 URI Too Long.
@dougwilson dougwilson mentioned this issue Feb 25, 2019
Closed
@lpinca lpinca added the http Issues or PRs related to the http subsystem. label Mar 2, 2019
@lpinca
Copy link
Member

lpinca commented Mar 3, 2019

It will soon (Node.js 12) return 431 instead of a generic 400. See #25605.

The code returned is the same (HPE_HEADER_OVERFLOW) so I'm not sure if we can differentiate between 414 and 431.

@bnoordhuis
Copy link
Member

You can distinguish between them when an on_url callback has been observed but no on_status or on_header_field callbacks. I'm inclined to say it's not worth the additional complexity though.

@caiolrm caiolrm mentioned this issue Mar 9, 2019
Closed
4 tasks
caiolrm added a commit to caiolrm/node that referenced this issue Mar 10, 2019
@caiolrm
http: create an option for setting a maximum size for uri parsing
45fa652 
The http server wasn't able to tell exactly what caused an
HPE_HEADER_OVERFLOW, meaning it would yield a 431 error even if what
caused it was the request URI being too long.

This adds a limit to the URI sizes through a new option called
max-http-uri-size, which will be checked against the actual URIs
after on_url callback at the node_http_parser_impl file.

Fixes: nodejs#26296
Refs: expressjs/express#3898
@jasnell
Copy link
Member

jasnell commented Jun 26, 2020

Is there anything actionable here?

@Fdawgs Fdawgs mentioned this issue Jun 26, 2023
Merged
2 tasks
@marco-ippolito marco-ippolito reopened this Nov 2, 2023
@marco-ippolito
Copy link
Member

Fixed by #25605

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
http Issues or PRs related to the http subsystem.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

http: create an option for setting a maximum size for uri parsing caiolrm/node
5 participants
@bnoordhuis @jasnell @lpinca @RMHonor @marco-ippolito