From e625302d1b9efcce194d8440a444e314288e43f0 Mon Sep 17 00:00:00 2001 From: "Bala.FA" Date: Tue, 13 Oct 2026 13:45:19 +0530 Subject: [PATCH] fix: keep the walk flat key sorted on Windows A directory is taken to end in `/` before sorting, so the walk comes out in flat key order: the order an object store lists the same names in. byName appended os.PathSeparator instead, which is that byte on Unix but 0x5C on Windows, above the digits and the upper case letters, so a directory there sorted after siblings it should have come before. Unix is unchanged, the separator there being `/` already. The test asserts the documented order, and the workflow runs it on ubuntu, macos and windows. Signed-off-by: Bala.FA --- .github/workflows/test.yml | 29 ++++++++++ .github/workflows/vulncheck.yml | 6 +-- go.mod | 2 +- walk.go | 15 +++++- walk_test.go | 94 +++++++++++++++++++++++++++++++++ 5 files changed, 140 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/test.yml create mode 100644 walk_test.go diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 0000000..557c012 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,29 @@ +name: Test +on: + pull_request: + branches: + - master + - main + push: + branches: + - master + - main +jobs: + test: + name: Walk order on ${{ matrix.os }} + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ ubuntu-latest, macos-latest, windows-latest ] + go-version: [ 1.27 ] + steps: + - name: Check out code into the Go module directory + uses: actions/checkout@v7 + - uses: actions/setup-go@v7 + with: + go-version: ${{ matrix.go-version }} + check-latest: true + - name: Run tests + run: go test -v ./... + shell: bash diff --git a/.github/workflows/vulncheck.yml b/.github/workflows/vulncheck.yml index 7b8dfe8..bc8a3cb 100644 --- a/.github/workflows/vulncheck.yml +++ b/.github/workflows/vulncheck.yml @@ -14,11 +14,11 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - go-version: [ 1.19 ] + go-version: [ 1.27 ] steps: - name: Check out code into the Go module directory - uses: actions/checkout@v3 - - uses: actions/setup-go@v3 + uses: actions/checkout@v7 + - uses: actions/setup-go@v7 with: go-version: ${{ matrix.go-version }} check-latest: true diff --git a/go.mod b/go.mod index 2ddfa30..17c67a7 100644 --- a/go.mod +++ b/go.mod @@ -1,3 +1,3 @@ module github.com/minio/filepath -go 1.18 +go 1.27 diff --git a/walk.go b/walk.go index b66b1b7..0557c22 100644 --- a/walk.go +++ b/walk.go @@ -30,6 +30,17 @@ func Walk(root string, walkFn WalkFunc) error { return walk(root, info, walkFn) } +// flatKeySeparator is what a directory name is taken to end in while entries +// are sorted. +// +// An object store sorts `a/b` under `a` because `/` is 0x2F, below every +// character a sibling name can begin with. os.PathSeparator is that same byte +// on Unix and 0x5C on Windows, which sits above the digits and the upper case +// letters: sorting by it there puts a directory after siblings it should come +// before, which is neither flat key order nor the order an object store lists +// the same names in. +const flatKeySeparator = "/" + // byName implements sort.Interface for sorting os.FileInfo list. type byName []os.FileInfo @@ -38,12 +49,12 @@ func (f byName) Swap(i, j int) { f[i], f[j] = f[j], f[i] } func (f byName) Less(i, j int) bool { n1 := f[i].Name() if f[i].IsDir() { - n1 = n1 + string(os.PathSeparator) + n1 = n1 + flatKeySeparator } n2 := f[j].Name() if f[j].IsDir() { - n2 = n2 + string(os.PathSeparator) + n2 = n2 + flatKeySeparator } return n1 < n2 diff --git a/walk_test.go b/walk_test.go new file mode 100644 index 0000000..0095905 --- /dev/null +++ b/walk_test.go @@ -0,0 +1,94 @@ +// Copyright 2010 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the https://golang.org/LICENSE file. + +package filepath + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// TestWalkIsFlatKeySorted checks the order this package exists to produce: +// +// lexically ordered output treats directories with `/` at the end before +// sorting, this is sorting is flat key sorting instead of regular +// filesystem sorting of entries. +// +// Flat key order is an object store's order. A key `a/b` sorts under `a` +// because `/` is 0x2F, below every character a sibling name can start with. +// byName appends os.PathSeparator rather than `/`: the same byte on Unix, and +// 0x5C on Windows, which sits above the digits and the upper case letters. +// +// So one directory and its siblings are ordered differently on Windows, and +// the walk there is neither flat key sorted nor in the order an object store +// lists the same names in. The tree below puts a directory among siblings +// whose first differing byte falls on each side of the two separators: +// +// 0x2D a-1.bin below `/` and below `\` +// 0x2F a/ the directory, where flat key order puts it +// 0x30 a0.bin above `/`, below `\` -- moves on Windows +// 0x5A aZ.bin above `/`, below `\` -- moves on Windows +// 0x5C a\ the directory, where this walk puts it on Windows +// 0x5F a_.bin above both +// 0x61 aa.bin above both +// +// Every name is reported in slash form, so what is compared here is the order +// alone and not how a platform spells a path. +func TestWalkIsFlatKeySorted(t *testing.T) { + root := t.TempDir() + if err := os.Mkdir(filepath.Join(root, "a"), 0o755); err != nil { + t.Fatal(err) + } + for _, name := range []string{ + "a/inner.bin", + "a-1.bin", + "a0.bin", + "aZ.bin", + "a_.bin", + "aa.bin", + } { + if err := os.WriteFile(filepath.Join(root, filepath.FromSlash(name)), []byte("x"), 0o600); err != nil { + t.Fatal(err) + } + } + + var got []string + err := Walk(root, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + rel, e := filepath.Rel(root, path) + if e != nil { + return e + } + if rel == "." { + return nil + } + got = append(got, filepath.ToSlash(rel)) + return nil + }) + if err != nil { + t.Fatal(err) + } + + want := []string{"a-1.bin", "a", "a/inner.bin", "a0.bin", "aZ.bin", "a_.bin", "aa.bin"} + + t.Logf("GOOS=%s, os.PathSeparator=%q (0x%02X)", + runtime.GOOS, os.PathSeparator, os.PathSeparator) + t.Logf("flat key order: %s", strings.Join(want, " ")) + t.Logf("this walk : %s", strings.Join(got, " ")) + + if strings.Join(got, " ") != strings.Join(want, " ") { + t.Errorf("the walk is not flat key sorted on %s\n"+ + " want: %s\n"+ + " got: %s\n"+ + "byName appends os.PathSeparator (%q) where the package comment says `/`, "+ + "so the directory sorts above every sibling whose next byte is below 0x%02X.", + runtime.GOOS, strings.Join(want, " "), strings.Join(got, " "), + os.PathSeparator, os.PathSeparator) + } +}