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.

Sign up for GitHub

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

Jump to bottom

Breaking change in path normalizeString in node 9.9.0 #19519

Closed
xPaw opened this issue Mar 21, 2018 · 2 comments
Closed

Breaking change in path normalizeString in node 9.9.0 #19519

xPaw opened this issue Mar 21, 2018 · 2 comments
Labels
confirmed-bug Issues with confirmed bugs. path Issues and PRs related to the path subsystem.

Comments

@xPaw
Copy link

xPaw commented Mar 21, 2018

I believe #19237 to be the culprit of this change.

I believe that PR unintentionally changed the behaviour of at least path.resolve which was not documented in the changelog.

We have a specific test around path.resolve and it started failing after 9.9.0 was released. See https://travis-ci.org/thelounge/thelounge/jobs/356475798#L582

https://github.com/thelounge/thelounge/blob/844ca1fbe602aa291b54085a6cf69198ea4a5390/test/src/helperTest.js#L14-L16

On 9.8.0 this happens:

> require("path").resolve("a\\b")
'/home/xpaw/a\\b'

But on 9.9.0 it seems that separator is changed to the OS specific one.

> require("path").resolve("a\\b")
'/home/xpaw/a/b'
@targos targos added confirmed-bug Issues with confirmed bugs. path Issues and PRs related to the path subsystem. labels Mar 21, 2018
@targos
Copy link
Member

targos commented Mar 21, 2018

I have a fix ready. Just need to write tests.

targos added a commit to targos/node that referenced this issue Mar 21, 2018
@targos
path: fix regression in posix.normalize
3b978c6 
Fixes a regression introduced in
nodejs@4ae320f
The posix version of normalize should not treat backslash as a path
separator.

Fixes: nodejs#19519
@targos targos mentioned this issue Mar 21, 2018
Closed
3 tasks
@targos
Copy link
Member

targos commented Mar 21, 2018

#19520

@targos targos closed this as completed in a0adf56 Mar 23, 2018
targos added a commit that referenced this issue Mar 24, 2018
@targos
path: fix regression in posix.normalize
40916a2 
Fixes a regression introduced in [1].
The posix version of normalize should not treat backslash as a path
separator.

[1] 4ae320f2

PR-URL: #19520
Fixes: #19519
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@astorije astorije mentioned this issue Mar 25, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
confirmed-bug Issues with confirmed bugs. path Issues and PRs related to the path subsystem.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@xPaw @targos