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

path/filepath: Clean doesn't remove trailing backslash from Windows volume name #27791

Open
QtRoS opened this issue Sep 21, 2018 · 8 comments
Open

path/filepath: Clean doesn't remove trailing backslash from Windows volume name #27791

QtRoS opened this issue Sep 21, 2018 · 8 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone
Backlog

Comments

@QtRoS
Copy link
Contributor

QtRoS commented Sep 21, 2018

What version of Go are you using (go version)?

go 1.10.3 windows/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

Windows 7, Core I7, x64

What did you do?

filepath.Clean(`\\somepath\dir\`)

What did you expect to see?

\\somepath\dir (without trailing slash as per documentation)

What did you see instead?

\\somepath\dir\ (with trailing shash)

Seems that problem is in two Windows slashes at the beginning, all other cases work just fine.
@QtRoS
Copy link
Contributor Author

QtRoS commented Sep 21, 2018

Some more testcases: https://paste.ofcode.org/KcUi8HKPrhLCcMmMET356T

@QtRoS
Copy link
Contributor Author

QtRoS commented Sep 21, 2018 •
edited
Loading

After some investigation I discovered that the problem is in volumeNameLen function. I dunno why, but it considers \\somepath\dir\ as volume name. I am not windows expert, but when we are speaking about network paths, it's simply machine name and it's shared folder.

@alexbrainman
Copy link
Member

I can confirm, that filepath.Clean does not remove trailing backslash from \\somepath\dir\. I think it should. And I don't see any reason why we should not fix that.

@QtRoS would you like to send a fix for this? https://golang.org/doc/contribute.html is how to contribute.

Alex

@alexbrainman alexbrainman changed the title filepath.Clean doesn't remove trailing slash path/filepath: Clean doesn't remove trailing slash Sep 22, 2018
@bcmills bcmills changed the title path/filepath: Clean doesn't remove trailing slash path/filepath: Clean doesn't remove trailing backslash from Windows volume name Sep 22, 2018
@bcmills bcmills added OS-Windows NeedsFix The path to resolution is known, but the work has not been done. labels Sep 22, 2018
@bcmills bcmills added this to the Go1.12 milestone Sep 22, 2018
@QtRoS
Copy link
Contributor Author

QtRoS commented Sep 24, 2018

@alexbrainman yes, I will surely try to fix it soon!

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/137055 mentions this issue: path/filepath: fix Windows-specific Clean bug

@lowenna lowenna mentioned this issue Feb 19, 2019
Closed
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/163077 mentions this issue: path/filepath: revert "fix Windows-specific Clean bug"

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/163078 mentions this issue: [release-branch.go1.12] path/filepath: revert "fix Windows-specific Clean bug"

gopherbot pushed a commit that referenced this issue Feb 20, 2019
@ianlancetaylor
path/filepath: revert "fix Windows-specific Clean bug"
153c0da 
Revert CL 137055, which changed Clean("\\somepath\dir\") to return
"\\somepath\dir" on Windows. It's not entirely clear this is correct,
as this path is really "\\server\share\", and as such the trailing
slash may be the path on that share, much like "C:\". In any case, the
change broke existing code, so roll it back for now and rethink for 1.13.

Updates #27791
Fixes #30307

Change-Id: I69200b1efe38bdb6d452b744582a2bfbb3acbcec
Reviewed-on: https://go-review.googlesource.com/c/163077
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
@ianlancetaylor
Copy link
Contributor

Reopening because the CL was reverted. But see #30307, which argues that this suggested change is not correct in at least some cases.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.12, Go1.13 Feb 20, 2019
@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 20, 2019
@gopherbot gopherbot removed the NeedsFix The path to resolution is known, but the work has not been done. label Feb 20, 2019
nebulabox pushed a commit to nebulabox/go that referenced this issue Feb 21, 2019
@ianlancetaylor @nebulabox
path/filepath: revert "fix Windows-specific Clean bug"
6c5c47d 
Revert CL 137055, which changed Clean("\\somepath\dir\") to return
"\\somepath\dir" on Windows. It's not entirely clear this is correct,
as this path is really "\\server\share\", and as such the trailing
slash may be the path on that share, much like "C:\". In any case, the
change broke existing code, so roll it back for now and rethink for 1.13.

Updates golang#27791
Fixes golang#30307

Change-Id: I69200b1efe38bdb6d452b744582a2bfbb3acbcec
Reviewed-on: https://go-review.googlesource.com/c/163077
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
gopherbot pushed a commit that referenced this issue Feb 26, 2019
@ianlancetaylor
[release-branch.go1.12] path/filepath: revert "fix Windows-specific C…
c55fb33 
…lean bug"

Revert CL 137055, which changed Clean("\\somepath\dir\") to return
"\\somepath\dir" on Windows. It's not entirely clear this is correct,
as this path is really "\\server\share\", and as such the trailing
slash may be the path on that share, much like "C:\". In any case, the
change broke existing code, so roll it back for now and rethink for 1.13.

Updates #27791
Updates #30307

Change-Id: I69200b1efe38bdb6d452b744582a2bfbb3acbcec
Reviewed-on: https://go-review.googlesource.com/c/163077
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
(cherry picked from commit 153c0da)
Reviewed-on: https://go-review.googlesource.com/c/163078
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@imsodin imsodin mentioned this issue May 11, 2019
Closed
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@gabyhelp gabyhelp mentioned this issue Jun 8, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
7 participants
@rsc @andybons @QtRoS @ianlancetaylor @bcmills @gopherbot @alexbrainman