fix: always bypass cache when ?write=true

isaacs · isaacs · commit ba8b4fe60eb6 · 2020-01-29T15:11:52.000-08:00
The npm CLI makes GET requests with ?write=true in some cases where it's
intending to send an immediate PUT or DELETE.  Always bypass the cache
for such requests, mirroring the behavior of the registry caching
mechanisms.

Back-ported for v4.
diff --git a/index.js b/index.js
@@ -53,26 +53,38 @@ function regFetch (uri, opts) {
       })
     }
   }
-  if (opts.query) {
-    let q = opts.query
+
+  let q = opts.query
+  if (q) {
     if (typeof q === 'string') {
       q = qs.parse(q)
+    } else if (typeof q !== 'object') {
+      throw new TypeError('invalid query option, must be string or object')
     }
     Object.keys(q).forEach(key => {
       if (q[key] === undefined) {
         delete q[key]
       }
     })
-    if (Object.keys(q).length) {
-      const parsed = url.parse(uri)
-      parsed.search = '?' + qs.stringify(
-        parsed.query
-          ? Object.assign(qs.parse(parsed.query), q)
-          : q
-      )
-      uri = url.format(parsed)
+  }
+  const parsed = url.parse(uri)
+
+  const query = parsed.query ? Object.assign(qs.parse(parsed.query), q || {})
+    : Object.keys(q || {}).length ? q
+      : null
+
+  if (query) {
+    if (String(query.write) === 'true' && opts.method === 'GET') {
+      opts = opts.concat({
+        offline: false,
+        'prefer-offline': false,
+        'prefer-online': true
+      })
     }
+    parsed.search = '?' + qs.stringify(query)
+    uri = url.format(parsed)
   }
+
   return opts.Promise.resolve(body).then(body => fetch(uri, {
     agent: opts.agent,
     algorithms: opts.algorithms,
diff --git a/test/index.js b/test/index.js
@@ -428,6 +428,36 @@ test('log warning header info', t => {
     .then(res => t.equal(res.status, 200, 'got successful response'))
 })
 
+test('query string with ?write=true', t => {
+  const {resolve} = require('path')
+  const cache = resolve(__dirname, 'index-query-string-with-write-true')
+  const mkdirp = require('mkdirp').sync
+  mkdirp(cache)
+  const rimraf = require('rimraf')
+  t.teardown(() => rimraf.sync(cache))
+
+  const opts = OPTS.concat({ 'prefer-offline': true, cache })
+  const qsString = opts.concat({ query: { write: 'true' } })
+  const qsBool = opts.concat({ query: { write: true } })
+  tnock(t, opts.registry)
+    .get('/hello?write=true')
+    .times(6)
+    .reply(200, { write: 'go for it' })
+
+  return fetch.json('/hello?write=true', opts)
+    .then(res => t.strictSame(res, { write: 'go for it' }))
+    .then(() => fetch.json('/hello?write=true', opts))
+    .then(res => t.strictSame(res, { write: 'go for it' }))
+    .then(() => fetch.json('/hello', qsString))
+    .then(res => t.strictSame(res, { write: 'go for it' }))
+    .then(() => fetch.json('/hello', qsString))
+    .then(res => t.strictSame(res, { write: 'go for it' }))
+    .then(() => fetch.json('/hello', qsBool))
+    .then(res => t.strictSame(res, { write: 'go for it' }))
+    .then(() => fetch.json('/hello', qsBool))
+    .then(res => t.strictSame(res, { write: 'go for it' }))
+})
+
 // TODO
 // * npm-session
 // * npm-in-ci
