Skip to content

Fix duplicate Content-Type on requests with a body - #71

Open
pyrogenic wants to merge 3 commits into
aknorw:masterfrom
pyrogenic:fix/request-headers
Open

pyrogenic wants to merge 3 commits into
aknorw:masterfrom
pyrogenic:fix/request-headers

Conversation

@pyrogenic

Copy link
Copy Markdown

(Part 2 answers your review question about the Buffer guard from #60.)

1. Duplicate "Content-Type"

In Fetcher.schedule:

const clonedHeaders = new Map(this.headers)

if (data) {
  clonedHeaders.set('Content-Type', 'application/json')
  clonedHeaders.set('Content-Length', Buffer.byteLength(stringifiedData, 'utf8').toString())
}

options.headers = Object.fromEntries(clonedHeaders)

this.headers is a Headers, and Headers lower-cases the names it yields when iterated, so the Map already has a content-type. Setting 'Content-Type' on a Map adds a second entry under a different key. Object.fromEntries keeps both, and fetch then joins them:

> const h = new Headers({ Accept: 'a', 'Content-Type': 'application/json' })
> const m = new Map(h)
[ [ 'accept', 'a' ], [ 'content-type', 'application/json' ] ]   // already lower-case

> m.set('Content-Type', 'application/json')
> Object.fromEntries(m)
{ accept: 'a', 'content-type': 'application/json', 'Content-Type': 'application/json' }

> new Headers(Object.fromEntries(m)).get('content-type')
'application/json, application/json'

This would go out on every request that carries a body (every POST and PUT). Some servers reject it outright; it's the same shape as this FastAPI report.

The fix is to set both in lower case so they replace the entries already in the map.

Tests: src/utils/fetch.headers.spec.ts mocks cross-fetch and asserts (a) no header name is
sent twice under two casings, and (b) content-type is exactly application/json. Both fail on
master and pass with the fix, at each commit individually. They take Headers from cross-fetch
rather than the global so they work on older jest environments too.

2. Content-Length and Buffer

Buffer is Node-only. In a browser bundle it's undefined, so referencing it
throws a ReferenceError on every request with a body. That's the other thing that stops a
bundled Discojs working in a browser after allowUnsafeHeaders: false.

Content-Length is a
forbidden header name (the user agent computes it on its own), so guarding that set on typeof Buffer !== 'undefined' lets the code work in a browser, which is now explained in the code.

Tested running in browsers (Chrome + iOS Safari) against the live API with allowUnsafeHeaders: false running POST routes (note edits and listing updates).

`clonedHeaders` is a `Map` built by iterating `this.headers`, and `Headers`
lower-cases the names it yields — so the clone is already keyed `content-type`.
Setting `Content-Type` on it adds a *second* entry rather than replacing the
first, `Object.fromEntries` keeps both, and fetch joins them back together:

    content-type: application/json, application/json

Every request with a body — every POST/PUT in the library — goes out that way.
Discogs rejects some of them, and the same shape breaks other servers too
(fastapi/fastapi#6682 (comment)).
`Content-Length` was duplicated the same way, but its value happens to be
identical to the one the runtime computes, so it was harmless.

Set both in lower case so they replace the entries already in the map.
`Buffer` is Node-only. In a browser bundle the identifier is not defined, so
referencing it throws a `ReferenceError` on every request that carries a body —
which is the remaining thing that stops a bundled Discojs from working in a
browser after `allowUnsafeHeaders: false`.

Setting it there would be pointless anyway: `Content-Length` is a forbidden
header name in the fetch spec, so the user agent computes it and drops whatever
the caller supplied.

Guard on `typeof Buffer` so the header is only set on Node.
@pyrogenic
pyrogenic marked this pull request as draft August 23, 2026 00:49
@pyrogenic
pyrogenic marked this pull request as ready for review August 23, 2026 02:58

This branch has not been deployed

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

Labels

None yet

1 participant