New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #8761 in three different ways. #8765

Merged
merged 2 commits into from Jun 6, 2017

Conversation

Projects
None yet
3 participants
@benjamn
Member

benjamn commented Jun 6, 2017

The root of the problem was that the es5-ext npm package contains directories called #, e.g.
https://github.com/medikoo/es5-ext/tree/master/array/%23

These directory names were being sanitized to '' and thus ignored when reserving paths in the Builder, which led to reservation conflicts later.

This commit fixes the problem in three different and independently sufficient ways:

  • Use files.mkdir_p instead of files.mkdir when creating parent directories of written files.
  • Replace illegal characters in sanitized paths with _ instead of ''.
  • Allow # in sanitized paths (only needs to be escaped in the shell, not actually forbidden in paths).

Fixes #8761.

Fix #8761 in three different ways.
The root of the problem was that the es5-ext npm package contains
directories called '#', e.g.
https://github.com/medikoo/es5-ext/tree/master/array/%23

These directory names were being sanitized to '' and thus ignored when
reserving paths in the Builder, which led to reservation conflicts later.

This commit fixes the problem in three different and independently
sufficient ways:

* Use files.mkdir_p instead of files.mkdir when creating parent
  directories of written files.

* Replace illegal characters in sanitized paths with '_' instead of ''.

* Allow '#' in sanitized paths (only needs to be escaped in the shell, not
  actually forbidden in paths).

@benjamn benjamn added this to the Release 1.5.1 milestone Jun 6, 2017

@benjamn benjamn self-assigned this Jun 6, 2017

@benjamn benjamn requested review from hwillson and abernix Jun 6, 2017

@abernix

abernix approved these changes Jun 6, 2017 edited

Like, I get it... but what an interesting directory name! LGTM!

@hwillson

Looks great to me!

@benjamn benjamn merged commit cdc047b into devel Jun 6, 2017

4 checks passed

CLA Author has signed the Meteor CLA.
Details
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment