fix: sanitize and normalize package bin field

Author: isaacs

lib/fetchers/directory.js

@@ -9,6 +9,7 @@ const readJson = require('../util/read-json')
 const path = require('path')
 const pipe = BB.promisify(require('mississippi').pipe)
 const through = require('mississippi').through
+const normalizePackageBin = require('npm-normalize-package-bin')
 
 const readFileAsync = BB.promisify(require('fs').readFile)
 
@@ -63,7 +64,7 @@ Fetcher.impl(fetchDirectory, {
       } else {
         return pkg
       }
-    })
+    }).then(pkg => normalizePackageBin(pkg))
   },
 
   // As of npm@5, the npm installer doesn't pack + install directories: it just

lib/finalize-manifest.js

@@ -14,6 +14,7 @@ const pipe = BB.promisify(require('mississippi').pipe)
 const ssri = require('ssri')
 const tar = require('tar')
 const readJson = require('./util/read-json')
+const normalizePackageBin = require('npm-normalize-package-bin')
 
 // `finalizeManifest` takes as input the various kinds of manifests that
 // manifest handlers ('lib/fetchers/*.js#manifest()') return, and makes sure
@@ -105,17 +106,8 @@ function Manifest (pkg, fromTarball, fullMetadata) {
   this._shrinkwrap = pkg._shrinkwrap || fromTarball._shrinkwrap || null
   this.bin = pkg.bin || fromTarball.bin || null
 
-  if (this.bin && Array.isArray(this.bin)) {
-    // Code yanked from read-package-json.
-    const m = (pkg.directories && pkg.directories.bin) || '.'
-    this.bin = this.bin.reduce((acc, mf) => {
-      if (mf && mf.charAt(0) !== '.') {
-        const f = path.basename(mf)
-        acc[f] = path.join(m, mf)
-      }
-      return acc
-    }, {})
-  }
+  // turn arrays and strings into a legit object, strip out bad stuff
+  normalizePackageBin(this)
 
   this._id = null
 

package-lock.json

@@ -58,6 +58,7 @@
     "mississippi": "^3.0.0",
     "mkdirp": "^0.5.1",
     "normalize-package-data": "^2.4.0",
+    "npm-normalize-package-bin": "^1.0.0",
     "npm-package-arg": "^6.1.0",
     "npm-packlist": "^1.1.12",
     "npm-pick-manifest": "^3.0.0",

package.json

@@ -62,7 +62,7 @@ test('returns a manifest with the right fields', t => {
       peerDependencies: {},
       peerDependenciesMeta: {},
       bin: {
-        testing: './foo.js'
+        testing: 'foo.js'
       },
       _shasum: 'deadbeef1',
       _resolved: 'resolved.to.this',
@@ -96,7 +96,6 @@ test('defaults all field to expected types + values', t => {
       bundleDependencies: false, // because npm does boolean checks on this
       peerDependencies: {},
       peerDependenciesMeta: {},
-      bin: null,
       _resolved: base._resolved,
       _integrity: base._integrity,
       _shasum: base._shasum,
@@ -212,28 +211,6 @@ test('fills in `bin` if `directories.bin` string', t => {
   })
 })
 
-test('fills in `bin` if original was an array', t => {
-  const tarballPath = 'testing/tarball-1.2.3.tgz'
-  const base = {
-    name: 'testing',
-    version: '1.2.3',
-    bin: ['my/bin1', 'bin2.js'],
-    directories: {
-      bin: 'foo'
-    },
-    _integrity: 'sha1-deadbeefc0ffeebad1dea',
-    _shasum: '75e69d6de79f7347df79e6da77575e',
-    _resolved: OPTS.registry + tarballPath,
-    _hasShrinkwrap: false
-  }
-  return finalizeManifest(base, npa(base.name), OPTS).then(manifest => {
-    t.deepEqual(manifest.bin, {
-      'bin1': path.join('foo', 'my', 'bin1'),
-      'bin2.js': path.join('foo', 'bin2.js')
-    }, 'bins successfully calculated')
-  })
-})
-
 test('uses package.json as base if passed null', t => {
   const tarballPath = 'testing/tarball-1.2.3.tgz'
   const base = {