ContainerRegistry: Switch to swift-http-types (#11)

### Motivation Swift HTTP Types provides currency types for HTTP clients and servers, which can be used with URLSession and AsyncHTTPClient. Generally speaking, we try to keep Swift Container Plugin's dependencies to a minimum because each dependency increases the initial build time and also increases the chance of a version conflict with the project which is trying to use the plugin. However `swift-http-types` is quite a small package and offers some nice ergonomic improvements over `URLRequest`/`URLResponse`, such as enums for HTTP status codes. ### Modifications Switch all uses of `URLRequest`/`URLResponse` to `HTTPRequest`/`HTTPResponse` ### Result No functional change. ### Test Plan The unit and integration tests continue to pass.
apple · Oct 4, 2024 · 345cd61 · 345cd61
1 parent dd38050
commit 345cd61
Show file tree

Hide file tree

Showing 8 changed files with 106 additions and 120 deletions.
diff --git a/Package.swift b/Package.swift
@@ -26,12 +26,15 @@ let package = Package(
     dependencies: [
         .package(url: "https://github.com/apple/swift-crypto.git", "1.0.0"..<"4.0.0"),
         .package(url: "https://github.com/apple/swift-argument-parser", from: "1.3.0"),
+        .package(url: "https://github.com/apple/swift-http-types.git", from: "1.2.0"),
     ],
     targets: [
         .target(
             name: "ContainerRegistry",
             dependencies: [
                 .product(name: "Crypto", package: "swift-crypto", condition: .when(platforms: [.linux])),
+                .product(name: "HTTPTypes", package: "swift-http-types"),
+                .product(name: "HTTPTypesFoundation", package: "swift-http-types"),
                 .target(
                     name: "Basics"  // AuthorizationProvider
                 ),

diff --git a/Sources/ContainerRegistry/AuthHandler.swift b/Sources/ContainerRegistry/AuthHandler.swift
@@ -14,10 +14,8 @@
 
 import Basics
 import Foundation
-#if canImport(FoundationNetworking)
-import FoundationNetworking
-#endif
 import RegexBuilder
+import HTTPTypes
 
 struct BearerTokenResponse: Codable {
     /// An opaque Bearer token that clients should supply to
@@ -130,7 +128,7 @@ public struct AuthHandler {
     }
 
     /// Get locally-configured credentials, such as netrc or username/password, for a request
-    func localCredentials(for request: URLRequest) -> String? {
+    func localCredentials(for request: HTTPRequest) -> String? {
         guard let requestURL = request.url else { return nil }
 
         if let netrcEntry = auth?.httpAuthorizationHeader(for: requestURL) { return netrcEntry }
@@ -149,7 +147,7 @@ public struct AuthHandler {
     /// In future it could provide cached responses from previous challenges.
     /// - Parameter request: The request to authorize.
     /// - Returns: The request, with an appropriate authorization header added, or nil if no credentials are available.
-    public func auth(for request: URLRequest) -> URLRequest? { nil }
+    public func auth(for request: HTTPRequest) -> HTTPRequest? { nil }
 
     /// Add authorization to an HTTP rquest in response to a challenge from the server.
     /// - Parameters:
@@ -158,13 +156,13 @@ public struct AuthHandler {
     ///   - client: An HTTP client, used to retrieve tokens if necessary.
     /// - Returns: The request, with an appropriate authorization header added, or nil if no credentials are available.
     /// - Throws: If an error occurs while retrieving a credential.
-    public func auth(for request: URLRequest, withChallenge challenge: String, usingClient client: HTTPClient)
-        async throws -> URLRequest?
+    public func auth(for request: HTTPRequest, withChallenge challenge: String, usingClient client: HTTPClient)
+        async throws -> HTTPRequest?
     {
         if challenge.lowercased().starts(with: "basic") {
             guard let authHeader = localCredentials(for: request) else { return nil }
             var request = request
-            request.addValue(authHeader, forHTTPHeaderField: "Authorization")
+            request.headerFields[.authorization] = authHeader
             return request
 
         } else if challenge.lowercased().starts(with: "bearer") {
@@ -176,15 +174,15 @@ public struct AuthHandler {
                 challenge.dropFirst("bearer".count).trimmingCharacters(in: .whitespacesAndNewlines)
             )
             guard let challengeURL = parsedChallenge.url else { return nil }
-            var req = URLRequest(url: challengeURL)
-            if let credentials = localCredentials(for: req) {
-                req.addValue("\(credentials)", forHTTPHeaderField: "Authorization")
+            var tokenRequest = HTTPRequest(url: challengeURL)
+            if let credentials = localCredentials(for: tokenRequest) {
+                tokenRequest.headerFields[.authorization] = credentials
             }
 
-            let (data, _) = try await client.executeRequestThrowing(req, expectingStatus: 200)
+            let (data, _) = try await client.executeRequestThrowing(tokenRequest, expectingStatus: .ok)
             let tokenResponse = try JSONDecoder().decode(BearerTokenResponse.self, from: data)
             var request = request
-            request.addValue("Bearer \(tokenResponse.token)", forHTTPHeaderField: "Authorization")
+            request.headerFields[.authorization] = "Bearer \(tokenResponse.token)"
             return request
 
         } else {

diff --git a/Sources/ContainerRegistry/Blobs.swift b/Sources/ContainerRegistry/Blobs.swift
@@ -13,17 +13,14 @@
 //===----------------------------------------------------------------------===//
 
 import Foundation
+import HTTPTypes
 
 #if canImport(CryptoKit)
 import CryptoKit
 #else
 import Crypto
 #endif
 
-#if canImport(FoundationNetworking)
-import FoundationNetworking
-#endif
-
 /// Calculates the digest of a blob of data.
 /// - Parameter data: Blob of data to digest.
 /// - Returns: The blob's digest, in the format expected by the distribution protocol.
@@ -45,17 +42,20 @@ extension RegistryClient {
         // Response will include a 'Location' header telling us where to PUT the blob data.
         let httpResponse = try await executeRequestThrowing(
             .post(registryURLForPath("/v2/\(repository)/blobs/uploads/")),
-            expectingStatus: 202,  // expected response code for a two-shot upload
-            decodingErrors: [404]
+            expectingStatus: .accepted,  // expected response code for a "two-shot" upload
+            decodingErrors: [.notFound]
         )
 
-        guard let location = httpResponse.response.value(forHTTPHeaderField: "Location") else {
+        guard let location = httpResponse.response.headerFields[.location] else {
             throw HTTPClientError.missingResponseHeader("Location")
         }
         return URLComponents(string: location)
     }
 }
 
+// The spec says that Docker- prefix headers are no longer to be used, but also specifies that the registry digest is returned in this header.
+extension HTTPField.Name { static let dockerContentDigest = Self("Docker-Content-Digest")! }
+
 public extension RegistryClient {
     func blobExists(repository: String, digest: String) async throws -> Bool {
         precondition(repository.count > 0, "repository must not be an empty string")
@@ -67,7 +67,7 @@ public extension RegistryClient {
                 decodingErrors: [404]
             )
             return true
-        } catch HTTPClientError.unexpectedStatusCode(status: 404, _, _) { return false }
+        } catch HTTPClientError.unexpectedStatusCode(status: .notFound, _, _) { return false }
     }
 
     /// Fetches an unstructured blob of data from the registry.
@@ -141,14 +141,14 @@ public extension RegistryClient {
             // All blob uploads have Content-Type: application/octet-stream on the wire, even if mediatype is different
             .put(uploadURL, contentType: "application/octet-stream"),
             uploading: data,
-            expectingStatus: 201,
-            decodingErrors: [400, 404]
+            expectingStatus: .created,
+            decodingErrors: [.badRequest, .notFound]
         )
 
         // The registry could compute a different digest and we should use its value
         // as the canonical digest for linking blobs.   If the registry sends a digest we
         // should check that it matches our locally-calculated digest.
-        if let serverDigest = httpResponse.response.value(forHTTPHeaderField: "Docker-Content-Digest") {
+        if let serverDigest = httpResponse.response.headerFields[.dockerContentDigest] {
             assert(digest == serverDigest)
         }
         return .init(mediaType: mediaType, digest: digest, size: Int64(data.count))

diff --git a/Sources/ContainerRegistry/CheckAPI.swift b/Sources/ContainerRegistry/CheckAPI.swift
@@ -20,8 +20,11 @@ public extension RegistryClient {
         // The registry may require authentication on this endpoint.
         // See https://github.com/opencontainers/distribution-spec/blob/main/spec.md#determining-support
         do {
-            return try await executeRequestThrowing(.get(registryURLForPath("/v2/")), decodingErrors: [401, 404]).data
-                == EmptyObject()
-        } catch HTTPClientError.unexpectedStatusCode(status: 404, _, _) { return false }
+            return try await executeRequestThrowing(
+                .get(registryURLForPath("/v2/")),
+                decodingErrors: [.unauthorized, .notFound]
+            )
+            .data == EmptyObject()
+        } catch HTTPClientError.unexpectedStatusCode(status: .notFound, _, _) { return false }
     }
 }