Skip to content

Commit 2fecd02

Browse files
fix: reject multiple values for singleton response headers (#2012)
## Summary - reject empty or multiple-value arrays; normalize one-item arrays. [edited by @yowainwright] - apply the validation consistently to both `ctx.set(name, value)` and `ctx.set(headers)` - add regression coverage for both call forms ## Test plan - `npm test` - `npm run lint` - `git diff --check` Related to #1973 ## Summary by Sourcery Enforce single-value response headers consistently while retaining valid multi-valued header support. Bug Fixes: - Reject multiple or empty values for singleton response headers while preserving support for multi-valued headers such as Set-Cookie. - Apply singleton-header validation consistently when setting individual headers and header maps, including error responses. Enhancements: - Normalize single-element Content-Type and Content-Length arrays to scalar header values and validate header maps before making assignments. Tests: - Add regression coverage for singleton-header validation, normalization, atomic header assignment, single-read field access, and error-response headers. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Setting multiple values for `Content-Type` or `Content-Length` now throws an error, whether headers are set individually or together. Error messages preserve the supplied header-name casing. * When setting headers together, an invalid value prevents earlier headers in the same assignment from being set. * Single-value arrays are accepted for these headers, and each object-form header value is read once. * Error responses continue to apply other supplied headers, including cookies, while ignoring supplied `Content-Type` and `Content-Length` values. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: shuaiyafei <16618929+shuaiyafei@users.noreply.github.com> Co-authored-by: Jeff Wainwright <1074042+yowainwright@users.noreply.github.com>
1 parent 824c1cf commit 2fecd02

4 files changed

Lines changed: 102 additions & 10 deletions

File tree

‎__tests__/context/onerror.test.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,19 +56,25 @@ describe('ctx.onerror(err)', () => {
5656
status: 418,
5757
expose: true,
5858
headers: {
59+
'Content-Length': ['1', '2'],
60+
'cOnTeNt-TyPe': ['text/plain', 'application/json'],
61+
'Set-Cookie': ['first=1', 'second=2'],
5962
'X-New-Header': 'Value'
6063
}
6164
})
6265
})
6366

6467
const res = await request(app.callback())
6568
.get('/')
66-
.expect(418)
69+
.timeout({ deadline: 1000 })
70+
.expect(418, 'boom')
6771
.expect('Content-Type', 'text/plain; charset=utf-8')
72+
.expect('Content-Length', '4')
6873
.expect('X-New-Header', 'Value')
6974

7075
assert.strictEqual(Object.prototype.hasOwnProperty.call(res.headers, 'vary'), false)
7176
assert.strictEqual(Object.prototype.hasOwnProperty.call(res.headers, 'x-csrf-token'), false)
77+
assert.deepStrictEqual(res.headers['set-cookie'], ['first=1', 'second=2'])
7278
})
7379

7480
it('should ignore error after headerSent', async () => {

‎__tests__/response/set.test.js‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,73 @@ describe('ctx.set(object)', () => {
4242
assert.strictEqual(ctx.response.header.foo, '1')
4343
assert.strictEqual(ctx.response.header.bar, '2')
4444
})
45+
46+
it('should not assign multiple values to singleton headers', () => {
47+
const ctx = context()
48+
49+
assert.throws(() => {
50+
ctx.set('content-length', ['1', '2'])
51+
}, /content-length must contain exactly one value/)
52+
53+
assert.throws(() => {
54+
ctx.set({ 'Content-Length': ['1', '2'] })
55+
}, /Content-Length must contain exactly one value/)
56+
57+
assert.throws(() => {
58+
ctx.set({ 'cOnTeNt-TyPe': ['text/plain', 'application/json'] })
59+
}, /cOnTeNt-TyPe must contain exactly one value/)
60+
})
61+
62+
it('should normalize singleton header arrays', () => {
63+
const ctx = context()
64+
65+
ctx.set('Content-Length', ['1'])
66+
assert.strictEqual(ctx.response.get('Content-Length'), '1')
67+
68+
ctx.set({ 'Content-Type': ['application/json'] })
69+
ctx.body = { ok: true }
70+
assert.strictEqual(ctx.response.get('Content-Type'), 'application/json')
71+
72+
ctx.set('Content-Type', ['application/json'])
73+
ctx.body = { ok: true }
74+
assert.strictEqual(ctx.response.get('Content-Type'), 'application/json')
75+
})
76+
77+
it('should reject empty arrays for singleton headers', () => {
78+
const ctx = context()
79+
80+
assert.throws(() => ctx.set('Content-Type', []), /must contain exactly one value/)
81+
assert.throws(() => ctx.set('Content-Length', []), /must contain exactly one value/)
82+
assert.throws(() => ctx.set({ 'Content-Type': [] }), /must contain exactly one value/)
83+
assert.throws(() => ctx.set({ 'Content-Length': [] }), /must contain exactly one value/)
84+
})
85+
86+
it('should validate every field before assigning any values', () => {
87+
const ctx = context()
88+
89+
assert.throws(() => {
90+
ctx.set({
91+
'X-Foo': 'bar',
92+
'Content-Length': ['1', '2']
93+
})
94+
}, /Content-Length must contain exactly one value/)
95+
96+
assert.strictEqual(ctx.response.header['x-foo'], undefined)
97+
})
98+
99+
it('should read each field value once', () => {
100+
const ctx = context()
101+
let reads = 0
102+
const fields = {
103+
get 'Content-Length' () {
104+
reads++
105+
return reads === 1 ? '1' : ['1', '2']
106+
}
107+
}
108+
109+
ctx.set(fields)
110+
111+
assert.strictEqual(reads, 1)
112+
assert.strictEqual(ctx.response.header['content-length'], '1')
113+
})
45114
})

‎lib/context.js‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,13 @@ const proto = module.exports = {
143143
}
144144

145145
// then set those specified
146-
this.set(err.headers)
146+
const headers = err.headers || {}
147+
Object.keys(headers).forEach(name => {
148+
const lowerName = name.toLowerCase()
149+
const generatedHeader = lowerName === 'content-type' || lowerName === 'content-length'
150+
if (generatedHeader) return
151+
this.set(name, headers[name])
152+
})
147153

148154
// force text/plain
149155
this.type = 'text'

‎lib/response.js‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,19 @@ const getType = require('mime-types').contentType
2121
const isStream = require('./is-stream.js')
2222
const only = require('./only.js')
2323

24+
function isSingleValueHeader (field) {
25+
const name = field.toLowerCase()
26+
return name === 'content-type' || name === 'content-length'
27+
}
28+
29+
function normalizeHeader (field, value) {
30+
const singletonArray = isSingleValueHeader(field) && Array.isArray(value)
31+
if (!singletonArray) return [field, value]
32+
33+
assert(value.length === 1, `${field} must contain exactly one value`)
34+
return [field, value[0]]
35+
}
36+
2437
/**
2538
* Prototype.
2639
*/
@@ -537,14 +550,12 @@ module.exports = {
537550
set (field, val) {
538551
if (this.headerSent || !field) return
539552

540-
if (typeof field === 'string') {
541-
if (field.toLowerCase() === 'content-type') {
542-
assert(!Array.isArray(val), 'Assign multiple Content-Type for response header is not allowed')
543-
}
544-
this.res.setHeader(field, val)
545-
} else {
546-
Object.keys(field).forEach(header => this.res.setHeader(header, field[header]))
547-
}
553+
const entries = typeof field === 'string' ? [[field, val]] : Object.entries(field)
554+
const headers = entries.map(([name, value]) => normalizeHeader(name, value))
555+
556+
headers.forEach(([name, value]) => {
557+
this.res.setHeader(name, value)
558+
})
548559
},
549560

550561
/**

0 commit comments

Comments
 (0)