Skip to content

Commit ee19d6f

Browse files
committedFeb 11, 2020
Do not mutate the opts arg passed into Fetcher
This removes a footgun that occurs when the Fetcher objects update their opts object to include the integrity when it's calculated. That's an appropriate thing to do with options objects passed through to subdeps of pacote, but mutating the object as it was in the *caller* means that passing the same options object to multiple pacote calls always results in numerous failures. CC: @mikemimik
1 parent 872a63e commit ee19d6f

File tree

2 files changed

+23
-3
lines changed

2 files changed

+23
-3
lines changed
 

‎lib/fetcher.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ class FetcherBase {
5858
? `${this.spec.name}@${this.spec.rawSpec}` : this.spec.saveSpec
5959

6060
this[_assertType]()
61-
this.opts = opts
61+
// clone the opts object so that others aren't upset when we mutate it
62+
// by adding/modifying the integrity value.
63+
this.opts = {...opts}
6264
this.cache = opts.cache || cacheDir()
6365
this.resolved = opts.resolved || null
6466

@@ -68,7 +70,7 @@ class FetcherBase {
6870
this.defaultIntegrityAlgorithm = opts.defaultIntegrityAlgorithm || 'sha512'
6971

7072
if (typeof opts.integrity === 'string')
71-
opts.integrity = ssri.parse(opts.integrity)
73+
this.opts.integrity = ssri.parse(opts.integrity)
7274

7375
this.package = null
7476
this.type = this.constructor.name

‎test/fetcher.js

+19-1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,24 @@ const Minipass = require('minipass')
4242
const FileFetcher = require('../lib/file.js')
4343
const cache = resolve(me, 'cache')
4444

45+
t.test('do not mutate opts object passed in', t => {
46+
const opts = {}
47+
const f = new FileFetcher(abbrevspec, opts)
48+
f.integrity = 'sha512-somekindofintegral'
49+
t.strictSame(opts, {})
50+
t.match(f.integrity, {
51+
sha512: [
52+
{
53+
source: 'sha512-somekindofintegral',
54+
digest: 'somekindofintegral',
55+
algorithm: 'sha512',
56+
options: [],
57+
},
58+
],
59+
})
60+
t.end()
61+
})
62+
4563
t.test('tarball data', t =>
4664
new FileFetcher(abbrevspec, { cache }).tarball()
4765
.then(data => {
@@ -434,7 +452,7 @@ t.test('set integrity, pick default algo', t => {
434452
}
435453
const f = new FileFetcher('pkg.tgz', opts)
436454
t.equal(f.pickIntegrityAlgorithm(), 'sha512')
437-
t.isa(opts.integrity, Object)
455+
t.isa(f.opts.integrity, Object)
438456
const i = f.integrity
439457
f.integrity = null
440458
t.equal(f.integrity, i, 'cannot remove integrity')

0 commit comments

Comments
 (0)
Please sign in to comment.