Skip to content
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

path: fix normalize for absolutes #5589

Merged
merged 1 commit into from
Mar 7, 2016
Merged

Conversation

evanlucas
Copy link
Contributor

Pull Request check-list

Please make sure to review and check all of these items:

  • Does make -j8 test (UNIX) or vcbuild test nosign (Windows) pass with
    this change (including linting)?
  • Is the commit message formatted according to CONTRIBUTING.md?
  • If this change fixes a bug (or a performance problem), is a regression
    test (or a benchmark) included?
  • Is a documentation update included (if this change modifies
    existing APIs, or introduces new ones)?

NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.

Affected core subsystem(s)

path

Description of change

Fixes a regression introduced by
b212be0.

path.normalize(''/a/b/c/../../../x/y/z'') should return '/x/y/z'.

Ref: #5585

@evanlucas evanlucas added the path Issues and PRs related to the path subsystem. label Mar 7, 2016
@evanlucas
Copy link
Contributor Author

@MylesBorins
Copy link
Contributor

LGTM

@Fishrock123 Fishrock123 added this to the 5.7.2 milestone Mar 7, 2016
@evanlucas evanlucas mentioned this pull request Mar 7, 2016
@jasnell
Copy link
Member

jasnell commented Mar 7, 2016

LGTM

@jasnell
Copy link
Member

jasnell commented Mar 7, 2016

Added don't land because I believe the original commit never landed on v4

@evanlucas
Copy link
Contributor Author

Oops, I listed the wrong commit with the regression. It is actually b212be0.

@mscdex
Copy link
Contributor

mscdex commented Mar 7, 2016

LGTM

Fixes a regression introduced by
b212be0.

path.normalize(''/a/b/c/../../../x/y/z'') should return '/x/y/z'.

Fixes: nodejs#5585
PR-URL: nodejs#5589
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
@evanlucas evanlucas closed this Mar 7, 2016
@evanlucas evanlucas deleted the fixpath branch March 7, 2016 20:53
@evanlucas evanlucas merged commit 3d3b45a into nodejs:master Mar 7, 2016
@evanlucas
Copy link
Contributor Author

Landed in 3d3b45a. Thanks!

MylesBorins pushed a commit that referenced this pull request Mar 7, 2016
Fixes a regression introduced by
b212be0.

path.normalize(''/a/b/c/../../../x/y/z'') should return '/x/y/z'.

Fixes: #5585
PR-URL: #5589
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
MylesBorins pushed a commit that referenced this pull request Mar 7, 2016
Notable changes

* http_parser:
  * revert d77c3bf which was causing errors inside of http client callbacks to not propagate.
* path:
  * a fix to a regression found in path.resolve for absolute paths with a single character name as the root directory (Evan Lucas) #5589
Fishrock123 pushed a commit that referenced this pull request Mar 8, 2016
Fixes a regression introduced by
b212be0.

path.normalize(''/a/b/c/../../../x/y/z'') should return '/x/y/z'.

Fixes: #5585
PR-URL: #5589
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Fishrock123 added a commit that referenced this pull request Mar 8, 2016
Notable changes

* path:
  * a fix to a regression found in path.resolve for absolute paths with a single character name as the root directory (Evan Lucas) #5589

PR-URL: #5559
Fishrock123 pushed a commit that referenced this pull request Mar 8, 2016
Fixes a regression introduced by
b212be0.

path.normalize(''/a/b/c/../../../x/y/z'') should return '/x/y/z'.

Fixes: #5585
PR-URL: #5589
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Fishrock123 added a commit that referenced this pull request Mar 8, 2016
Notable changes:

* child_process: “send()” now accepts an options parameter (cjihrig)
#5283
  - Currently the only option is “keepOpen”, which keeps the underlying
socket open after the message is sent.
* constants: “ENGINE_METHOD_RSA” is now correctly exposed (Sam Roberts)
#5463
* Fixed two regressions which originated in v5.7.0:
  - http: Errors inside of http client callbacks now propagate
correctly (Trevor Norris) #5591
  - path: Fixed normalization of absolute paths (Evan Lucas)
#5589
* repl: “start()” no longer requires an options parameter (cjihrig)
#5388
* util: Improved “format()” performance 50-300% (Evan Lucas)
#5360

PR-URL: #5559
Fishrock123 added a commit to Fishrock123/node that referenced this pull request Mar 9, 2016
Notable changes:

* child_process: “send()” now accepts an options parameter (cjihrig)
nodejs#5283
- Currently the only option is “keepOpen”, which keeps the underlying
socket open after the message is sent.
* constants: “ENGINE_METHOD_RSA” is now correctly exposed (Sam Roberts)
nodejs#5463
* Fixed two regressions which originated in v5.7.0:
  - http: Errors inside of http client callbacks now propagate
correctly (Trevor Norris) nodejs#5591
  - path: Fixed normalization of absolute paths (Evan Lucas)
nodejs#5589
* repl: “start()” no longer requires an options parameter (cjihrig)
nodejs#5388
* util: Improved “format()” performance 50-300% (Evan Lucas)
nodejs#5360

PR-URL: nodejs#5559
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
path Issues and PRs related to the path subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants