Skip to content

Sanitize HTTP header values to strip CR/LF (RFC 7230 §3.2.4)#38744

Open
Copilot wants to merge 3 commits into
mainfrom
copilot/sanitize-header-values
Open

Sanitize HTTP header values to strip CR/LF (RFC 7230 §3.2.4)#38744
Copilot wants to merge 3 commits into
mainfrom
copilot/sanitize-header-values

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 29, 2026

Packages impacted by this PR

  • @typespec/ts-http-runtime

Issues associated with this PR

Describe the problem that is addressed by this PR

HttpHeadersImpl.set() stored header values via String(value).trim(), which trims surrounding whitespace but does not remove illegal CR (\r) / LF (\n) characters. Per RFC 7230 §3.2.4, senders MUST NOT generate field-values containing obs-fold (line folding) sequences; leaving these in place allows accidental multi-line values to produce malformed HTTP messages.

What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?

The issue suggested either stripping the offending characters silently or throwing on invalid input. Silent stripping was chosen for consistency with the existing whitespace trim() behavior and to avoid introducing a breaking change for callers.

  • Added a normalizeValue helper that trims whitespace and removes \r/\n via .replace(/[\r\n]/g, "").
  • HttpHeadersImpl.set() now routes values through normalizeValue. The constructor delegates to set(), so all entry points are covered.
  • @azure/core-rest-pipeline's createHttpHeaders delegates to @typespec/ts-http-runtime, so it inherits this behavior without separate changes.
createHttpHeaders({ "x-custom": "value1\r\ninjected: bad" }).get("x-custom");
// => "value1injected: bad"

Are there test cases added in this PR? (If not, why?)

Yes. Added a test in httpHeaders.spec.ts covering CR, LF, CRLF, and trailing CRLF stripping.

Provide a list of related PRs (if any)

Command used to generate this PR:**(Applicable only to SDK release request PRs)

Checklists

  • Added impacted package name to the issue description
  • Does this PR needs any fixes in the SDK Generator?** (If so, create an Issue in the Autorest/typescript repository and link it here)
  • Added a changelog (if necessary)

Copilot AI and others added 2 commits May 29, 2026 19:24
Co-authored-by: jeremymeng <7583839+jeremymeng@users.noreply.github.com>
Co-authored-by: jeremymeng <7583839+jeremymeng@users.noreply.github.com>
Copilot AI changed the title [WIP] Sanitize header values in createHttpHeaders function Sanitize HTTP header values to strip CR/LF (RFC 7230 §3.2.4) May 29, 2026
Copilot AI requested a review from jeremymeng May 29, 2026 19:26
@jeremymeng jeremymeng marked this pull request as ready for review May 29, 2026 21:55
@jeremymeng jeremymeng requested a review from a team as a code owner May 29, 2026 21:55
Copilot AI review requested due to automatic review settings May 29, 2026 21:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Sanitizes HTTP header values in @typespec/ts-http-runtime's HttpHeadersImpl to strip CR/LF characters, preventing potential header injection / obs-fold sequences forbidden by RFC 7230 §3.2.4.

Changes:

  • Add normalizeValue helper that trims whitespace and removes \r/\n characters, used by HttpHeadersImpl.set().
  • Add a vitest case covering CR, LF, CRLF, and trailing CRLF stripping.
  • Document the fix in the 0.3.6 (Unreleased) changelog entry.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
sdk/core/ts-http-runtime/src/httpHeaders.ts Introduces normalizeValue and routes set() through it to strip CR/LF.
sdk/core/ts-http-runtime/test/public/httpHeaders.spec.ts Adds a test verifying CR/LF/CRLF stripping behavior.
sdk/core/ts-http-runtime/CHANGELOG.md Adds a Bugs Fixed entry describing the sanitization behavior.

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.

HttpHeadersImpl should sanitize header values to prevent obs-fold sequences (RFC 7230 §3.2.4)

3 participants