Skip to content

Fetch: validating headers in Headers.set() - #1107

Closed
VadimZhestikov wants to merge 1 commit into
nginx:masterfrom
VadimZhestikov:fix/headers-set-validation
Closed

Fetch: validating headers in Headers.set()#1107
VadimZhestikov wants to merge 1 commit into
nginx:masterfrom
VadimZhestikov:fix/headers-set-validation

Conversation

@VadimZhestikov

Copy link
Copy Markdown
Contributor

Proposed changes

Headers.set() validates its arguments only when the header does not already
exist. In that case it falls through to ngx_js_headers_append(), which trims
optional whitespace and then checks the name with ngx_js_check_header_name(),
the value with ngx_js_check_header_value(), and the immutable guard.

When the header is already present, set() takes a different path: it finds the
existing entry, overwrites h[i].value in place and unlinks the duplicates, so
none of those checks run. As a result a value that append() rejects is
accepted by set():

var h = new Headers();
h.append('X-A', 'good');
h.append('X-B', 'a\r\nInjected: 1');   // TypeError: invalid header value
h.set('X-A', 'b\r\nInjected: 2');      // accepted

The immutable guard is skipped on the same path, so an existing header on an
immutable Headers object can be overwritten.

This moves the checks ahead of the lookup so both paths behave identically. Both
engines have the same structure and both are changed.

After the change the example above throws for set() as well, set() with an
ordinary value still works, and optional whitespace is trimmed as it is by
append().

Checklist

Before creating a PR, run through this checklist and mark each as complete:

  • I have read the CONTRIBUTING document
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have checked that any relevant tests pass after adding my changes

Headers.set() validated its arguments only when the header did not
already exist, in which case it fell through to ngx_js_headers_append().
When the header was present its value was overwritten in place, so the
name and value checks and the immutable guard were all skipped.

Now the same checks are done before the lookup, in both engines.
@xeioex

xeioex commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

See #1108. I do not amend it here because no original changes left.

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

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants