From 5c75db9e4598c1c3eba423bcd2f8182cb05cec03 Mon Sep 17 00:00:00 2001 From: Euan Harris Date: Thu, 12 Dec 2024 14:33:29 +0000 Subject: [PATCH 1/2] ContainerRegistry: Do not force unwrap the startBlobUploadSession()'s return value URL's .appending(queryItems:) method can be used to append the digest to the upload location URL; URLComponents is not needed. .appending(queryItems:) always succeeds, so we do not need to check for nil in putBlob() and can instead throw in startBlobUploadSession if necessary. --- Sources/ContainerRegistry/Blobs.swift | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/Sources/ContainerRegistry/Blobs.swift b/Sources/ContainerRegistry/Blobs.swift index 5336168..dd694f2 100644 --- a/Sources/ContainerRegistry/Blobs.swift +++ b/Sources/ContainerRegistry/Blobs.swift @@ -28,10 +28,11 @@ public func digest(of data: D) -> String { extension RegistryClient { // Internal helper method to initiate a blob upload in 'two shot' mode - func startBlobUploadSession(repository: String) async throws -> URLComponents? { + func startBlobUploadSession(repository: String) async throws -> URL { precondition(repository.count > 0, "repository must not be an empty string") // Upload in "two shot" mode. + // See https://github.com/opencontainers/distribution-spec/blob/main/spec.md#post-then-put // - POST to obtain a session ID. // - Do not include the digest. // Response will include a 'Location' header telling us where to PUT the blob data. @@ -44,7 +45,12 @@ extension RegistryClient { guard let location = httpResponse.response.headerFields[.location] else { throw HTTPClientError.missingResponseHeader("Location") } - return URLComponents(string: location) + + guard let locationURL = URL(string: location) else { + throw RegistryClientError.invalidUploadLocation("\(location)") + } + + return locationURL } } @@ -123,14 +129,14 @@ public extension RegistryClient { precondition(repository.count > 0, "repository must not be an empty string") // Ask the server to open a session and tell us where to upload our data - var location = try await startBlobUploadSession(repository: repository)! + let location = try await startBlobUploadSession(repository: repository) // Append the digest to the upload location, as the specification requires. // The server's URL is arbitrary and might already contain query items which we must not overwrite. // The URL could even point to a different host. let digest = digest(of: data) - location.queryItems = (location.queryItems ?? []) + [URLQueryItem(name: "digest", value: "\(digest.utf8)")] - guard let uploadURL = location.url else { throw RegistryClientError.invalidUploadLocation("\(location)") } + let uploadURL = location.appending(queryItems: [.init(name: "digest", value: "\(digest.utf8)")]) + let httpResponse = try await executeRequestThrowing( // All blob uploads have Content-Type: application/octet-stream on the wire, even if mediatype is different .put(repository, url: uploadURL, contentType: "application/octet-stream"), From 39211b184065acfab06d0ec644bfcc3cc739d696 Mon Sep 17 00:00:00 2001 From: Euan Harris Date: Thu, 12 Dec 2024 14:34:16 +0000 Subject: [PATCH 2/2] ContainerRegistry: The registry may return a relative blob upload URL Motivation ---------- In the 'Post then Put' blob upload method (https://github.com/opencontainers/distribution-spec/blob/main/spec.md#post-then-put) the client starts by making a POST request asking the registry to start an upload session. The registry responds with a URL to which the client should PUT the blob. The upload location might not be provided by the registry server, allowing the registry to offload storage to a different service. Until now all registries we have encountered have returned absolute upload URLs, however GHCR returns a relative URL causing uploads to fail as reported in #43. Modifications ------------- If the registry returns a relative URL, startBlobUpload() rewrites it to be an absolute URL referring to the registry. Result ------ Images can be pushed to GHCR and other registries which return relative upload locations. Test Plan --------- Automated tests continue to pass. Tested manually with GHCR and other known registries. Fixes: #43 --- Sources/ContainerRegistry/Blobs.swift | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Sources/ContainerRegistry/Blobs.swift b/Sources/ContainerRegistry/Blobs.swift index dd694f2..d7e8251 100644 --- a/Sources/ContainerRegistry/Blobs.swift +++ b/Sources/ContainerRegistry/Blobs.swift @@ -50,6 +50,15 @@ extension RegistryClient { throw RegistryClientError.invalidUploadLocation("\(location)") } + // The location may be either an absolute URL or a relative URL + // If it is relative we need to make it absolute + guard locationURL.host != nil else { + guard let absoluteURL = URL(string: location, relativeTo: registryURL) else { + throw RegistryClientError.invalidUploadLocation("\(location)") + } + return absoluteURL + } + return locationURL } }