Skip to content

fix: treat space as a delimiter in content-type parsing#6064

Merged
mcollina merged 2 commits intomainfrom
fix-content-media-type
Apr 18, 2025
Merged

fix: treat space as a delimiter in content-type parsing#6064
mcollina merged 2 commits intomainfrom
fix-content-media-type

Conversation

@mcollina
Copy link
Copy Markdown
Member

Follow-up from GHSA-mg2h-6x62-wpwc

@mcollina
Copy link
Copy Markdown
Member Author

Unfortunately, the fix from GHSA-mg2h-6x62-wpwc left a hole open. At this point, it's better to close the gap completely.

This is quite urgent.

@mcollina
Copy link
Copy Markdown
Member Author

Will do something slightly differently

@mcollina mcollina marked this pull request as draft April 18, 2025 16:55
@mcollina mcollina force-pushed the fix-content-media-type branch from 48f8dd1 to ee820b8 Compare April 18, 2025 17:04
@mcollina mcollina changed the title Return 415 on unknown media types fix: treat space as a delimiter in content-type parsing Apr 18, 2025
@mcollina mcollina marked this pull request as ready for review April 18, 2025 17:04
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina mcollina force-pushed the fix-content-media-type branch from ee820b8 to c7d6749 Compare April 18, 2025 17:13
Co-authored-by: Manuel Spigolon <manuel.spigolon@nearform.com>
Signed-off-by: Matteo Collina <matteo.collina@gmail.com>
@mcollina mcollina merged commit f3d2bcb into main Apr 18, 2025
38 of 41 checks passed
@mcollina mcollina deleted the fix-content-media-type branch April 18, 2025 20:23
function getEssenceMediaType (header) {
if (!header) return ''
return header.split(';', 1)[0].trim().toLowerCase()
return header.split(/[ ;]/, 1)[0].trim().toLowerCase()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We shouldn't ever see headers written like this (https://httpwg.org/specs/rfc9110.html#media.type). The spec is <type>[;][<parameter>]. But I suppose we have to handle this case due to jerks. However, it feel like a slippery slope to me.

Anyway, I think we might be able to eek out some more performance with slice instead:

'use strict'

const iterations = 1_000_000
const hrtime = process.hrtime.bigint
const fixture = 'application/json foo; charset=utf-8'

const startSplit = hrtime()
for (var i = 0; i < iterations; i += 1) {
  const result = fixture.split(/[ ;]/, 1)[0].trim().toLowerCase()
  doSomething(result)
}
const endSplit = hrtime()

const startSlice = hrtime()
for (var i = 0; i < iterations; i += 1) {
  const result = fixture.slice(
    0, fixture.indexOf(' ')
  ).replace(';', '').trim().toLowerCase()
  doSomething(result)
}
const endSlice = hrtime()

const splitTime = endSplit - startSplit
console.log('split:', splitTime)

const sliceTime = endSlice - startSlice
console.log('slice:', sliceTime)

console.log('split > slice =', splitTime > sliceTime)

function doSomething(value) {
  return value >> 1
}

Eomm added a commit that referenced this pull request Apr 28, 2025
* fix: treat space as a delimiter in content-type parsing

Signed-off-by: Matteo Collina <hello@matteocollina.com>

* Update lib/validation.js

Co-authored-by: Manuel Spigolon <manuel.spigolon@nearform.com>
Signed-off-by: Matteo Collina <matteo.collina@gmail.com>

---------

Signed-off-by: Matteo Collina <hello@matteocollina.com>
Signed-off-by: Matteo Collina <matteo.collina@gmail.com>
Co-authored-by: Manuel Spigolon <manuel.spigolon@nearform.com>
vaernion pushed a commit to Arbeidstilsynet/brevgen2 that referenced this pull request Dec 3, 2025
This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [fastify](https://fastify.dev/) ([source](https://github.com/fastify/fastify)) | [`5.2.2` -> `5.3.2`](https://renovatebot.com/diffs/npm/fastify/5.2.2/5.3.2) | [![age](https://developer.mend.io/api/mc/badges/age/npm/fastify/5.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/fastify/5.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/fastify/5.2.2/5.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/fastify/5.2.2/5.3.2?slim=true)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>fastify/fastify (fastify)</summary>

### [`v5.3.2`](https://github.com/fastify/fastify/releases/tag/v5.3.2)

[Compare Source](fastify/fastify@v5.3.1...v5.3.2)

#### ⚠️ Security Release ⚠️

Unfortunately, v5.3.1 did not include a complete fix for ["Invalid content-type parsing could lead to validation bypass"](GHSA-mg2h-6x62-wpwc) and CVE-2025-32442. This is a follow-up patch to cover an edge case.

#### What's Changed

-   docs: fix archived concurrently link to point to active repo by [@&#8203;TimTeylor](https://github.com/TimTeylor) in fastify/fastify#6063
-   fix: treat space as a delimiter in content-type parsing by [@&#8203;mcollina](https://github.com/mcollina) in fastify/fastify#6064

#### New Contributors

-   [@&#8203;TimTeylor](https://github.com/TimTeylor) made their first contribution in fastify/fastify#6063

**Full Changelog**: fastify/fastify@v5.3.1...v5.3.2

### [`v5.3.1`](https://github.com/fastify/fastify/releases/tag/v5.3.1)

[Compare Source](fastify/fastify@v5.3.0...v5.3.1)

#### ⚠️ Security Release ⚠️

-   Fix for ["Invalid content-type parsing could lead to validation bypass"](GHSA-mg2h-6x62-wpwc) and CVE-2025-32442

#### What's Changed

-   test: migrate logger options to node test runner by [@&#8203;ilteoood](https://github.com/ilteoood) in fastify/fastify#6059
-   test: migrate logger logging to node test runner by [@&#8203;ilteoood](https://github.com/ilteoood) in fastify/fastify#6060
-   test: convert custom parser 1 to node test runner by [@&#8203;ilteoood](https://github.com/ilteoood) in fastify/fastify#6053
-   test: custom querystring parser by [@&#8203;ilteoood](https://github.com/ilteoood) in fastify/fastify#6054
-   test: migrate stream 4 to node test runner by [@&#820...
@github-actions
Copy link
Copy Markdown

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 25, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants