Skip to content

Commit c8a7319

Browse files
samthanawallasamthanawalla
committed
x/mod: fix handling of vendored packages with '/vendor' in non-top-level paths
This CL address a bug in the handling of vendored packages where the '/vendor' element appears in non-top level import paths within a module zip file. This issue arose from an incorrect offset calculation that caused non-vendored packages to be incorrectly identified as vendored. This CL corrects the offset calculation for Go 1.24 and beyond. Fixes golang/go#37397 Change-Id: Ibf1751057e8383c7b82f1622674204597e735b7c Reviewed-on: https://go-review.googlesource.com/c/mod/+/619175 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Michael Matloob <[email protected]>
1 parent 9cd0e4c commit c8a7319

12 files changed

+67
-35
lines changed

‎zip/testdata/check_dir/various.txt

+2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ $work/vendor/modules.txt
66
omitted:
77
$work/.hg_archival.txt: file is inserted by 'hg archive' and is always omitted
88
$work/.git: directory is a version control repository
9+
$work/pkg/vendor/vendor.go: file is in vendor directory
910
$work/sub: directory is in another module
1011
$work/vendor/x/y: file is in vendor directory
1112

@@ -20,3 +21,4 @@ $work/invalid.go': malformed file path "invalid.go'": invalid char '\''
2021
-- sub/go.mod --
2122
-- .hg_archival.txt --
2223
-- .git/x --
24+
-- pkg/vendor/vendor.go --

‎zip/testdata/check_dir/various_go123.txt

+3-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ $work/vendor/modules.txt
77
omitted:
88
$work/.hg_archival.txt: file is inserted by 'hg archive' and is always omitted
99
$work/.git: directory is a version control repository
10+
$work/pkg/vendor/vendor.go: file is in vendor directory
1011
$work/sub: directory is in another module
1112
$work/vendor/x/y: file is in vendor directory
1213

@@ -20,4 +21,5 @@ go 1.23
2021
-- vendor/x/y --
2122
-- sub/go.mod --
2223
-- .hg_archival.txt --
23-
-- .git/x --
24+
-- .git/x --
25+
-- pkg/vendor/vendor.go --

‎zip/testdata/check_dir/various_go124.txt

+2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
-- want --
22
valid:
33
$work/go.mod
4+
$work/pkg/vendor/vendor.go
45
$work/valid.go
56

67
omitted:
@@ -22,3 +23,4 @@ go 1.24
2223
go 1.23
2324
-- .hg_archival.txt --
2425
-- .git/x --
26+
-- pkg/vendor/vendor.go --

‎zip/testdata/check_files/various.txt

+2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ omitted:
77
vendor/x/y: file is in vendor directory
88
sub/go.mod: file is in another module
99
.hg_archival.txt: file is inserted by 'hg archive' and is always omitted
10+
pkg/vendor/vendor.go: file is in vendor directory
1011

1112
invalid:
1213
not/../clean: file path is not clean
@@ -25,3 +26,4 @@ valid.go: multiple entries for file "valid.go"
2526
duplicate
2627
-- valid.go --
2728
another duplicate
29+
-- pkg/vendor/vendor.go --

‎zip/testdata/check_files/various_go123.txt

+2
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ omitted:
88
vendor/x/y: file is in vendor directory
99
sub/go.mod: file is in another module
1010
.hg_archival.txt: file is inserted by 'hg archive' and is always omitted
11+
pkg/vendor/vendor.go: file is in vendor directory
1112

1213
invalid:
1314
not/../clean: file path is not clean
@@ -26,3 +27,4 @@ go 1.23
2627
duplicate
2728
-- valid.go --
2829
another duplicate
30+
-- pkg/vendor/vendor.go --

‎zip/testdata/check_files/various_go124.txt

+2
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
valid:
33
valid.go
44
go.mod
5+
pkg/vendor/vendor.go
56

67
omitted:
78
vendor/x/y: file is in vendor directory
@@ -27,3 +28,4 @@ go 1.23
2728
duplicate
2829
-- valid.go --
2930
another duplicate
31+
-- pkg/vendor/vendor.go --

‎zip/testdata/create/exclude_vendor.txt

+3
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,6 @@ see comment in isVendoredPackage and golang.org/issue/31562.
1212
excluded
1313
-- sub/vendor/sub.txt --
1414
excluded
15+
-- pkg/vendor/vendor.go --
16+
excluded
17+
see comment in isVendoredPackage and golang.org/issue/37397

‎zip/testdata/create/exclude_vendor_go124.txt

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
path=example.com/m
22
version=v1.0.0
3-
hash=h1:WxQ7ERgHLILawPCiuSwXp5UHul7CUSRRftO8b7GTnV8=
3+
hash=h1:mJR1q75yiMK6+CDw6DuhMS4v4hNoLXqkyk2Cph4VS8Q=
44
-- go.mod --
55
module example.com/m
66

@@ -13,3 +13,6 @@ see comment in isVendoredPackage and golang.org/issue/31562.
1313
excluded
1414
-- sub/vendor/sub.txt --
1515
excluded
16+
-- pkg/vendor/vendor.go --
17+
included
18+
see comment in isVendoredPackage and golang.org/issue/37397

‎zip/testdata/create_from_dir/exclude_vendor.txt

+3
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,6 @@ see comment in isVendoredPackage and golang.org/issue/31562.
1212
excluded
1313
-- sub/vendor/sub.txt --
1414
excluded
15+
-- pkg/vendor/vendor.go --
16+
excluded
17+
see comment in isVendoredPackage and golang.org/issue/37397

‎zip/testdata/create_from_dir/exclude_vendor_go124.txt

+4-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
path=example.com/m
22
version=v1.0.0
3-
hash=h1:WxQ7ERgHLILawPCiuSwXp5UHul7CUSRRftO8b7GTnV8=
3+
hash=h1:mJR1q75yiMK6+CDw6DuhMS4v4hNoLXqkyk2Cph4VS8Q=
44
-- go.mod --
55
module example.com/m
66

@@ -13,3 +13,6 @@ see comment in isVendoredPackage and golang.org/issue/31562.
1313
excluded
1414
-- sub/vendor/sub.txt --
1515
excluded
16+
-- pkg/vendor/vendor.go --
17+
included
18+
see comment in isVendoredPackage and golang.org/issue/37397

‎zip/vendor_test.go

+20-23
Original file line numberDiff line numberDiff line change
@@ -18,43 +18,40 @@ var pre124 []string = []string{
1818
"go1.9",
1919
}
2020

21+
var after124 []string = []string{"go1.24.0", "go1.24", "go1.99.0"}
22+
23+
var allVers []string = append(pre124, after124...)
24+
2125
func TestIsVendoredPackage(t *testing.T) {
2226
for _, tc := range []struct {
23-
path string
24-
want bool
25-
falsePositive bool // is this case affected by https://golang.org/issue/37397?
26-
versions []string
27+
path string
28+
want bool
29+
versions []string
2730
}{
28-
{path: "vendor/foo/foo.go", want: true, versions: pre124},
29-
{path: "pkg/vendor/foo/foo.go", want: true, versions: pre124},
30-
{path: "longpackagename/vendor/foo/foo.go", want: true, versions: pre124},
31-
{path: "vendor/vendor.go", want: false, versions: pre124},
32-
{path: "vendor/foo/modules.txt", want: true, versions: pre124},
33-
{path: "modules.txt", want: false, versions: pre124},
34-
{path: "vendor/amodules.txt", want: false, versions: pre124},
31+
{path: "vendor/foo/foo.go", want: true, versions: allVers},
32+
{path: "pkg/vendor/foo/foo.go", want: true, versions: allVers},
33+
{path: "longpackagename/vendor/foo/foo.go", want: true, versions: allVers},
34+
{path: "vendor/vendor.go", want: false, versions: allVers},
35+
{path: "vendor/foo/modules.txt", want: true, versions: allVers},
36+
{path: "modules.txt", want: false, versions: allVers},
37+
{path: "vendor/amodules.txt", want: false, versions: allVers},
3538

3639
// These test cases were affected by https://golang.org/issue/63395
3740
{path: "vendor/modules.txt", want: false, versions: pre124},
38-
{path: "vendor/modules.txt", want: true, versions: []string{"go1.24.0", "go1.24", "go1.99.0"}},
41+
{path: "vendor/modules.txt", want: true, versions: after124},
3942

40-
// We ideally want these cases to be false, but they are affected by
41-
// https://golang.org/issue/37397, and if we fix them we will invalidate
42-
// existing module checksums. We must leave them as-is-for now.
43-
{path: "pkg/vendor/vendor.go", falsePositive: true},
44-
{path: "longpackagename/vendor/vendor.go", falsePositive: true},
43+
// These test cases were affected by https://golang.org/issue/37397
44+
{path: "pkg/vendor/vendor.go", want: true, versions: pre124},
45+
{path: "pkg/vendor/vendor.go", want: false, versions: after124},
46+
{path: "longpackagename/vendor/vendor.go", want: true, versions: pre124},
47+
{path: "longpackagename/vendor/vendor.go", want: false, versions: after124},
4548
} {
4649
for _, v := range tc.versions {
4750
got := isVendoredPackage(tc.path, v)
4851
want := tc.want
49-
if tc.falsePositive {
50-
want = true
51-
}
5252
if got != want {
5353
t.Errorf("isVendoredPackage(%q, %s) = %t; want %t", tc.path, v, got, tc.want)
5454
}
55-
if tc.falsePositive {
56-
t.Logf("(Expected a false-positive due to https://golang.org/issue/37397.)")
57-
}
5855
}
5956
}
6057
}

‎zip/zip.go

+20-9
Original file line numberDiff line numberDiff line change
@@ -786,8 +786,6 @@ func (fi dataFileInfo) Sys() interface{} { return nil }
786786
// in a package whose import path contains (but does not end with) the component
787787
// "vendor".
788788
//
789-
// Unfortunately, isVendoredPackage reports false positives for files in any
790-
// non-top-level package whose import path ends in "vendor".
791789
// The 'vers' parameter specifies the Go version declared in the module's
792790
// go.mod file and must be a valid Go version according to the
793791
// go/version.IsValid function.
@@ -803,13 +801,27 @@ func isVendoredPackage(name string, vers string) bool {
803801
if strings.HasPrefix(name, "vendor/") {
804802
i += len("vendor/")
805803
} else if j := strings.Index(name, "/vendor/"); j >= 0 {
806-
// This offset looks incorrect; this should probably be
804+
// Calculate the correct starting position within the import path
805+
// to determine if a package is vendored.
807806
//
808-
// i = j + len("/vendor/")
807+
// Due to a bug in Go versions before 1.24
808+
// (see https://golang.org/issue/37397), the "/vendor/" prefix within
809+
// a package path was not always correctly interpreted.
809810
//
810-
// (See https://golang.org/issue/31562 and https://golang.org/issue/37397.)
811-
// Unfortunately, we can't fix it without invalidating module checksums.
812-
i += len("/vendor/")
811+
// This bug affected how vendored packages were identified in cases like:
812+
//
813+
// - "pkg/vendor/vendor.go" (incorrectly identified as vendored in pre-1.24)
814+
// - "pkg/vendor/foo/foo.go" (correctly identified as vendored)
815+
//
816+
// To correct this, in Go 1.24 and later, we skip the entire "/vendor/" prefix
817+
// when it's part of a nested package path (as in the first example above).
818+
// In earlier versions, we only skipped the length of "/vendor/", leading
819+
// to the incorrect behavior.
820+
if version.Compare(vers, "go1.24") >= 0 {
821+
i = j + len("/vendor/")
822+
} else {
823+
i += len("/vendor/")
824+
}
813825
} else {
814826
return false
815827
}
@@ -950,8 +962,7 @@ func listFilesInDir(dir string) (files []File, omitted []FileError, err error) {
950962
}
951963
slashPath := filepath.ToSlash(relPath)
952964

953-
// Skip some subdirectories inside vendor, but maintain bug
954-
// golang.org/issue/31562, described in isVendoredPackage.
965+
// Skip some subdirectories inside vendor.
955966
// We would like Create and CreateFromDir to produce the same result
956967
// for a set of files, whether expressed as a directory tree or zip.
957968
if isVendoredPackage(slashPath, vers) {

0 commit comments

Comments
 (0)