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

Allow some control characters in a request target #5845

Merged
merged 2 commits into from
Aug 2, 2024

Conversation

trustin
Copy link
Member

@trustin trustin commented Jul 30, 2024

Motivation:

RFC 3986 doesn't prohibit control characters in a request target as long as it's percent-encoded. However, DefaultRequestTarget doesn't allow any control characters except \r and \n in a query string.

This is to reject potentially harmful paths such as /foo\nbar but it can be overly strict to some users.

Modifications:

  • DefaultRequestTarget now allows the following control characters additionally:
    • TAB (0x09)
    • FS (0x1C)
    • GS (0x1D)
    • RS (0x1E)
    • US (0x1F)
  • However, other control characters will remain prohibited until there's a good reason to allow them.
  • Simplified appendOneByte() by introducing additional BitSets of percent-encodable characters.

Result:

  • A user can send and receive an HTTP request whose :path contains the following characters now:
    • TAB (0x09)
    • FS (0x1C)
    • GS (0x1D)
    • RS (0x1E)
    • US (0x1F)

Motivation:

RFC 3986 doesn't prohibit control characters in a request target as long
as it's percent-encoded. However, `DefaultRequestTarget` doesn't allow
any control characters except `\r` and `\n` in a query string.

This is to reject potentially harmful paths such as `/foo\nbar` but it
can be overly strict to some users.

Modifications:

- `DefaultRequestTarget` now allows the following control characters
  additionally:
  - TAB (0x09)
  - FS (0x1C)
  - GS (0x1D)
  - RS (0x1E)
  - US (0x1F)
- However, other control characters will remain prohibited until there's
  a good reason to allow them.
- Simplified `appendOneByte()` by introducing additional `BitSet`s of
  percent-encodable characters.

Result:

- A user can send and receive an HTTP request whose `:path` contains the
  following characters now:
  - TAB (0x09)
  - FS (0x1C)
  - GS (0x1D)
  - RS (0x1E)
  - US (0x1F)
@trustin trustin added the defect label Jul 30, 2024
@trustin trustin added this to the 1.29.4 milestone Jul 30, 2024
@trustin trustin modified the milestones: 1.29.4, 1.30.0 Jul 30, 2024
Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

Thanks! 🙇‍♂️🙇‍♂️

Copy link
Contributor

@jrhee17 jrhee17 left a comment

Choose a reason for hiding this comment

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

👍 👍 👍

Copy link
Contributor

@minwoox minwoox left a comment

Choose a reason for hiding this comment

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

👍 👍 👍

@trustin trustin merged commit 9f04c8b into line:main Aug 2, 2024
14 of 15 checks passed
@trustin trustin deleted the allow_some_control_characters branch August 2, 2024 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants