diff --git a/lib/handler/retry-handler.js b/lib/handler/retry-handler.js index 2258801ba58..56ea4be79be 100644 --- a/lib/handler/retry-handler.js +++ b/lib/handler/retry-handler.js @@ -7,9 +7,7 @@ const { isDisturbed, parseHeaders, parseRangeHeader } = require('../core/util') function calculateRetryAfterHeader (retryAfter) { const current = Date.now() - const diff = new Date(retryAfter).getTime() - current - - return diff + return new Date(retryAfter).getTime() - current } class RetryHandler { @@ -116,11 +114,7 @@ class RetryHandler { const { counter } = state // Any code that is not a Undici's originated and allowed to retry - if ( - code && - code !== 'UND_ERR_REQ_RETRY' && - !errorCodes.includes(code) - ) { + if (code && code !== 'UND_ERR_REQ_RETRY' && !errorCodes.includes(code)) { cb(err) return } @@ -246,10 +240,7 @@ class RetryHandler { start != null && Number.isFinite(start), 'content-range mismatch' ) - assert( - end != null && Number.isFinite(end), - 'invalid content-length' - ) + assert(end != null && Number.isFinite(end), 'invalid content-length') this.start = start this.end = end @@ -270,6 +261,13 @@ class RetryHandler { this.resume = resume this.etag = headers.etag != null ? headers.etag : null + // Weak etags are not useful for comparison nor cache + // for instance not safe to assume if the response is byte-per-byte + // equal + if (this.etag != null && this.etag.startsWith('W/')) { + this.etag = null + } + return this.handler.onHeaders( statusCode, rawHeaders, @@ -308,7 +306,9 @@ class RetryHandler { // and server error response if (this.retryCount - this.retryCountCheckpoint > 0) { // We count the difference between the last checkpoint and the current retry count - this.retryCount = this.retryCountCheckpoint + (this.retryCount - this.retryCountCheckpoint) + this.retryCount = + this.retryCountCheckpoint + + (this.retryCount - this.retryCountCheckpoint) } else { this.retryCount += 1 } @@ -328,11 +328,18 @@ class RetryHandler { } if (this.start !== 0) { + const headers = { range: `bytes=${this.start}-${this.end ?? ''}` } + + // Weak etag check - weak etags will make comparison algorithms never match + if (this.etag != null) { + headers['if-match'] = this.etag + } + this.opts = { ...this.opts, headers: { ...this.opts.headers, - range: `bytes=${this.start}-${this.end ?? ''}` + ...headers } } } diff --git a/test/retry-handler.js b/test/retry-handler.js index 2596a7d80cb..8894ea86668 100644 --- a/test/retry-handler.js +++ b/test/retry-handler.js @@ -979,3 +979,321 @@ test('Issue#2986 - Handle custom 206', async t => { await t.completed }) + +test('Issue#3128 - Support if-match', async t => { + t = tspl(t, { plan: 9 }) + + const chunks = [] + let counter = 0 + + // Took from: https://github.com/nxtedition/nxt-lib/blob/4b001ebc2f22cf735a398f35ff800dd553fe5933/test/undici/retry.js#L47 + let x = 0 + const server = createServer((req, res) => { + if (x === 0) { + t.deepStrictEqual(req.headers.range, 'bytes=0-3') + res.setHeader('etag', 'asd') + res.write('abc') + setTimeout(() => { + res.destroy() + }, 1e2) + } else if (x === 1) { + t.deepStrictEqual(req.headers.range, 'bytes=3-') + t.deepStrictEqual(req.headers['if-match'], 'asd') + + res.setHeader('content-range', 'bytes 3-6/6') + res.setHeader('etag', 'asd') + res.statusCode = 206 + res.end('def') + } + x++ + }) + + const dispatchOptions = { + retryOptions: { + retry: function (err, _, done) { + counter++ + + if (err.code && err.code === 'UND_ERR_DESTROYED') { + return done(false) + } + + if (err.statusCode === 206) return done(err) + + setTimeout(done, 800) + } + }, + method: 'GET', + path: '/', + headers: { + 'content-type': 'application/json' + } + } + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`) + const handler = new RetryHandler(dispatchOptions, { + dispatch: (...args) => { + return client.dispatch(...args) + }, + handler: { + onRequestSent () { + t.ok(true, 'pass') + }, + onConnect () { + t.ok(true, 'pass') + }, + onBodySent () { + t.ok(true, 'pass') + }, + onHeaders (status, _rawHeaders, resume, _statusMessage) { + t.strictEqual(status, 200) + return true + }, + onData (chunk) { + chunks.push(chunk) + return true + }, + onComplete () { + t.strictEqual(Buffer.concat(chunks).toString('utf-8'), 'abcdef') + t.strictEqual(counter, 1) + }, + onError () { + t.fail() + } + } + }) + + client.dispatch( + { + method: 'GET', + path: '/', + headers: { + 'content-type': 'application/json', + Range: 'bytes=0-3' + } + }, + handler + ) + + after(async () => { + await client.close() + + server.close() + await once(server, 'close') + }) + }) + + await t.completed +}) + +test('Issue#3128 - Should ignore weak etags', async t => { + t = tspl(t, { plan: 9 }) + + const chunks = [] + let counter = 0 + + // Took from: https://github.com/nxtedition/nxt-lib/blob/4b001ebc2f22cf735a398f35ff800dd553fe5933/test/undici/retry.js#L47 + let x = 0 + const server = createServer((req, res) => { + if (x === 0) { + t.deepStrictEqual(req.headers.range, 'bytes=0-3') + res.setHeader('etag', 'W/asd') + res.write('abc') + setTimeout(() => { + res.destroy() + }, 1e2) + } else if (x === 1) { + t.deepStrictEqual(req.headers.range, 'bytes=3-') + t.equal(req.headers['if-match'], undefined) + + res.setHeader('content-range', 'bytes 3-6/6') + res.setHeader('etag', 'W/asd') + res.statusCode = 206 + res.end('def') + } + x++ + }) + + const dispatchOptions = { + retryOptions: { + retry: function (err, _, done) { + counter++ + + if (err.code && err.code === 'UND_ERR_DESTROYED') { + return done(false) + } + + if (err.statusCode === 206) return done(err) + + setTimeout(done, 800) + } + }, + method: 'GET', + path: '/', + headers: { + 'content-type': 'application/json' + } + } + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`) + const handler = new RetryHandler(dispatchOptions, { + dispatch: (...args) => { + return client.dispatch(...args) + }, + handler: { + onRequestSent () { + t.ok(true, 'pass') + }, + onConnect () { + t.ok(true, 'pass') + }, + onBodySent () { + t.ok(true, 'pass') + }, + onHeaders (status, _rawHeaders, resume, _statusMessage) { + t.strictEqual(status, 200) + return true + }, + onData (chunk) { + chunks.push(chunk) + return true + }, + onComplete () { + t.strictEqual(Buffer.concat(chunks).toString('utf-8'), 'abcdef') + t.strictEqual(counter, 1) + }, + onError () { + t.fail() + } + } + }) + + client.dispatch( + { + method: 'GET', + path: '/', + headers: { + 'content-type': 'application/json', + Range: 'bytes=0-3' + } + }, + handler + ) + + after(async () => { + await client.close() + + server.close() + await once(server, 'close') + }) + }) + + await t.completed +}) + +test('Weak etags are ignored on range-requests', async t => { + t = tspl(t, { plan: 9 }) + + const chunks = [] + let counter = 0 + + // Took from: https://github.com/nxtedition/nxt-lib/blob/4b001ebc2f22cf735a398f35ff800dd553fe5933/test/undici/retry.js#L47 + let x = 0 + const server = createServer((req, res) => { + if (x === 0) { + t.deepStrictEqual(req.headers.range, 'bytes=0-3') + res.setHeader('etag', 'W/asd') + res.write('abc') + setTimeout(() => { + res.destroy() + }, 1e2) + } else if (x === 1) { + t.deepStrictEqual(req.headers.range, 'bytes=3-') + t.equal(req.headers['if-match'], undefined) + + res.setHeader('content-range', 'bytes 3-6/6') + res.setHeader('etag', 'W/efg') + res.statusCode = 206 + res.end('def') + } + x++ + }) + + const dispatchOptions = { + retryOptions: { + retry: function (err, _, done) { + counter++ + + if (err.code && err.code === 'UND_ERR_DESTROYED') { + return done(false) + } + + if (err.statusCode === 206) return done(err) + + setTimeout(done, 800) + } + }, + method: 'GET', + path: '/', + headers: { + 'content-type': 'application/json' + } + } + + server.listen(0, () => { + const client = new Client(`http://localhost:${server.address().port}`) + const handler = new RetryHandler(dispatchOptions, { + dispatch: (...args) => { + return client.dispatch(...args) + }, + handler: { + onRequestSent () { + t.ok(true, 'pass') + }, + onConnect () { + t.ok(true, 'pass') + }, + onBodySent () { + t.ok(true, 'pass') + }, + onHeaders (status, _rawHeaders, resume, _statusMessage) { + t.strictEqual(status, 200) + return true + }, + onData (chunk) { + chunks.push(chunk) + return true + }, + onComplete () { + t.strictEqual(Buffer.concat(chunks).toString('utf-8'), 'abcdef') + t.strictEqual(counter, 1) + }, + onError () { + t.fail() + } + } + }) + + client.dispatch( + { + method: 'GET', + path: '/', + headers: { + 'content-type': 'application/json', + Range: 'bytes=0-3' + } + }, + handler + ) + + after(async () => { + await client.close() + + server.close() + await once(server, 'close') + }) + }) + + await t.completed +})