Skip to content

Comments

Ignore illegal response header (#2439)#5

Open
MitchLewis930 wants to merge 1 commit intopr_055_beforefrom
pr_055_after
Open

Ignore illegal response header (#2439)#5
MitchLewis930 wants to merge 1 commit intopr_055_beforefrom
pr_055_after

Conversation

@MitchLewis930
Copy link

@MitchLewis930 MitchLewis930 commented Jan 30, 2026

PR_055


Note

Medium Risk
Touches core response header serialization and filtering; mistakes could drop legitimate headers or affect interoperability, though the change is narrowly scoped and well-covered by tests.

Overview
Response header handling is tightened to ignore illegal headers per Rack spec. Response header keys are now validated against an RFC token-style character set, header values are rejected if they contain control characters, and both checks are applied to normal responses and 103 Early Hints.

Additionally, response headers with banned keys (e.g. Status and rack.*) are now dropped instead of being sent to clients. Tests previously marked skip are enabled/added to cover these cases, and History.md notes the bugfix.

Written by Cursor Bugbot for commit 31c72cf. This will update automatically on new commits. Configure here.

* Ignore illegal response header
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 3 potential issues.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.

# Illegal character in the key or value of response header
DQUOTE = "\"".freeze
HTTP_HEADER_DELIMITER = Regexp.escape("(),/:;<=>?@[]{}").freeze
ILLEGAL_HEADER_KEY_REGEX = /[\u0000-\u0025|#{DQUOTE}|#{HTTP_HEADER_DELIMITER}]/.freeze
Copy link

Choose a reason for hiding this comment

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

Regex blocks valid HTTP header token characters

Medium Severity

The ILLEGAL_HEADER_KEY_REGEX uses range \u0000-\u0025 which blocks characters 0-37 decimal. This incorrectly blocks valid RFC 7230 token characters ! (33), # (35), $ (36), and % (37). Per RFC 7230, these are explicitly valid tchar characters. Valid response headers containing these characters would be incorrectly filtered out. The range likely intended to be \u0000-\u0020 (control chars + space) with DQUOTE added separately.

Fix in Cursor Fix in Web

ILLEGAL_HEADER_VALUE_REGEX = /[\000-\037]/.freeze

# Banned keys of response header
BANNED_HEADER_KEY = /rack.|status/.freeze
Copy link

Choose a reason for hiding this comment

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

Banned header regex matches unintended header names

Medium Severity

The BANNED_HEADER_KEY regex /rack.|status/ has two issues: the . after rack is unescaped and matches any character (so "racka", "rack1" would match), and status lacks anchors so it matches substrings (so "x-status-code", "mystatus" would incorrectly match). The regex likely intended /^rack\.|^status$/i to match headers starting with "rack." or exactly equal to "status".

Fix in Cursor Fix in Web

# Illegal character in the key or value of response header
DQUOTE = "\"".freeze
HTTP_HEADER_DELIMITER = Regexp.escape("(),/:;<=>?@[]{}").freeze
ILLEGAL_HEADER_KEY_REGEX = /[\u0000-\u0025|#{DQUOTE}|#{HTTP_HEADER_DELIMITER}]/.freeze
Copy link

Choose a reason for hiding this comment

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

Regex incorrectly blocks valid pipe character in headers

Medium Severity

The ILLEGAL_HEADER_KEY_REGEX contains literal | characters inside the character class brackets, which are treated as literal matches rather than alternation operators. This causes headers containing | in their names to be incorrectly filtered. Per RFC 7230, | is a valid tchar character allowed in HTTP header tokens. The author likely intended alternation but | inside [] is literal in regex.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants