Skip to content

Fix stale archive reuse after unauthorized Xcode download #487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
raisulchowdhury wants to merge 1 commit into
base: main
Choose a base branch
from
+53 −9
Open

Fix stale archive reuse after unauthorized Xcode download #487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions Sources/XcodesKit/Downloader.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public enum Downloader: Sendable {
}

private func withAria(aria2Path: Path, url: URL, to destination: Path, progressChanged: @escaping @Sendable (Progress) -> Void) async throws -> URL {
try await archiveDownloadStrategyService(aria2Path: aria2Path).download(
try await archiveDownloadStrategyService(destination: destination, aria2Path: aria2Path).download(
url: url,
destination: destination,
downloader: .aria2,
Expand All @@ -51,7 +51,7 @@ public enum Downloader: Sendable {
}

private func withUrlSession(url: URL, to destination: Path, progressChanged: @escaping @Sendable (Progress) -> Void) async throws -> URL {
try await archiveDownloadStrategyService().download(
try await archiveDownloadStrategyService(destination: destination).download(
url: url,
destination: destination,
downloader: .urlSession,
Expand All @@ -72,7 +72,7 @@ public enum Downloader: Sendable {
return true
}

private var archiveDownloadService: ArchiveDownloadService {
private func archiveDownloadService(destination: Path) -> ArchiveDownloadService {
ArchiveDownloadService(
aria2Download: Current.shell.downloadWithAria2,
urlSessionDownload: { url, destination, resumeData in
Expand All @@ -89,17 +89,24 @@ public enum Downloader: Sendable {
removeItem: { try Current.files.removeItem(at: $0) },
shouldRetry: { Self.shouldRetryDownloadError($0) },
validateResponse: { response in
try ArchiveDownloadService.validateDeveloperDownloadResponse(
response,
unauthorizedError: { XcodeInstaller.Error.unauthorized }
)
do {
try ArchiveDownloadService.validateDeveloperDownloadResponse(
response,
unauthorizedError: { XcodeInstaller.Error.unauthorized }
)
} catch {
// URLSession moves its completed response before validation. Do not leave an
// unauthorized HTML response at the archive path for a later invocation to reuse.
try? Current.files.removeItem(at: destination.url)
throw error
}
}
)
}

private func archiveDownloadStrategyService(aria2Path: Path? = nil) -> ArchiveDownloadStrategyService {
private func archiveDownloadStrategyService(destination: Path, aria2Path: Path? = nil) -> ArchiveDownloadStrategyService {
ArchiveDownloadStrategyService(
archiveDownloadService: archiveDownloadService,
archiveDownloadService: archiveDownloadService(destination: destination),
aria2Path: {
guard let aria2Path else {
throw XcodesKitError("aria2 path is unavailable.")
Expand Down
37 changes: 37 additions & 0 deletions Tests/XcodesKitTests/XcodesKitTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,43 @@ final class XcodesKitTests: XCTestCase {
XCTAssertEqual(xcodeDownloadURL.value, URL(string: "https://apple.com/xcode.xip")!)
}

func test_DownloadOrUseExistingArchive_RedownloadsAfterUnauthorizedResponse() async throws {
let archiveExists = LockedBox(false)
let downloadAttempts = LockedBox(0)
Current.files.fileExistsAtPath = { _ in archiveExists.value }
Current.files.removeItem = { _ in archiveExists.set(false) }
Current.network.downloadTask = { url, destination, _ in
archiveExists.set(true)

let attempt = downloadAttempts.increment()
let responseURL = attempt == 1
? URL(string: "https://developer.apple.com/unauthorized/")!
: url.url!
return (
Progress(),
Task {
(destination, HTTPURLResponse(url: responseURL, statusCode: 200, httpVersion: nil, headerFields: nil)!)
}
)
}

let xcode = Xcode(version: Version("0.0.0")!, url: URL(string: "https://apple.com/xcode.xip")!, filename: "mock.xip", releaseDate: nil)

do {
_ = try await xcodeInstaller.downloadOrUseExistingArchive(for: xcode, downloader: .urlSession, willInstall: false, progressChanged: { _ in })
XCTFail("Expected the unauthorized response to fail")
} catch let error as XcodeInstaller.Error {
XCTAssertEqual(error, .unauthorized)
}

XCTAssertFalse(archiveExists.value)

let archiveURL = try await xcodeInstaller.downloadOrUseExistingArchive(for: xcode, downloader: .urlSession, willInstall: false, progressChanged: { _ in })

XCTAssertEqual(archiveURL, Path.environmentApplicationSupport.join("com.robotsandpencils.xcodes").join("Xcode-0.0.0.xip").url)
XCTAssertEqual(downloadAttempts.value, 2)
}

func test_InstallLatestPrerelease_WithoutPrereleases_ThrowsNoPrereleaseVersionAvailable() async throws {
Current.files.contentsAtPath = { _ in nil }
Current.network.loadData = { request in
Expand Down