Skip to content
This repository has been archived by the owner on May 4, 2024. It is now read-only.

FIX don’t crash on non-string ‘bin’ entries #54

Closed
wants to merge 2 commits into from

Conversation

rentzsch
Copy link
Contributor

I've run read-package-json against every version of every package on npm and discovered an edge case where it throws an uncatchable exception when bin entry of package.json isn't a string. The exception stack trace is:

TypeError: Arguments to path.resolve must be strings
    at Object.posix.resolve (path.js:439:13)
    at /root/normalize-package-jsons/node_modules/read-package-json/read-json.js:324:24
    at Array.forEach (native)
    at checkBinReferences_ (/root/normalize-package-jsons/node_modules/read-package-json/read-json.js:321:8)
    at final (/root/normalize-package-jsons/node_modules/read-package-json/read-json.js:343:3)
    at then (/root/normalize-package-jsons/node_modules/read-package-json/read-json.js:113:5)
    at /root/normalize-package-jsons/node_modules/read-package-json/read-json.js:284:20
    at /root/normalize-package-jsons/node_modules/read-package-json/node_modules/graceful-fs/graceful-fs.js:76:16
    at fs.js:263:20
    at FSReqWrap.oncomplete (fs.js:95:15)

Here's a repro online. That one old version of mstrwebci is just one example, there's other problematic versions of this package and other packages.

This Pull Request catches the exact exception and simply ignores the bin key. It could be made better to warn about the issue, but for our use on https://tonicdev.com we don't need fine-grained warnings (we just clean up bad package.json's to make them usable if possible).

Passing test included.

@iarna
Copy link
Contributor

iarna commented Oct 30, 2015

Hi, thanks for this! This is definitely a bug– we don't want it crashing, but I may change the guard a little bit before landing this PR.

@rentzsch
Copy link
Contributor Author

Thanks! Changing the guard is fine, it's intentionally very focused on this one crasher and definitely could be generalized.

@rentzsch
Copy link
Contributor Author

Ping.

1 similar comment
@rentzsch
Copy link
Contributor Author

Ping.

@iarna
Copy link
Contributor

iarna commented Jun 26, 2017

This landed in 2.0.6 as 9beda7a. Changes:

  1. The guard is less strict.
  2. We warn when if a bin is a non-string.
  3. Other errors are passed back as an error to the callback, not thrown.
  4. The test now has an assert in it (and prints any warnings)

@iarna iarna closed this Jun 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants