Skip to content

Commit

Permalink
internal/mod/modresolve: fix stripPrefix for exact match
Browse files Browse the repository at this point in the history
When a module path exactly matches the prefix that's specified for it,
the logic was not stripping the prefix as it should, resulting in
unexpected access to the wrong repository.

This CL fixes that, and also causes the config file parsing to fail when
`stripPrefix` is set and there's no repository specified in the
registry, because that would cause an exact match to use the empty
repository, which isn't allowed in the OCI spec. Technically it would be
possible to allow all modules that have the prefix but don't _exactly_
match the prefix, but that use case seems relatively limited (in
practice almost everyone seems to use a repository), and we could always
relax that restriction later if we choose.

I thought about adding a more end-to-end test in `cmd/cue` but that
turns out quite hard to do without duplicating a lot of logic which is
unit tested. Instead I manually verified that this scenario does indeed
work correctly.

Thanks to Erik Mogensen for bringing this issue to attention.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I9e2aeae599cca0b4a161b40c763262338e7007f4
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1178365
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: CUEcueckoo <cueckoo@gmail.com>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
  • Loading branch information
rogpeppe committed Mar 14, 2024
1 parent ec6cc09 commit fa65317
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 2 deletions.
13 changes: 11 additions & 2 deletions internal/mod/modresolve/resolve.go
Expand Up @@ -155,8 +155,16 @@ func (r *registryConfig) init() error {
// Shouldn't happen because default should apply.
return fmt.Errorf("empty pathEncoding")
}
if r.StripPrefix && r.PathEncoding != encPath {
return fmt.Errorf("cannot strip prefix unless using path encoding")
if r.StripPrefix {
if r.PathEncoding != encPath {
// TODO we could relax this to allow storing of naked tags
// when the module path matches exactly and hash tags
// otherwise.
return fmt.Errorf("cannot strip prefix unless using path encoding")
}
if r.repository == "" {
return fmt.Errorf("use of stripPrefix requires a non-empty repository within the registry")
}
}
return nil
}
Expand Down Expand Up @@ -388,6 +396,7 @@ func (r *resolver) ResolveToLocation(mpath, vers string) (Location, bool) {
bestMatchReg := r.cfg.DefaultRegistry
for pat, reg := range r.cfg.ModuleRegistries {
if pat == mpath {
bestMatch = pat
bestMatchReg = reg
break
}
Expand Down
17 changes: 17 additions & 0 deletions internal/mod/modresolve/resolve_test.go
Expand Up @@ -381,6 +381,11 @@ moduleRegistries: {
Repository: "repo/one/two/three",
Tag: "v0.0.1",
},
"stripped.org/bar v0.0.1": {
Host: "r3.example",
Repository: "repo",
Tag: "v0.0.1",
},
},
}, {
testName: "InvalidModulePath",
Expand Down Expand Up @@ -428,6 +433,18 @@ moduleRegistries: {
}
`,
err: `registry host "ok.com" is specified both as secure and insecure`,
}, {
testName: "StripPrefixWithNoRepo",
catchAllDefault: "c.example",
in: `
moduleRegistries: {
"a.example/foo": {
registry: "foo.example"
stripPrefix: true
}
}
`,
err: `invalid registry configuration in "a.example/foo": use of stripPrefix requires a non-empty repository within the registry`,
}}

for _, tc := range testCases {
Expand Down

0 comments on commit fa65317

Please sign in to comment.