diff --git a/README.md b/README.md index 98d5446..8f36782 100644 --- a/README.md +++ b/README.md @@ -274,9 +274,11 @@ are now tracked only in the [issues](https://git.eeqj.de/sneak/mfer/issues). tree later. Each file is downloaded to a temp file beside it, such as `.a.txt.tmp` for `a.txt`, then moved into place. A manifest is refused before any file is downloaded if it lists a file where fetch writes - another: at the temp file of a listed file, or at `index.mf` or - `.index.mf.tmp` at the top of the tree. Names are compared in any letter - case. + another: at another listed file or a directory one is in, at the temp file + of a listed file, or at `index.mf` or `.index.mf.tmp` at the top of the + tree. Names are compared in any letter case, on every filesystem, since on + a case-insensitive one `A.txt` and `a.txt` are one file; a directory two + listed files are in must be spelled alike in both. - `mfer fetch --require-signature https://example.com/stuff/` - as above, but first refuses a manifest not signed by the key with that fingerprint, as `mfer check --require-signature` does, before downloading diff --git a/internal/cli/fetch.go b/internal/cli/fetch.go index 966a771..cc0a8aa 100644 --- a/internal/cli/fetch.go +++ b/internal/cli/fetch.go @@ -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 } } diff --git a/internal/cli/fetch_test.go b/internal/cli/fetch_test.go index 10e263c..57fcae0 100644 --- a/internal/cli/fetch_test.go +++ b/internal/cli/fetch_test.go @@ -1236,6 +1236,66 @@ func TestFetchRefusesListedTempName(t *testing.T) { } } +// TestFetchRefusesNamesEqualIgnoringCase fetches manifests that list two +// paths that are one name on a case-insensitive filesystem: two files, +// at the top of the tree and in a directory, a file and a directory +// another file is in, and two directories. Fetching such a manifest +// there would replace one with the other, so fetch must refuse it 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 "./".