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.

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

doc: fix description of the max header size in HPE_HEADER_OVERFLOW section #54125

Merged
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion doc/api/errors.md
Original file line number Diff line number Diff line change
Expand Up @@ -3180,6 +3180,9 @@ Creation of a [`zlib`][] object failed due to incorrect configuration.

<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/54125
description: Corrected the description of the max header size to `maxHeaderSize`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/54125
description: Corrected the description of the max header size to `maxHeaderSize`.

I might be wrong, but I think this is only for changes to the API, not the docs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the same thing but the comment below says

description: Max header size in `http_parser` was set to 8 KiB.

which is misleading.

Copy link
Member

@RedYetiDev RedYetiDev Aug 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit for that one changes the API (1860352), but I'm not sure. There's no harm in leaving it in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RedYetiDev @lpinca
The PR changed from 8 KiB to 16 KiB appears to be here.

The following change log is left in other than errors.md, is such a description also unnecessary?

node/doc/api/cli.md

Lines 1554 to 1557 in e4f61de

changes:
- version: v13.13.0
pr-url: https://github.com/nodejs/node/pull/32520
description: Change maximum default size of HTTP headers from 8 KiB to 16 KiB.

Copy link
Contributor Author

@erm1116 erm1116 Aug 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said above, but there was no particular change log description in http.md.

node/doc/api/http.md

Lines 3751 to 3763 in e4f61de

## `http.maxHeaderSize`
<!-- YAML
added:
- v11.6.0
- v10.15.0
-->
* {number}
Read-only property specifying the maximum allowed size of HTTP headers in bytes.
Defaults to 16 KiB. Configurable using the [`--max-http-header-size`][] CLI
option.

I thought this change might not be necessary this time, so I'm going to remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed 944900b

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

- version:
- v11.4.0
- v10.15.0
Expand All @@ -3189,7 +3192,7 @@ changes:
-->

Too much HTTP header data was received. In order to protect against malicious or
malconfigured clients, if more than 8 KiB of HTTP header data is received then
malconfigured clients, if more than `maxHeaderSize` of HTTP header data is received then
HTTP parsing will abort without a request or response object being created, and
an `Error` with this code will be emitted.

Expand Down
Loading