refactor: drop support for weak etags

Author: metcoder95

lib/handler/retry-handler.js

@@ -64,7 +64,6 @@ class RetryHandler {
     this.start = 0
     this.end = null
     this.etag = null
-    this.isWeakEtag = false
     this.resume = null
 
     // Handle possible onConnect duplication
@@ -205,7 +204,7 @@ class RetryHandler {
       }
 
       // Let's start with a weak etag check
-      if (this.etag != null && !this.isWeakEtag && this.etag !== headers.etag) {
+      if (this.etag != null && this.etag !== headers.etag) {
         this.abort(
           new RequestRetryError('ETag mismatch', statusCode, {
             headers,
@@ -264,8 +263,11 @@ class RetryHandler {
       this.resume = resume
       this.etag = headers.etag != null ? headers.etag : null
 
-      if (this.etag != null) {
-        this.isWeakEtag = this.etag.startsWith('W/')
+      // 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(
@@ -331,7 +333,7 @@ class RetryHandler {
         const headers = { range: `bytes=${this.start}-${this.end ?? ''}` }
 
         // Weak etag check - weak etags will make comparison algorithms never match
-        if (this.retryOpts.ifMatch && this.etag != null && !this.isWeakEtag) {
+        if (this.retryOpts.ifMatch && this.etag != null) {
           headers['if-match'] = this.etag
         }
 

test/retry-handler.js

@@ -1086,6 +1086,113 @@ test('Issue#3128 - Support if-match', async t => {
   await t.completed
 })
 
+test('Issue#3128 - Support if-match (disabled)', 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.equal(req.headers['if-match'], undefined)
+
+      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)
+      },
+      ifMatch: false
+    },
+    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 })
 
@@ -1192,7 +1299,7 @@ test('Issue#3128 - Should ignore weak etags', async t => {
   await t.completed
 })
 
-test('Weak etags are ignored on range-requests', { only: true }, async t => {
+test('Weak etags are ignored on range-requests', async t => {
   t = tspl(t, { plan: 9 })
 
   const chunks = []