path/filepath: Clean removes ending slash for volume on Windows in Go 1.21.4 #64028

calmh · 2023-11-09T07:34:39Z

Go 1.21.4 fixed a problem in filepath.Clean regarding \??\ paths. However, it also removed the ending slash for volumes in \\?\ paths. Is this expected? The docs still state

The returned path ends in a slash only if it represents a root directory, such as "/" on Unix or C:\ on Windows.

The following test passes on Go 1.21.3.

func TestFilepathClean(t *testing.T) {
	cases := []struct {
		in  string
		out string
	}{
		{`C:\`, `C:\`},
		{`\\?\C:\`, `\\?\C:\`},
	}
	for _, c := range cases {
		if out := filepath.Clean(c.in); out != c.out {
			t.Errorf("filepath.Clean(%s) => %s expected %s", c.in, out, c.out)
		}
	}
}

On Go 1.21.4 this happens:

PS C:\Users\jb\dev\syncthing\lib\fs> go1.21.4 test -run FilepathClean
--- FAIL: TestFilepathClean (0.00s)
    basicfs_windows_test.go:30: filepath.Clean(\\?\C:\) => \\?\C: expected \\?\C:\
FAIL
exit status 1
FAIL    github.com/syncthing/syncthing/lib/fs   0.062s

The text was updated successfully, but these errors were encountered:

qmuntal · 2023-11-09T08:22:22Z

Thanks for reporting. Seems like a regression.

@golang/windows @neild

bcmills · 2023-11-09T14:46:56Z

@gopherbot, please backport to Go 1.21 and maybe 1.20. This appears to be a regression introduced in a security release.

gopherbot · 2023-11-09T14:47:07Z

Backport issue(s) opened: #64040 (for 1.20), #64041 (for 1.21).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

gopherbot · 2023-11-09T18:02:04Z

Change https://go.dev/cl/541175 mentions this issue: path/filepath: consider \\?\c: as a volume on Windows

neild · 2023-11-09T18:04:53Z

This falls out from an inadvertent change to what we consider the "volume name" to be in paths starting with \\?\. https://go.dev/cl/541175 restores the previous behavior.

dagood · 2023-11-09T18:19:42Z

I confirmed the CL also fixes another bit of fallout: calling filepath.EvalSymlinks on a path like \\?\Volume{45a4...f138}\ fails with go1.21.4, go1.20.11, and gotip: GetFileInformationByHandle \\?\Volume{45a4...f138}: Incorrect function.. With gotip download 541175, the call succeeds.

Reading the code, this makes sense, but I figured it might still be worth mentioning. 🙂

… Windows While fixing several bugs in path handling on Windows, beginning with \\?\. Prior to #540277, VolumeName considered the first path component after the \\?\ prefix to be part of the volume name. After, it considered only the \\? prefix to be the volume name. Restore the previous behavior. Fixes golang#64040 Updates golang#64028 Change-Id: I6523789e61776342800bd607fb3f29d496257e68 Reviewed-on: https://go-review.googlesource.com/c/go/+/541175 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> (cherry picked from commit eda42f7) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

… Windows While fixing several bugs in path handling on Windows, beginning with \\?\. Prior to #540277, VolumeName considered the first path component after the \\?\ prefix to be part of the volume name. After, it considered only the \\? prefix to be the volume name. Restore the previous behavior. Fixes golang#64041 Updates golang#64028 Change-Id: I6523789e61776342800bd607fb3f29d496257e68 Reviewed-on: https://go-review.googlesource.com/c/go/+/541175 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> (cherry picked from commit eda42f7) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

… Windows While fixing several bugs in path handling on Windows, beginning with \\?\. Prior to #540277, VolumeName considered the first path component after the \\?\ prefix to be part of the volume name. After, it considered only the \\? prefix to be the volume name. Restore the previous behavior. Fixes golang#64040 Updates golang#64028 Change-Id: I6523789e61776342800bd607fb3f29d496257e68 Reviewed-on: https://go-review.googlesource.com/c/go/+/541175 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> (cherry picked from commit eda42f7)

… Windows While fixing several bugs in path handling on Windows, beginning with \\?\. Prior to #540277, VolumeName considered the first path component after the \\?\ prefix to be part of the volume name. After, it considered only the \\? prefix to be the volume name. Restore the previous behavior. Fixes golang#64041 Updates golang#64028 Change-Id: I6523789e61776342800bd607fb3f29d496257e68 Reviewed-on: https://go-review.googlesource.com/c/go/+/541175 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> (cherry picked from commit eda42f7)

gopherbot · 2023-11-13T09:11:39Z

Change https://go.dev/cl/541520 mentions this issue: [release-branch.go1.20] path/filepath: consider \\?\c: as a volume on Windows

gopherbot · 2023-11-13T09:12:06Z

Change https://go.dev/cl/541521 mentions this issue: [release-branch.go1.21] path/filepath: consider \\?\c: as a volume on Windows

thaJeztah · 2023-11-13T09:14:01Z

Opened backports for go1.20 and go1.21;

go1.20: [release-branch.go1.20] path/filepath: consider \\?\c: as a volume on Windows #64091 / https://go-review.googlesource.com/c/go/+/541520
go1.21: [release-branch.go1.21] path/filepath: consider \\?\c: as a volume on Windows #64092 / https://go-review.googlesource.com/c/go/+/541521

@bcmills do you know if there's an ETA for patch-releases with this fix? It's currently preventing various projects from updating (and thus missing the security fixes)

gopherbot · 2023-11-13T09:25:43Z

Change https://go.dev/cl/541836 mentions this issue: [release-branch.go1.20] path/filepath: consider \\?\c: as a volume on Windows

gopherbot · 2023-11-13T09:26:09Z

Change https://go.dev/cl/541837 mentions this issue: [release-branch.go1.21] path/filepath: consider \\?\c: as a volume on Windows

thaJeztah · 2023-11-20T22:43:49Z

@bcmills do you know if there's an ETAfor go1.20 and 1.21 patch releases with this fix? This regression is currently preventing various projects from updating (and thus patching the CVEs fixed in the latest release)

thaJeztah · 2023-11-27T21:30:10Z

@rsc @bcmills anyone able to provide info on an ETA for a new patch release?

bcmills · 2023-11-27T21:42:05Z

Generally patch releases occur monthly, although I'm not sure about the timing for the next releases due to U.S. holidays in November and December. (@golang/release may have a clearer idea of when they will occur.)

thaJeztah · 2023-11-27T21:44:31Z

Thanks! Yeah, people may get itchy if they run their vulnerability scanner and see "unpatched CVE" on binaries, so was curious if there was an ETA with the patches for this regression.

dmitshur · 2023-11-27T21:45:22Z

Minor releases are planned for the beginning of each month. I expect the next month's (December) minor releases to follow that pattern.

thaJeztah · 2023-11-27T22:08:07Z

Gotcha. I wasn't sure if regressions introduced in security releases followed the same schedule, or if there were exceptions to that.

… Windows While fixing several bugs in path handling on Windows, beginning with \\?\. Prior to #540277, VolumeName considered the first path component after the \\?\ prefix to be part of the volume name. After, it considered only the \\? prefix to be the volume name. Restore the previous behavior. For #64028. Fixes #64041. Change-Id: I6523789e61776342800bd607fb3f29d496257e68 Reviewed-on: https://go-review.googlesource.com/c/go/+/541175 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> (cherry picked from commit eda42f7) Reviewed-on: https://go-review.googlesource.com/c/go/+/541521 Reviewed-by: Damien Neil <dneil@google.com> Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

… Windows While fixing several bugs in path handling on Windows, beginning with \\?\. Prior to #540277, VolumeName considered the first path component after the \\?\ prefix to be part of the volume name. After, it considered only the \\? prefix to be the volume name. Restore the previous behavior. For #64028. Fixes #64040. Change-Id: I6523789e61776342800bd607fb3f29d496257e68 Reviewed-on: https://go-review.googlesource.com/c/go/+/541175 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> (cherry picked from commit eda42f7) Reviewed-on: https://go-review.googlesource.com/c/go/+/541520 Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

neild · 2023-11-28T21:37:07Z

Relnote:

Go 1.20.11 and Go 1.21.4 inadvertently changed the definition of the volume name in Windows paths starting with \\?\, resulting in filepath.Clean(\\?\c:\) returning \\?\c: rather than \\?\c:\ (among other effects). The previous behavior has been restored.

This is an update to CVE-2023-45283 and https://go.dev/issue/64028.

We found an issue[1] in golang's path/filepath stdlib impacting the build of at least osconfig (potentially others). The bug was fixed and will potentially be available in 1.31.5, we'll get it update whenever it's released. [1] - golang/go#64028

While fixing several bugs in path handling on Windows, beginning with \\?\. Prior to #540277, VolumeName considered the first path component after the \\?\ prefix to be part of the volume name. After, it considered only the \\? prefix to be the volume name. Restore the previous behavior. For golang#64028. Fixes golang#64040. Change-Id: I6523789e61776342800bd607fb3f29d496257e68 Reviewed-on: https://go-review.googlesource.com/c/go/+/541175 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Roland Shoemaker <roland@golang.org> (cherry picked from commit eda42f7) Reviewed-on: https://go-review.googlesource.com/c/go/+/541520 Auto-Submit: Dmitri Shuralyov <dmitshur@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org> Reviewed-by: Damien Neil <dneil@google.com> Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

calmh mentioned this issue Nov 9, 2023

Go 1.21.4 path handling changes breaks Windows build syncthing/syncthing#9207

Closed

qmuntal added OS-Windows NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 9, 2023

bcmills assigned neild Nov 9, 2023

bcmills added the release-blocker label Nov 9, 2023

bcmills added this to the Go1.22 milestone Nov 9, 2023

gopherbot mentioned this issue Nov 9, 2023

path/filepath: Clean removes ending slash for volume on Windows in Go 1.21.4 [1.20 backport] #64040

Closed

gopherbot mentioned this issue Nov 9, 2023

path/filepath: Clean removes ending slash for volume on Windows in Go 1.21.4 [1.21 backport] #64041

Closed

bcmills added the Security label Nov 9, 2023

corhere mentioned this issue Nov 9, 2023

update to go1.21.5 moby/moby#46787

Merged

thaJeztah mentioned this issue Nov 9, 2023

update to go1.21.5, go1.20.12 containerd/containerd#9351

Merged

gopherbot closed this as completed in eda42f7 Nov 9, 2023

This was referenced Nov 13, 2023

[release-branch.go1.20] path/filepath: consider \\?\c: as a volume on Windows #64091

Closed

[release-branch.go1.21] path/filepath: consider \\?\c: as a volume on Windows #64092

Closed

bcmills mentioned this issue Nov 13, 2023

path/filepath: regression in filepath.VolumeName #64101

Closed

thaJeztah mentioned this issue Nov 20, 2023

build: bump golang from 1.20.7 to 1.21.4 nginx-proxy/docker-gen#565

Closed

dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Nov 27, 2023

dorileo mentioned this issue Nov 29, 2023

package build: downgrade golang to 1.21.3 GoogleCloudPlatform/guest-test-infra#941

Merged

thaJeztah mentioned this issue Dec 1, 2023

update: bump Go version distribution/distribution#4176

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

path/filepath: Clean removes ending slash for volume on Windows in Go 1.21.4 #64028

path/filepath: Clean removes ending slash for volume on Windows in Go 1.21.4 #64028

calmh commented Nov 9, 2023

qmuntal commented Nov 9, 2023 •

edited

bcmills commented Nov 9, 2023

gopherbot commented Nov 9, 2023

gopherbot commented Nov 9, 2023

neild commented Nov 9, 2023

dagood commented Nov 9, 2023

gopherbot commented Nov 13, 2023

gopherbot commented Nov 13, 2023

thaJeztah commented Nov 13, 2023

gopherbot commented Nov 13, 2023

gopherbot commented Nov 13, 2023

thaJeztah commented Nov 20, 2023

thaJeztah commented Nov 27, 2023

bcmills commented Nov 27, 2023

thaJeztah commented Nov 27, 2023

dmitshur commented Nov 27, 2023

thaJeztah commented Nov 27, 2023

neild commented Nov 28, 2023 •

edited

path/filepath: Clean removes ending slash for volume on Windows in Go 1.21.4 #64028

path/filepath: Clean removes ending slash for volume on Windows in Go 1.21.4 #64028

Comments

calmh commented Nov 9, 2023

qmuntal commented Nov 9, 2023 • edited

bcmills commented Nov 9, 2023

gopherbot commented Nov 9, 2023

gopherbot commented Nov 9, 2023

neild commented Nov 9, 2023

dagood commented Nov 9, 2023

gopherbot commented Nov 13, 2023

gopherbot commented Nov 13, 2023

thaJeztah commented Nov 13, 2023

gopherbot commented Nov 13, 2023

gopherbot commented Nov 13, 2023

thaJeztah commented Nov 20, 2023

thaJeztah commented Nov 27, 2023

bcmills commented Nov 27, 2023

thaJeztah commented Nov 27, 2023

dmitshur commented Nov 27, 2023

thaJeztah commented Nov 27, 2023

neild commented Nov 28, 2023 • edited

qmuntal commented Nov 9, 2023 •

edited

neild commented Nov 28, 2023 •

edited