Skip to content

Commit 397bc13

Browse files
authored
fix(retry-handler): validate response body length against Content-Range (#4975)
1 parent 204740c commit 397bc13

2 files changed

Lines changed: 81 additions & 0 deletions

File tree

lib/handler/retry-handler.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ class RetryHandler {
6868
this.start = 0
6969
this.end = null
7070
this.etag = null
71+
this.statusCode = null
72+
this.headers = null
7173
}
7274

7375
onResponseStartWithRetry (controller, statusCode, headers, statusMessage, err) {
@@ -183,6 +185,8 @@ class RetryHandler {
183185
onResponseStart (controller, statusCode, headers, statusMessage) {
184186
this.error = null
185187
this.retryCount += 1
188+
this.statusCode = statusCode
189+
this.headers = headers
186190

187191
if (statusCode >= 300) {
188192
const err = new RequestRetryError('Request failed', statusCode, {
@@ -320,6 +324,16 @@ class RetryHandler {
320324
}
321325

322326
if (!this.error) {
327+
// Verify that the received body length matches the expected range
328+
// when we have a finite end position (from Content-Length or Content-Range)
329+
if (this.end != null && Number.isFinite(this.end)) {
330+
if (this.start !== this.end + 1) {
331+
throw new RequestRetryError('Content-Range mismatch', this.statusCode, {
332+
headers: this.headers,
333+
data: { count: this.retryCount }
334+
})
335+
}
336+
}
323337
this.retryCount = 0
324338
return this.handler.onResponseEnd?.(controller, trailers)
325339
}

test/interceptors/retry.js

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,73 @@ test('Should handle 206 partial content - bad-etag', async t => {
458458
}
459459
})
460460

461+
test('#4970 - Should reject resumed partial content when body exceeds Content-Range', async t => {
462+
t = tspl(t, { plan: 5 })
463+
464+
let x = 0
465+
const injectedResponse = 'HTTP/1.1 302 Found\r\nLocation: http://evil.com\r\nContent-Length: 0\r\n\r\n'
466+
const server = createServer({ joinDuplicateHeaders: true }, (req, res) => {
467+
if (x === 0) {
468+
t.ok(true, 'pass')
469+
res.setHeader('content-length', '5')
470+
res.setHeader('etag', '123')
471+
res.write('use')
472+
setTimeout(() => {
473+
res.destroy()
474+
}, 1e2)
475+
} else if (x === 1) {
476+
t.deepStrictEqual(req.headers.range, 'bytes=3-4')
477+
t.deepStrictEqual(req.headers['if-match'], '123')
478+
res.statusCode = 206
479+
res.setHeader('etag', '123')
480+
res.setHeader('content-range', 'bytes 3-4/5')
481+
res.end(`r1${injectedResponse}`)
482+
}
483+
x++
484+
})
485+
486+
const requestOptions = {
487+
method: 'GET',
488+
path: '/',
489+
headers: {
490+
'content-type': 'application/json'
491+
},
492+
retryOptions: {
493+
retry: (err, { state, opts }, done) => {
494+
if (err.message.includes('other side closed')) {
495+
setTimeout(done, 100)
496+
return
497+
}
498+
499+
return done(err)
500+
}
501+
}
502+
}
503+
504+
server.listen(0)
505+
506+
await once(server, 'listening')
507+
508+
const client = new Client(
509+
`http://localhost:${server.address().port}`
510+
).compose(retry())
511+
512+
after(async () => {
513+
await client.close()
514+
server.close()
515+
516+
await once(server, 'close')
517+
})
518+
519+
const response = await client.request(requestOptions)
520+
t.strictEqual(response.statusCode, 200)
521+
await t.rejects(response.body.text(), {
522+
name: 'RequestRetryError',
523+
code: 'UND_ERR_REQ_RETRY',
524+
message: 'Content-Range mismatch'
525+
})
526+
})
527+
461528
test('retrying a request with a body', async t => {
462529
t = tspl(t, { plan: 2 })
463530
let counter = 0

0 commit comments

Comments
 (0)