Skip to content

Commit e389a18

Browse files
cwayne18claude
andcommitted
Fix go install: no colons in tracked file paths
A module zip may not contain a colon in a file path -- the allowed punctuation is "!#$%&()+,-.=@[]^_{}~" and nothing else, because a colon is not a legal filename on Windows and the format refuses to produce archives that unpack on some machines and not others. internal/pkgdb/testdata/debian12 is a real dpkg database, and dpkg names the multiarch file lists "libc6:amd64.list". Four such files made go install github.com/cwayne18/vexscan@latest fail for every user on every platform, before compiling anything: create zip: internal/pkgdb/testdata/debian12/var/lib/dpkg/info/libc6:amd64.list: malformed file path: invalid char ':' Store those four %3A-escaped and have debianFS materialise the fixture into a temp directory with the colons restored, so the parser still sees byte-for-byte what dpkg wrote -- the arch-qualified .list lookup is the thing TestDebReadsFileLists exists to cover, and reading the escaped name would quietly stop testing it. Add TestTrackedFilesCanGoInAModuleZip so the next one is caught here rather than by the first person to install a tag, which cannot be fixed in place once published. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 4de646a commit e389a18

6 files changed

Lines changed: 107 additions & 1 deletion

File tree

internal/pkgdb/deb_test.go

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package pkgdb
22

33
import (
4+
"io/fs"
45
"os"
56
"path/filepath"
67
"strings"
@@ -14,9 +15,48 @@ import (
1415
// between them cover every filename shape dpkg produces. The status file is
1516
// untouched: parsing a database someone else wrote is the entire job, so the
1617
// fixture is not one this code's author got to design.
18+
//
19+
// The fixture is materialised into a temp directory rather than read in place
20+
// because of the one filename shape that matters most here. dpkg writes the
21+
// multiarch lists as "libc6:amd64.list", and a module zip cannot contain a
22+
// colon: the punctuation a module file path may use is "!#$%&()+,-.=@[]^_{}~"
23+
// and nothing else, so one such file makes "go install" of this module fail
24+
// for everyone, on every platform, before a line of it is compiled. The files
25+
// are stored %3A-escaped and the colon is put back here, which keeps the name
26+
// the parser sees byte-for-byte what dpkg wrote while keeping the repository
27+
// installable. TestTrackedFilesCanGoInAModuleZip pins the rule.
1728
func debianFS(t *testing.T) target.RootFS {
1829
t.Helper()
19-
return target.NewDirFS(filepath.Join("testdata", "debian12"))
30+
return target.NewDirFS(unescapeTree(t, filepath.Join("testdata", "debian12")))
31+
}
32+
33+
// unescapeTree copies a fixture tree into a temp directory, turning %3A in any
34+
// path element back into a colon.
35+
func unescapeTree(t *testing.T, src string) string {
36+
t.Helper()
37+
dst := t.TempDir()
38+
err := filepath.WalkDir(src, func(path string, d fs.DirEntry, err error) error {
39+
if err != nil {
40+
return err
41+
}
42+
rel, err := filepath.Rel(src, path)
43+
if err != nil {
44+
return err
45+
}
46+
out := filepath.Join(dst, strings.ReplaceAll(rel, "%3A", ":"))
47+
if d.IsDir() {
48+
return os.MkdirAll(out, 0o755)
49+
}
50+
b, err := os.ReadFile(path)
51+
if err != nil {
52+
return err
53+
}
54+
return os.WriteFile(out, b, 0o644)
55+
})
56+
if err != nil {
57+
t.Fatal(err)
58+
}
59+
return dst
2060
}
2161

2262
func TestDebReadsARealDebianStatus(t *testing.T) {

internal/pkgdb/testdata/debian12/var/lib/dpkg/info/gcc-12-base:amd64.list renamed to internal/pkgdb/testdata/debian12/var/lib/dpkg/info/gcc-12-base%3Aamd64.list

File renamed without changes.

internal/pkgdb/testdata/debian12/var/lib/dpkg/info/libc6:amd64.list renamed to internal/pkgdb/testdata/debian12/var/lib/dpkg/info/libc6%3Aamd64.list

File renamed without changes.

internal/pkgdb/testdata/debian12/var/lib/dpkg/info/libcrypt1:amd64.list renamed to internal/pkgdb/testdata/debian12/var/lib/dpkg/info/libcrypt1%3Aamd64.list

File renamed without changes.

internal/pkgdb/testdata/debian12/var/lib/dpkg/info/libgcc-s1:amd64.list renamed to internal/pkgdb/testdata/debian12/var/lib/dpkg/info/libgcc-s1%3Aamd64.list

File renamed without changes.

module_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package main
2+
3+
import (
4+
"os/exec"
5+
"strings"
6+
"testing"
7+
"unicode"
8+
)
9+
10+
// modulePunct is every punctuation character a file inside a module zip may
11+
// use. The full rule is that a path element may hold Unicode letters, ASCII
12+
// digits, the ASCII space, and these -- nothing else.
13+
const modulePunct = "!#$%&()+,-.=@[]^_{}~"
14+
15+
// TestTrackedFilesCanGoInAModuleZip fails on a committed file that "go install"
16+
// cannot package.
17+
//
18+
// This is not a style rule. A module zip is built from the repository as it
19+
// stands, and one illegal character anywhere in the tree makes
20+
//
21+
// go install github.com/cwayne18/vexscan@latest
22+
//
23+
// fail for every user on every platform, with an error naming a file none of
24+
// them asked for. The tag that carries the bad file is immutable once
25+
// published, so the only remedy is a new release -- which is why this is
26+
// checked here rather than found by the first person to try installing.
27+
//
28+
// The character set is Windows-driven: a colon, an asterisk or a quote cannot
29+
// be a filename there, so the module format forbids them everywhere rather
30+
// than producing archives that unpack on some machines and not others. That is
31+
// how it caught internal/pkgdb/testdata/debian12, whose fixture is a real dpkg
32+
// database and whose multiarch file lists are genuinely named "libc6:amd64.list".
33+
// See debianFS for how that fixture keeps the real names at test time.
34+
func TestTrackedFilesCanGoInAModuleZip(t *testing.T) {
35+
out, err := exec.Command("git", "ls-files", "-z").Output()
36+
if err != nil {
37+
t.Skipf("git ls-files: %v", err)
38+
}
39+
for _, path := range strings.Split(strings.TrimRight(string(out), "\x00"), "\x00") {
40+
if path == "" {
41+
continue
42+
}
43+
for _, elem := range strings.Split(path, "/") {
44+
if bad, ok := badChar(elem); ok {
45+
t.Errorf("%s: %q cannot appear in a file inside a module zip; "+
46+
"store the file under an escaped name and restore it in the test that reads it",
47+
path, bad)
48+
break
49+
}
50+
}
51+
}
52+
}
53+
54+
// badChar returns the first character of a path element that a module zip does
55+
// not allow.
56+
func badChar(elem string) (rune, bool) {
57+
for _, r := range elem {
58+
switch {
59+
case unicode.IsLetter(r), '0' <= r && r <= '9', r == ' ':
60+
case strings.ContainsRune(modulePunct, r):
61+
default:
62+
return r, true
63+
}
64+
}
65+
return 0, false
66+
}

0 commit comments

Comments
 (0)