Skip to content

Commit 016c80f

Browse files
authored
Better parsing for www-authenticate headers (#155)
There was a bug where the `www-authenticate` header in the HTTP response from a registry would not be parsed accurately. Specifically, if the header value had more than one `<space>` character, the entire header would be ignored. This PR fixes this bug and adds unit test to detect this in the future. Fixes apple/container#240 And most likely fixes apple/container#237 Signed-off-by: Aditya Ramani <a_ramani@apple.com>
1 parent cad00de commit 016c80f

File tree

4 files changed

+73
-20
lines changed

4 files changed

+73
-20
lines changed

Sources/ContainerizationOCI/Client/RegistryClient+Token.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ struct TokenResponse: Codable, Hashable {
108108
}
109109
}
110110

111-
struct AuthenticateChallenge {
111+
struct AuthenticateChallenge: Equatable {
112112
let type: String
113113
let realm: String?
114114
let service: String?
@@ -158,7 +158,7 @@ extension RegistryClient {
158158
}
159159

160160
internal func createTokenRequest(parsing authenticateHeaders: [String]) throws -> TokenRequest {
161-
let parsedHeaders = parseWWWAuthenticateHeaders(headers: authenticateHeaders)
161+
let parsedHeaders = Self.parseWWWAuthenticateHeaders(headers: authenticateHeaders)
162162
let bearerChallenge = parsedHeaders.first { $0.type == "Bearer" }
163163
guard let bearerChallenge else {
164164
throw ContainerizationError(.invalidArgument, message: "Missing Bearer challenge in \(TokenRequest.authenticateHeaderName) header")
@@ -174,11 +174,11 @@ extension RegistryClient {
174174
return tokenRequest
175175
}
176176

177-
internal func parseWWWAuthenticateHeaders(headers: [String]) -> [AuthenticateChallenge] {
177+
internal static func parseWWWAuthenticateHeaders(headers: [String]) -> [AuthenticateChallenge] {
178178
var parsed: [String: [String: String]] = [:]
179179
for challenge in headers {
180180
let trimmedChallenge = challenge.trimmingCharacters(in: .whitespacesAndNewlines)
181-
let parts = trimmedChallenge.split(separator: " ", maxSplits: 2)
181+
let parts = trimmedChallenge.split(separator: " ", maxSplits: 1)
182182
guard parts.count == 2 else {
183183
continue
184184
}

Sources/ContainerizationOCI/Client/RegistryClient.swift

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ public final class RegistryClient: ContentClient {
179179
// The server did not tell us how to authenticate our requests,
180180
// Or we do not support scheme the server is requesting for.
181181
// Throw the 401/403 to the caller, and let them decide how to proceed.
182-
throw RegistryClient.Error.invalidStatus(url: path, _response.status)
182+
throw RegistryClient.Error.invalidStatus(url: path, _response.status, reason: String(describing: error))
183183
}
184184
if let ct = currentToken, ct.isValid(scope: tokenRequest.scope) {
185185
break
@@ -245,19 +245,8 @@ public final class RegistryClient: ContentClient {
245245
components: URLComponents,
246246
headers: [(String, String)]? = nil
247247
) async throws -> Data {
248-
try await request(components: components, method: .GET, headers: headers) { response in
249-
guard response.status == .ok else {
250-
let url = components.url?.absoluteString ?? "unknown"
251-
let reason = await ErrorResponse.fromResponseBody(response.body)?.jsonString
252-
throw Error.invalidStatus(url: url, response.status, reason: reason)
253-
}
254-
255-
var body = try await response.body.collect(upTo: self.bufferSize)
256-
guard let bytes = body.readBytes(length: body.readableBytes) else {
257-
throw ContainerizationError(.internalError, message: "Cannot read bytes from HTTP response")
258-
}
259-
return Data(bytes)
260-
}
248+
let bytes: ByteBuffer = try await requestBuffer(components: components, headers: headers)
249+
return Data(buffer: bytes)
261250
}
262251

263252
internal func requestBuffer(
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
//===----------------------------------------------------------------------===//
2+
// Copyright © 2025 Apple Inc. and the Containerization project authors. All rights reserved.
3+
//
4+
// Licensed under the Apache License, Version 2.0 (the "License");
5+
// you may not use this file except in compliance with the License.
6+
// You may obtain a copy of the License at
7+
//
8+
// https://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
//===----------------------------------------------------------------------===//
16+
17+
import Foundation
18+
import Testing
19+
20+
@testable import ContainerizationOCI
21+
22+
struct AuthChallengeTests {
23+
internal struct TestCase: Sendable {
24+
let input: String
25+
let expected: AuthenticateChallenge
26+
}
27+
28+
private static let testCases: [TestCase] = [
29+
.init(
30+
input: """
31+
Bearer realm="https://domain.io/token",service="domain.io",scope="repository:user/image:pull"
32+
""",
33+
expected: .init(type: "Bearer", realm: "https://domain.io/token", service: "domain.io", scope: "repository:user/image:pull", error: nil)),
34+
.init(
35+
input: """
36+
Bearer realm="https://foo-bar-registry.com/auth",service="Awesome Registry"
37+
""",
38+
expected: .init(type: "Bearer", realm: "https://foo-bar-registry.com/auth", service: "Awesome Registry", scope: nil, error: nil)),
39+
.init(
40+
input: """
41+
Bearer realm="users.example.com", scope="create delete"
42+
""",
43+
expected: .init(type: "Bearer", realm: "users.example.com", service: nil, scope: "create delete", error: nil)),
44+
.init(
45+
input: """
46+
Bearer realm="https://auth.server.io/token",service="registry.server.io"
47+
""",
48+
expected: .init(type: "Bearer", realm: "https://auth.server.io/token", service: "registry.server.io", scope: nil, error: nil)),
49+
50+
]
51+
52+
@Test(arguments: testCases)
53+
func parseAuthHeader(testCase: TestCase) throws {
54+
let challenges = RegistryClient.parseWWWAuthenticateHeaders(headers: [testCase.input])
55+
#expect(challenges.count == 1)
56+
#expect(challenges[0] == testCase.expected)
57+
}
58+
}

Tests/ContainerizationOCITests/RegistryClientTests.swift

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,14 @@ struct OCIClientTests: ~Copyable {
7272
#expect(response.getToken() != nil)
7373
}
7474

75-
@Test func ping() async throws {
76-
let client = RegistryClient(host: "registry-1.docker.io")
75+
@Test(arguments: [
76+
"registry-1.docker.io",
77+
"public.ecr.aws",
78+
"registry.k8s.io",
79+
"mcr.microsoft.com",
80+
])
81+
func ping(host: String) async throws {
82+
let client = RegistryClient(host: host)
7783
try await client.ping()
7884
}
7985

0 commit comments

Comments
 (0)