Skip to content

fix: validate response headers before sending, prevent invalid transform input#921

Open
itslenny wants to merge 1 commit intomasterfrom
lenny/fix-header-validation-errors
Open

fix: validate response headers before sending, prevent invalid transform input#921
itslenny wants to merge 1 commit intomasterfrom
lenny/fix-header-validation-errors

Conversation

@itslenny
Copy link
Contributor

@itslenny itslenny commented Mar 19, 2026

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

  • transform query parameters on HEAD and info routes are not validated these values are passed to the x-transformation headers as-is allowing invalid input to be passed
  • if invalid characters are sent in response headers node http module throws ERR_INVALID_CHAR which is an uncaught exception resulting in a process crash

What is the new behavior?

Validate response headers before sending and throw an http error instead of allowing node to crash. This acts as a backstop for any invalid response header type errors.

Add missing query parameters validation for transform operations to prevent the invalid input from being set in the first place

Additional context

This issue could be triggered via format, resize, and quality query params on all of the following endpoints:

  • HEAD /object/public/bucket-name/file-name.jpg
  • HEAD /object/authenticated/bucket-name/file-name.jpg
  • HEAD/object/bucket-name/file-name.jpg
  • GET /object/info/public/bucket-name/file-name.jpg
  • GET /object/info/authenticated/bucket-name/file-name.jpg
  • GET /object/info/bucket-name/file-name.jpg

Without header validation any invalid chars in a header results in an uncaught exception

Screenshot 2026-03-19 at 11 06 53 AM

With header validation we handle the characters gracefully and log the offending header

Screenshot 2026-03-19 at 11 06 17 AM

With query param validation this is handled earlier by fastify validation (FST_ERR_VALIDATION)

Screenshot 2026-03-19 at 11 53 46 AM

@itslenny itslenny requested a review from a team as a code owner March 19, 2026 05:05
const headers = reply.getHeaders()

for (const [key, value] of Object.entries(headers)) {
if (typeof value === 'string' && INVALID_HEADER_CHAR_PATTERN.test(value)) {
Copy link
Member

Choose a reason for hiding this comment

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

reply.getHeaders() type includes string array.

I think we don't use but shall we add coverage since this is generic

)
app.register(plugins.tracing)
app.register(plugins.logRequest({ excludeUrls: excludedRoutesFromMonitoring }))
app.register(plugins.headerValidator)
Copy link
Member

Choose a reason for hiding this comment

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

Probably negligible and fine, I would use excludedRoutesFromMonitoring to skip this.

Aside we should probably switch it into a set that also contains / variants

* Invalid: control characters (0x00-0x1F except TAB) and DEL (0x7F).
* @see https://tools.ietf.org/html/rfc7230#section-3.2
*/
const INVALID_HEADER_CHAR_PATTERN = /[^\t\x20-\x7e\x80-\xff]/
Copy link
Member

Choose a reason for hiding this comment

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

Btw, we have this regex for s3

Copy link
Member

@ferhatelmas ferhatelmas left a comment

Choose a reason for hiding this comment

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

My main points are addressed. Others are nice to have so approving 👍🏻

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