The S3 driver's Walk used a bare strings.HasPrefix to decide whether a
walkInfo was under the last ErrSkipDir directory. Without a trailing
"/" on the parent, a sibling whose name starts with the skipped
directory's name (e.g. "0.1.20" after "0.1.2") falsely matched and was
skipped, so its tag dir was never emitted to handleTag and the tag was
omitted from /v2/<name>/tags/list.
Replace the prefix check with isSubpath, which appends "/" to the
parent so only true descendants match. Add a unit test pinning the
sibling-with-lexical-prefix case.
The bug is S3-only because the filesystem and inmemory drivers list
directories recursively via WalkFallback and don't use this skip
mechanism. It surfaced in 3.1.0 when the tags handler switched from
tagService.All() (driver List with delimiter) to tagService.List()
(driver Walk) for pagination support.
Fixes#4891
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
Write PutContent objects as block blobs so Azure retries overwrite small objects instead of appending duplicate link contents. Delete legacy append-blob objects before uploading because Azure cannot replace an AppendBlob with a BlockBlob in place. Preserve zero-byte append compatibility and add Azure regression coverage for link-style content, legacy migration, and concurrent same-path writes.
Signed-off-by: Baptiste Girard-Carrabin <baptiste.girardcarrabin@datadoghq.com>
This PR changes the code based on the `go fix` suggestions
to modernize the codebase and keep up with the latest Go features.
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
Currently, the Azure driver always creates blobs as an AppendBlob, but in previous versions, it used to create them as BlockBlobs.
There is migration logic to handle this, but it's currently inversed, so if the blob exists as a BlockBlob, we don't delete it, and get an error back from Azure:
```
RESPONSE 409: 409 The blob type is invalid for this operation.
ERROR CODE: InvalidBlobType
```
Since the check is inversed, this also means that any operation against AppendBlobs does an extra delete -> create for no reason.
Signed-off-by: David Marby <david@dmarby.se>
PurgeUploads' Walk callback split the visited path with path.Split and
indexed file[0] immediately. path.Split returns an empty basename for
paths that end in a trailing slash - in practice this happens when an
S3 driver surfaces a bare directory (common prefix) with an empty
Key. Indexing a zero-length string then panics with
'index out of range [0] with length 0' and takes down the whole
PurgeUploads goroutine (#4713).
Guard the length before touching file[0] so a trailing-slash /
empty-basename entry is simply skipped as 'not a reserved directory',
which matches what the branch was trying to do anyway. Runtime
behaviour for every non-empty entry is unchanged.
Closes#4713
Signed-off-by: SAY-5 <SAY-5@users.noreply.github.com>
PR #4353 made MaxTags (default 1000) a hard ceiling on the `n` query
parameter — anything larger and the handler returns 400
PAGINATION_NUMBER_INVALID before the request ever reaches storage or
the proxy tag service. That broke clients like Renovate which use
n=10000 against pull-through caches. #4846 fixed a related 500 in
proxy mode but not this 400, so users reported the regression still
persisted.
The OCI distribution-spec describes pagination differently: a server
MAY return fewer than `n` results "when the total number of tags
attached to the repository is less than <int> or a Link header is
provided" — otherwise it MUST include `<int>` results. In other
words, the right answer for "client asked for more than we'll serve"
is `maxtags` results plus a Link header, not a rejection.
PAGINATION_NUMBER_INVALID isn't among the 14 error codes the spec
defines, either.
Drop the oversized-n rejection and clamp to MaxTags instead; the
existing Link-header path already handles continuation correctly.
Malformed (non-integer) and negative `n` values keep returning 400,
since the spec defines `n` as a non-negative integer and those
requests are genuinely invalid.
Verified end-to-end against registry-1.docker.io in proxy mode:
n=10000 now returns the tag list (or a clamped page with Link)
instead of 400. Also restores pre-3.1.0 behavior for Renovate-style
clients without needing proxy-specific logic.
Spec reference:
https://github.com/opencontainers/distribution-spec/blob/main/spec.md#listing-tags
Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>
- Added a dummy hash for nonexistent users to prevent timing attacks.
- Updated test cases to include a nonexistent user scenario for better coverage.
- Introduced a global dummy hash variable to streamline authentication for nonexistent users.
- Updated the authentication logic to utilize the new dummy hash for improved consistency.
- Added support for overriding the dummy hash in the access controller for testing purposes.
- Updated the authentication logic to utilize the provided dummy hash during user authentication.
- Updated test cases to use `t.TempDir()` for creating temporary htpasswd files, enhancing test isolation and cleanup.
- Simplified file reading and error handling in the `TestCreateHtpasswdFile` function.
Co-authored-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: HexMix <32300164+mnixry@users.noreply.github.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
bind proxy-mode bearer realms to the upstream trust boundary before sending credentials, move challenge filtering behind the auth manager, and use golang.org/x/net/publicsuffix for registrable-domain handling.
When proxy TTL is set to 0, `NewRegistryPullThroughCache`
skips creating a `TTLExpirationScheduler`. When `Close()`
calls `pr.scheduler.Stop()`, it causes causing a nil pointer
dereference panic.
Signed-off-by: Joonas Bergius <joonas@defenseunicorns.com>
Since version 3.0.0, the response completed log line is no longer
present for HEAD requests to manifests that return 200.
The regression is caused by the implicit handling of manifest HEAD
responses that bypass the logging middleware when returning from
`GetManifest`.
This change ensures that the logging middleware handles responses for
manifest HEAD requests by explicitly writing `StatusOK` into the
response header before returning from `GetManifest`.
Closes: https://github.com/distribution/distribution/issues/4733
Signed-off-by: Thomas Cuthbert <tom.cuthbert@elastic.co>
