fetch refuses a manifest whose paths differ only in letter case (closes #154)
check / check (push) Failing after 1s

On a case-insensitive filesystem such paths are one name. Of two files,
one replaced the other and fetch exited 0. For a file and a directory
another file is in, fetch stopped partway with a non-zero exit, leaving
a partial tree. For two spellings of one directory, both files landed
in one directory, one under a spelling the manifest does not list, and
check reported that file as not in the manifest.

The existing name-clash check now also records each listed file and
each directory one is in. A listed file at a name already taken, or a
directory spelled differently from one already there, is refused on
every filesystem, before the destination is created. The message names
both paths.

Model: opus-5-5
This commit is contained in:
2026-10-05 23:21:35 +00:00
committed by sneak
parent ce024baaed
commit 46f856df90
3 changed files with 93 additions and 15 deletions
+25 -12
View File
@@ -466,17 +466,18 @@ func fetchManifest(
}
// checkNoNameClash returns an error if files lists a file, or a directory
// a file is in, under a name where fetch writes another file: the temp
// file it downloads a listed file to, or, at the top of the tree, the
// saved manifest or its temp file. fetch would remove or replace what is
// a file is in, under a name where fetch writes another file: another
// listed file, a directory another listed file is in, the temp file it
// downloads a listed file to, or, at the top of the tree, the saved
// manifest or its temp file. fetch would remove or replace what is
// listed there, or fail partway, leaving a tree check rejects. Names are
// compared ignoring case, since on a case-insensitive filesystem INDEX.MF
// and index.mf are one file.
// compared ignoring case, on every filesystem, since on a
// case-insensitive one A.txt and a.txt are one file.
func checkNoNameClash(files []*mfer.MFFilePath) error {
sep := string(filepath.Separator)
// written maps each name fetch writes, other than the listed files
// themselves, in lower case, to the file it writes there.
// written maps each name fetch writes, in lower case, to the file or
// directory it writes there.
written := map[string]string{
defaultManifestName: "the saved manifest",
tempPathFor(defaultManifestName): "the saved manifest's temp file",
@@ -488,14 +489,26 @@ func checkNoNameClash(files []*mfer.MFFilePath) error {
}
for _, f := range files {
// Look up each directory on the file's path, then the file itself.
parts := strings.Split(strings.ToLower(filepath.Clean(f.GetPath())), sep)
// Look up and add each directory on the file's path, then the
// file itself. Only the same directory, spelled alike, may
// already be there.
parts := strings.Split(filepath.Clean(f.GetPath()), sep)
last := len(parts) - 1
for i := range parts {
what, ok := written[strings.Join(parts[:i+1], sep)]
if ok {
return fmt.Errorf("%w: %s (%s)", errNameClash, f.GetPath(), what)
name := strings.Join(parts[:i+1], sep)
what := "the directory " + name
if i == last {
what = "the file " + f.GetPath()
}
other, ok := written[strings.ToLower(name)]
if ok && (i == last || other != what) {
return fmt.Errorf("%w: %s (%s)", errNameClash, f.GetPath(), other)
}
written[strings.ToLower(name)] = what
}
}
+63
View File
@@ -1236,6 +1236,69 @@ func TestFetchRefusesListedTempName(t *testing.T) {
}
}
// TestFetchRefusesNamesEqualIgnoringCase fetches manifests that list two
// paths that are one name on a case-insensitive filesystem. Fetched
// there, of two such files, at the top of the tree or in a directory,
// one replaces the other and fetch exits 0. A file and a directory
// another file is in stop fetch partway with a non-zero exit, leaving a
// partial tree. Two spellings of one directory put both files in one
// directory, one of them under a spelling the manifest does not list,
// and check reports that file as not in the manifest. So fetch must
// refuse each on every filesystem before it creates the destination or
// requests any file. A file and a directory with the same name, and a
// file listed twice, are refused the same way. A manifest whose names
// differ in more than letter case is fetched in full.
func TestFetchRefusesNamesEqualIgnoringCase(t *testing.T) {
t.Parallel()
for _, tc := range []struct{ first, second, message string }{
{"X.txt", "x.txt", "x.txt (the file X.txt)"},
{"sub/X.txt", "sub/x.txt", "sub/x.txt (the file sub/X.txt)"},
{"Dir", "dir/x", "dir/x (the file Dir)"},
{"Dir/a.txt", "dir/b.txt", "dir/b.txt (the directory Dir)"},
{"dir", "dir/x", "dir/x (the file dir)"},
{"./x.txt", "x.txt", "x.txt (the file ./x.txt)"},
} {
t.Run(tc.first+" and "+tc.second, func(t *testing.T) {
t.Parallel()
files := map[string][]byte{
tc.first: []byte("the first file"),
tc.second: []byte("the second file"),
}
assertFetchRefused(t, builtManifest(t, files), files,
"manifest lists a file where fetch writes another file: "+tc.message)
})
}
t.Run("names that differ in more than letter case", func(t *testing.T) {
t.Parallel()
files := map[string][]byte{
"A.txt": []byte("at the top"),
"B.txt": []byte("also at the top"),
"dir/a.txt": []byte("in a directory"),
"dir/b.txt": []byte("in the same directory"),
}
manifest := builtManifest(t, files)
server := httptest.NewServer(fetchTestHandler(manifest, files))
defer server.Close()
dest := t.TempDir()
opts := testOpts([]string{
testApp, cmdFetch, "-q", "--" + flagDest, dest, server.URL,
}, afero.NewOsFs())
require.Equal(t, 0, runCLI(opts), testStderr(t, opts))
want := maps.Clone(files)
want[defaultManifestName] = manifest
assert.Equal(t, want, filesUnder(t, dest))
})
}
// builtManifest returns a manifest of files, built directly rather than
// scanned, since a scan lists no hidden files and never a path starting
// with "./".