From 552b1526c6821a84daab48cbc7f5456ae215d6c4 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 9 Nov 2022 15:20:23 +0100 Subject: [PATCH] reference: check for prefix instead of splitting, and use consts - use strings.HasPrefix() to check for the prefix we're interested in instead of doing a strings.Split() without limits. This makes the code both easier to read, and prevents potential situations where we end up with a long slice. - use consts for defaults; these should never be modified, so better to use consts for them to indicate they're fixed values. Signed-off-by: Sebastiaan van Stijn --- reference/normalize.go | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/reference/normalize.go b/reference/normalize.go index 9a23ffa9d..47de5850a 100644 --- a/reference/normalize.go +++ b/reference/normalize.go @@ -7,10 +7,10 @@ import ( "github.com/opencontainers/go-digest" ) -var ( +const ( legacyDefaultDomain = "index.docker.io" defaultDomain = "docker.io" - officialRepoName = "library" + officialRepoPrefix = "library/" defaultTag = "latest" ) @@ -96,8 +96,13 @@ func splitDockerDomain(name string) (domain, remainder string) { if domain == legacyDefaultDomain { domain = defaultDomain } + // TODO(thaJeztah): this check may be too strict, as it assumes the + // "library/" namespace does not have nested namespaces. While this + // is true (currently), technically it would be possible for Docker + // Hub to use those (e.g. "library/distros/ubuntu:latest"). + // See https://github.com/distribution/distribution/pull/3769#issuecomment-1302031785. if domain == defaultDomain && !strings.ContainsRune(remainder, '/') { - remainder = officialRepoName + "/" + remainder + remainder = officialRepoPrefix + remainder } return } @@ -117,8 +122,15 @@ func familiarizeName(named namedRepository) repository { if repo.domain == defaultDomain { repo.domain = "" // Handle official repositories which have the pattern "library/" - if split := strings.Split(repo.path, "/"); len(split) == 2 && split[0] == officialRepoName { - repo.path = split[1] + if strings.HasPrefix(repo.path, officialRepoPrefix) { + // TODO(thaJeztah): this check may be too strict, as it assumes the + // "library/" namespace does not have nested namespaces. While this + // is true (currently), technically it would be possible for Docker + // Hub to use those (e.g. "library/distros/ubuntu:latest"). + // See https://github.com/distribution/distribution/pull/3769#issuecomment-1302031785. + if remainder := strings.TrimPrefix(repo.path, officialRepoPrefix); !strings.ContainsRune(remainder, '/') { + repo.path = remainder + } } } return repo