Skip to content

Commit 3e9d805

Browse files
rogpepperogpeppe
committed
mod/module: improve error messages for bad module path
This improves the error messages when a bad module path is encountered. Example of former message: invalid module name "github.com/MyUsername/MyRepoName": non-conforming path "github.com/MyUsername/MyRepoName" Example of new message: malformed module path "github.com/MyUsername/MyRepoName": invalid char 'M' Fixes #3022 Signed-off-by: Roger Peppe <[email protected]> Change-Id: I7981c69619ff33c6dda1523255c96cfb44b3e370 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1198136 Unity-Result: CUE porcuepine <[email protected]> Reviewed-by: Daniel Martí <[email protected]> TryBot-Result: CUEcueckoo <[email protected]>
1 parent f7e48bb commit 3e9d805

File tree

3 files changed

+48
-47
lines changed

3 files changed

+48
-47
lines changed

‎cmd/cue/cmd/testdata/script/modinit_badpath.txtar

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ cmp stderr want-stderr-3
1414
cmp stderr want-stderr-4
1515

1616
-- want-stderr-1 --
17-
malformed module path "github.com/FooBar": non-conforming path "github.com/FooBar"
17+
malformed module path "github.com/FooBar": invalid char 'F'
1818
-- want-stderr-2 --
19-
malformed module path "github.com/foo/.bar": leading dot in path element
19+
malformed module path "github.com/foo/.bar": leading '.' in path element
2020
-- want-stderr-3 --
21-
malformed module path "github.com/foo/bar.": trailing dot in path element
21+
malformed module path "github.com/foo/bar.": trailing '.' in path element
2222
-- want-stderr-4 --
23-
malformed module path "github.com/foo..bar": non-conforming path "github.com/foo..bar"
23+
malformed module path "github.com/foo..bar": path does not conform to OCI repository name restrictions; see https://github.com/opencontainers/distribution-spec/blob/HEAD/spec.md#pulling-manifests

‎mod/module/module_test.go

Lines changed: 28 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ package module
77
import (
88
"testing"
99

10-
"cuelang.org/go/internal/tdtest"
10+
"cuelang.org/go/internal/cuetest"
1111
"github.com/go-quicktest/qt"
1212
)
1313

@@ -72,19 +72,19 @@ var checkPathTests = []checkPathTest{{
7272
fileErr: `malformed file path "/x.y/z": empty path element`,
7373
}, {
7474
path: `x./z`,
75-
modErr: `trailing dot in path element`,
75+
modErr: `trailing '.' in path element`,
7676
importErr: `malformed import path "x./z": trailing dot in path element`,
7777
fileErr: `malformed file path "x./z": trailing dot in path element`,
7878
}, {
7979
path: `.x/z`,
80-
modErr: `leading dot in path element`,
80+
modErr: `leading '.' in path element`,
8181
}, {
8282
path: `-x/z`,
8383
modErr: `leading dash`,
8484
importErr: `malformed import path "-x/z": leading dash`,
8585
}, {
8686
path: `x..y/z`,
87-
modErr: `non-conforming path "x..y/z"`,
87+
modErr: `path does not conform to OCI repository name restrictions; see https://github.com/opencontainers/distribution-spec/blob/HEAD/spec.md#pulling-manifests`,
8888
}, {
8989
path: `x.y/z/../../w`,
9090
modErr: `invalid path element ".."`,
@@ -115,14 +115,14 @@ var checkPathTests = []checkPathTest{{
115115
path: `x.y/z/v2.0`,
116116
}, {
117117
path: `X.y/z`,
118-
modErr: `invalid char 'X' in first path element`,
118+
modErr: `invalid char 'X'`,
119119
}, {
120120
path: `!x.y/z`,
121121
modErr: `invalid char '!'`,
122122
importErr: `malformed import path "!x.y/z": invalid char '!'`,
123123
}, {
124124
path: `_x.y/z`,
125-
modErr: `invalid char '_' in first path element`,
125+
modErr: `leading '_' in path element`,
126126
}, {
127127
path: `x.y!/z`,
128128
modErr: `invalid char '!'`,
@@ -175,10 +175,10 @@ var checkPathTests = []checkPathTest{{
175175
importErr: `malformed import path "x.y,/z": invalid char ','`,
176176
}, {
177177
path: `x.y-/z`,
178-
modErr: `non-conforming path "x.y-/z"`,
178+
modErr: `trailing '-' in path element`,
179179
}, {
180180
path: `x.y./zt`,
181-
modErr: `trailing dot in path element`,
181+
modErr: `trailing '.' in path element`,
182182
importErr: `malformed import path "x.y./zt": trailing dot in path element`,
183183
fileErr: `malformed file path "x.y./zt": trailing dot in path element`,
184184
}, {
@@ -233,7 +233,7 @@ var checkPathTests = []checkPathTest{{
233233
importErr: `malformed import path "x.y^/z": invalid char '^'`,
234234
}, {
235235
path: `x.y_/z`,
236-
modErr: `invalid char '_' in first path element`,
236+
modErr: `trailing '_' in path element`,
237237
}, {
238238
path: "x.y`/z",
239239
modErr: "invalid char '`'",
@@ -249,7 +249,7 @@ var checkPathTests = []checkPathTest{{
249249
importErr: `malformed import path "x.y}/z": invalid char '}'`,
250250
}, {
251251
path: `x.y~/z`,
252-
modErr: `invalid char '~' in first path element`,
252+
modErr: `invalid char '~'`,
253253
}, {
254254
path: `x.y/z!`,
255255
modErr: `invalid char '!'`,
@@ -302,7 +302,7 @@ var checkPathTests = []checkPathTest{{
302302
importErr: `malformed import path "x.y/z,": invalid char ','`,
303303
}, {
304304
path: `x.y/z-`,
305-
modErr: `non-conforming path "x.y/z-"`,
305+
modErr: `trailing '-' in path element`,
306306
}, {
307307
path: `x.y/z.t`,
308308
}, {
@@ -358,7 +358,7 @@ var checkPathTests = []checkPathTest{{
358358
importErr: `malformed import path "x.y/z^": invalid char '^'`,
359359
}, {
360360
path: `x.y/z_`,
361-
modErr: `non-conforming path "x.y/z_"`,
361+
modErr: `trailing '_' in path element`,
362362
}, {
363363
path: "x.y/z`",
364364
modErr: "invalid char '`'",
@@ -374,7 +374,7 @@ var checkPathTests = []checkPathTest{{
374374
importErr: `malformed import path "x.y/z}": invalid char '}'`,
375375
}, {
376376
path: `x.y/z~`,
377-
modErr: `non-conforming path "x.y/z~"`,
377+
modErr: `invalid char '~'`,
378378
}, {
379379
path: `x.y/x.foo`,
380380
}, {
@@ -405,26 +405,26 @@ var checkPathTests = []checkPathTest{{
405405
path: `x.y/calm1`,
406406
}, {
407407
path: `x.y/z~`,
408-
modErr: `non-conforming path "x.y/z~"`,
408+
modErr: `invalid char '~'`,
409409
}, {
410410
path: `x.y/z~0`,
411-
modErr: `trailing tilde and digits in path element`,
411+
modErr: `invalid char '~'`,
412412
importErr: `malformed import path "x.y/z~0": trailing tilde and digits in path element`,
413413
}, {
414414
path: `x.y/z~09`,
415-
modErr: `trailing tilde and digits in path element`,
415+
modErr: `invalid char '~'`,
416416
importErr: `malformed import path "x.y/z~09": trailing tilde and digits in path element`,
417417
}, {
418418
path: `x.y/z09`,
419419
}, {
420420
path: `x.y/z09~`,
421-
modErr: `non-conforming path "x.y/z09~"`,
421+
modErr: `invalid char '~'`,
422422
}, {
423423
path: `x.y/z09~09z`,
424-
modErr: `non-conforming path "x.y/z09~09z"`,
424+
modErr: `invalid char '~'`,
425425
}, {
426426
path: `x.y/z09~09z~09`,
427-
modErr: `trailing tilde and digits in path element`,
427+
modErr: `invalid char '~'`,
428428
importErr: `malformed import path "x.y/z09~09z~09": trailing tilde and digits in path element`,
429429
}, {
430430
path: `github.com/!123/logrus`,
@@ -455,29 +455,29 @@ var checkPathTests = []checkPathTest{{
455455
fileErr: `malformed file path "\\temp\\foo": invalid char '\\'`,
456456
}, {
457457
path: `.gitignore`,
458-
modErr: `leading dot in path element`,
458+
modErr: `leading '.' in path element`,
459459
}, {
460460
path: `.github/ISSUE_TEMPLATE`,
461-
modErr: `leading dot in path element`,
461+
modErr: `leading '.' in path element`,
462462
}, {
463463
path: `x☺y`,
464464
modErr: `invalid char '☺'`,
465465
importErr: `malformed import path "x☺y": invalid char '☺'`,
466466
fileErr: `malformed file path "x☺y": invalid char '☺'`,
467467
}, {
468468
path: `bar.com/foo.`,
469-
modErr: `trailing dot in path element`,
469+
modErr: `trailing '.' in path element`,
470470
importErr: `malformed import path "bar.com/foo.": trailing dot in path element`,
471471
fileErr: `malformed file path "bar.com/foo.": trailing dot in path element`,
472472
}, {
473473
path: `bar.com/_foo`,
474-
modErr: `non-conforming path "bar.com/_foo"`,
474+
modErr: `leading '_' in path element`,
475475
}, {
476476
path: `bar.com/foo___x`,
477-
modErr: `non-conforming path "bar.com/foo___x"`,
477+
modErr: `path does not conform to OCI repository name restrictions; see https://github.com/opencontainers/distribution-spec/blob/HEAD/spec.md#pulling-manifests`,
478478
}, {
479479
path: `bar.com/Sushi`,
480-
modErr: `non-conforming path "bar.com/Sushi"`,
480+
modErr: `invalid char 'S'`,
481481
}, {
482482
path: `rsc io/quote`,
483483
modErr: `invalid char ' '`,
@@ -490,21 +490,21 @@ var checkPathTests = []checkPathTest{{
490490
}}
491491

492492
func TestCheckPathWithoutVersion(t *testing.T) {
493-
tdtest.Run(t, checkPathTests, func(t *tdtest.T, test *checkPathTest) {
493+
cuetest.Run(t, checkPathTests, func(t *cuetest.T, test *checkPathTest) {
494494
t.Logf("path: `%s`", test.path)
495495
t.Equal(errStr(CheckPathWithoutVersion(test.path)), test.modErr)
496496
})
497497
}
498498

499499
func TestCheckImportPath(t *testing.T) {
500-
tdtest.Run(t, checkPathTests, func(t *tdtest.T, test *checkPathTest) {
500+
cuetest.Run(t, checkPathTests, func(t *cuetest.T, test *checkPathTest) {
501501
t.Logf("path: `%s`", test.path)
502502
t.Equal(errStr(CheckImportPath(test.path)), test.importErr)
503503
})
504504
}
505505

506506
func TestCheckFilePath(t *testing.T) {
507-
tdtest.Run(t, checkPathTests, func(t *tdtest.T, test *checkPathTest) {
507+
cuetest.Run(t, checkPathTests, func(t *cuetest.T, test *checkPathTest) {
508508
t.Logf("path: `%s`", test.path)
509509
t.Equal(errStr(CheckFilePath(test.path)), test.fileErr)
510510
})

‎mod/module/path.go

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,10 @@ func firstPathOK(r rune) bool {
5353

5454
// modPathOK reports whether r can appear in a module path element.
5555
// Paths can be ASCII letters, ASCII digits, and limited ASCII punctuation: - . _ and ~.
56-
//
57-
// This matches what "go get" has historically recognized in import paths,
58-
// and avoids confusing sequences like '%20' or '+' that would change meaning
59-
// if used in a URL.
60-
//
61-
// TODO(rsc): We would like to allow Unicode letters, but that requires additional
62-
// care in the safe encoding (see "escaped paths" above).
6356
func modPathOK(r rune) bool {
6457
if r < utf8.RuneSelf {
65-
return r == '-' || r == '.' || r == '_' || r == '~' ||
58+
return r == '-' || r == '.' || r == '_' ||
6659
'0' <= r && r <= '9' ||
67-
'A' <= r && r <= 'Z' ||
6860
'a' <= r && r <= 'z'
6961
}
7062
return false
@@ -78,7 +70,10 @@ func modPathOK(r rune) bool {
7870
// otherwise-unambiguous on the command line and historically used for some
7971
// binary names (such as '++' as a suffix for compiler binaries and wrappers).
8072
func importPathOK(r rune) bool {
81-
return modPathOK(r) || r == '+'
73+
return modPathOK(r) ||
74+
r == '+' ||
75+
r == '~' ||
76+
'A' <= r && r <= 'Z'
8277
}
8378

8479
// fileNameOK reports whether r can appear in a file name.
@@ -134,7 +129,7 @@ func CheckPathWithoutVersion(basePath string) (err error) {
134129
}
135130
// Sanity check agreement with OCI specs.
136131
if !basePathPat.MatchString(basePath) {
137-
return fmt.Errorf("non-conforming path %q", basePath)
132+
return fmt.Errorf("path does not conform to OCI repository name restrictions; see https://github.com/opencontainers/distribution-spec/blob/HEAD/spec.md#pulling-manifests")
138133
}
139134
return nil
140135
}
@@ -267,10 +262,16 @@ func checkElem(elem string, kind pathKind) error {
267262
if strings.Count(elem, ".") == len(elem) {
268263
return fmt.Errorf("invalid path element %q", elem)
269264
}
270-
if elem[0] == '.' && kind == modulePath {
271-
return fmt.Errorf("leading dot in path element")
272-
}
273-
if elem[len(elem)-1] == '.' {
265+
266+
if kind == modulePath {
267+
268+
if r := rune(elem[0]); r == '.' || r == '_' || r == '-' {
269+
return fmt.Errorf("leading %q in path element", r)
270+
}
271+
if r := rune(elem[len(elem)-1]); r == '.' || r == '_' || r == '-' {
272+
return fmt.Errorf("trailing %q in path element", r)
273+
}
274+
} else if elem[len(elem)-1] == '.' {
274275
return fmt.Errorf("trailing dot in path element")
275276
}
276277
for _, r := range elem {

0 commit comments

Comments
 (0)