From 1003b904dc8a5f7eefc9e9f7251c2cdfaa8cbc77 Mon Sep 17 00:00:00 2001 From: Qiu Yuzhou Date: Sat, 24 Oct 2026 03:13:47 +0800 Subject: [PATCH] test(doh): remove CI timing assumptions CI scheduling can exhaust the fallback budget or exceed the elapsed-time assertion. Separate record filtering and control deadline delivery so tests verify fallback and request cancellation directly. --- .../ConnectivityDoHResolver.swift | 28 +++-- .../Connectivity/ConnectivityDoHTests.swift | 111 +++++++++++++----- 2 files changed, 99 insertions(+), 40 deletions(-) diff --git a/ShadowsocksX-NG2/App/Application/Connectivity/ConnectivityDoHResolver.swift b/ShadowsocksX-NG2/App/Application/Connectivity/ConnectivityDoHResolver.swift index 0b182c86..9f442223 100644 --- a/ShadowsocksX-NG2/App/Application/Connectivity/ConnectivityDoHResolver.swift +++ b/ShadowsocksX-NG2/App/Application/Connectivity/ConnectivityDoHResolver.swift @@ -20,7 +20,15 @@ private struct ConnectivityDoHResponse: Decodable { } struct ConnectivityDoHResolver: Sendable { - var sessionConfiguration: @Sendable () -> URLSessionConfiguration = { .ephemeral } + var sessionConfiguration: @Sendable () -> URLSessionConfiguration = { + let configuration = URLSessionConfiguration.ephemeral + configuration.timeoutIntervalForRequest = 3 + configuration.timeoutIntervalForResource = 3 + return configuration + } + var waitForDeadline: @Sendable (ContinuousClock.Instant) async -> Void = { deadline in + try? await ContinuousClock().sleep(until: deadline) + } struct Resolution: Sendable { var addresses: [String] = [] @@ -77,8 +85,6 @@ struct ConnectivityDoHResolver: Sendable { configuration.httpCookieStorage = nil configuration.urlCredentialStorage = nil configuration.requestCachePolicy = .reloadIgnoringLocalCacheData - configuration.timeoutIntervalForRequest = 3 - configuration.timeoutIntervalForResource = 3 let session = URLSession( configuration: configuration, delegate: NoRedirects(), delegateQueue: nil) defer { session.invalidateAndCancel() } @@ -89,7 +95,7 @@ struct ConnectivityDoHResolver: Sendable { } } group.addTask { - try? await ContinuousClock().sleep(until: deadline) + await waitForDeadline(deadline) return nil } defer { group.cancelAll() } @@ -137,16 +143,20 @@ struct ConnectivityDoHResolver: Sendable { guard data.count < 65536 else { return [] } data.append(byte) } - let answer = try JSONDecoder().decode(ConnectivityDoHResponse.self, from: data) - guard answer.status == 0 else { return [] } - return (answer.answers ?? []).filter { - $0.type == type && ConnectivityAddress.family($0.data) == (type == 1 ? AF_INET : AF_INET6) - }.map(\.data) + return try Self.addresses(from: data, type: type) } catch { return [] } } + static func addresses(from data: Data, type: Int) throws -> [String] { + let answer = try JSONDecoder().decode(ConnectivityDoHResponse.self, from: data) + guard answer.status == 0 else { return [] } + return (answer.answers ?? []).filter { + $0.type == type && ConnectivityAddress.family($0.data) == (type == 1 ? AF_INET : AF_INET6) + }.map(\.data) + } + private func elapsed(since started: ContinuousClock.Instant) -> Double { let parts = started.duration(to: .now).components return Double(parts.seconds) * 1000 + Double(parts.attoseconds) / 1e15 diff --git a/ShadowsocksX-NG2/Tests/Connectivity/ConnectivityDoHTests.swift b/ShadowsocksX-NG2/Tests/Connectivity/ConnectivityDoHTests.swift index bdd17007..d6f11a2a 100644 --- a/ShadowsocksX-NG2/Tests/Connectivity/ConnectivityDoHTests.swift +++ b/ShadowsocksX-NG2/Tests/Connectivity/ConnectivityDoHTests.swift @@ -1,24 +1,41 @@ import Foundation +import Synchronization import Testing +import XCTest @testable import ShadowsocksX_NG2 @Suite(.timeLimit(.minutes(1))) struct ConnectivityDoHTests { - @Test func fallsBackToCloudflareAndAcceptsOnlyMatchingIPRecords() async { - let resolver = ConnectivityDoHResolver(sessionConfiguration: { - let configuration = URLSessionConfiguration.ephemeral - configuration.protocolClasses = [DNSAnswersProtocol.self] - return configuration - }) + @Test func fallsBackToCloudflare() async { + let resolver = ConnectivityDoHResolver( + sessionConfiguration: { + let configuration = URLSessionConfiguration.ephemeral + configuration.protocolClasses = [FallbackAnswersProtocol.self] + return configuration + }, + waitForDeadline: { _ in + let pending = AsyncStream { (_: AsyncStream.Continuation) in } + for await _ in pending {} + }) let result = await resolver.resolve( - host: "target.invalid", proxyPort: 1234, deadline: .now.advanced(by: .seconds(1))) + host: "target.invalid", proxyPort: 1234, deadline: .now.advanced(by: .seconds(60))) #expect(result.addresses == ["192.0.2.1", "2001:db8::1"]) #expect(result.trace.map(\.phase) == [.google, .cloudflare]) #expect(result.trace.map(\.status) == [.network, .success]) #expect(!result.trace.map(\.description).joined().contains("target.invalid")) } + @Test func acceptsOnlyMatchingIPRecords() throws { + let data = Data( + (#"{"Status":0,"Answer":[{"type":5,"data":"alias.invalid"},"# + + #"{"type":1,"data":"192.0.2.1"},{"type":1,"data":"2001:db8::bad"},"# + + #"{"type":1,"data":"bad.invalid"},{"type":28,"data":"2001:db8::1"},"# + + #"{"type":28,"data":"192.0.2.4"}]}"#).utf8) + #expect(try ConnectivityDoHResolver.addresses(from: data, type: 1) == ["192.0.2.1"]) + #expect(try ConnectivityDoHResolver.addresses(from: data, type: 28) == ["2001:db8::1"]) + } + @Test func familyThatExceedsDeadlineDoesNotDiscardOtherFamily() async { let events = AsyncStream<[String]?> { continuation in continuation.yield(["192.0.2.1"]) @@ -39,29 +56,52 @@ struct ConnectivityDoHTests { #expect(addresses == ["192.0.2.1", "2001:db8::1"]) } + @MainActor @Test func pendingFamiliesAreCancelledAtDeadline() async throws { - let resolver = ConnectivityDoHResolver(sessionConfiguration: { - let configuration = URLSessionConfiguration.ephemeral - configuration.protocolClasses = [PendingAnswersProtocol.self] - return configuration - }) - let started = ContinuousClock.now - do { - _ = try await resolver.query( - host: "target.invalid", endpoint: "https://resolver.invalid/resolve", proxyPort: 1234, - deadline: .now.advanced(by: .milliseconds(100))) - Issue.record("Pending requests unexpectedly resolved") - } catch { - #expect(error is ConnectivityDoHResolver.Failure) + let host = UUID().uuidString.lowercased() + ".invalid" + let started = XCTestExpectation(description: "Both address families started") + let stopped = XCTestExpectation(description: "Both address families cancelled") + started.expectedFulfillmentCount = 2 + stopped.expectedFulfillmentCount = 2 + PendingAnswersProtocol.observers.withLock { + $0[host] = { loading in + if loading { started.fulfill() } else { stopped.fulfill() } + } } - #expect(ContinuousClock.now - started >= .milliseconds(100)) - #expect(ContinuousClock.now - started < .seconds(1)) + defer { PendingAnswersProtocol.observers.withLock { $0[host] = nil } } + let deadline = AsyncStream.makeStream() + let resolver = ConnectivityDoHResolver( + sessionConfiguration: { + let configuration = URLSessionConfiguration.ephemeral + configuration.protocolClasses = [PendingAnswersProtocol.self] + return configuration + }, + waitForDeadline: { _ in + for await _ in deadline.stream {} + }) + let task = Task { + do { + _ = try await resolver.query( + host: host, endpoint: "https://\(host)/resolve", proxyPort: 1234, + deadline: .now) + Issue.record("Pending requests unexpectedly resolved") + } catch { + #expect(error is ConnectivityDoHResolver.Failure) + } + } + try await withTestTask( + task, release: { deadline.continuation.finish() }, + body: { + try await requireTestPhase([started]) + deadline.continuation.finish() + try await requireTestPhase([stopped]) + }) } } -private class DNSAnswersProtocol: URLProtocol, @unchecked Sendable { - override class func canInit(with request: URLRequest) -> Bool { true } - override class func canonicalRequest(for request: URLRequest) -> URLRequest { request } +private final class FallbackAnswersProtocol: URLProtocol, @unchecked Sendable { + override static func canInit(with request: URLRequest) -> Bool { true } + override static func canonicalRequest(for request: URLRequest) -> URLRequest { request } override func startLoading() { guard let url = request.url else { return } let type = URLComponents(url: url, resolvingAgainstBaseURL: false)? @@ -69,10 +109,8 @@ private class DNSAnswersProtocol: URLProtocol, @unchecked Sendable { let failed = url.host == "dns.google" let answer = type == "1" - ? #"{"Status":0,"Answer":[{"type":5,"data":"alias.invalid"},"# - + #"{"type":1,"data":"192.0.2.1"},{"type":1,"data":"2001:db8::bad"},"# - + #"{"type":1,"data":"bad.invalid"}]}"# - : #"{"Status":0,"Answer":[{"type":28,"data":"2001:db8::1"},{"type":1,"data":"192.0.2.4"}]}"# + ? #"{"Status":0,"Answer":[{"type":1,"data":"192.0.2.1"}]}"# + : #"{"Status":0,"Answer":[{"type":28,"data":"2001:db8::1"}]}"# let response = HTTPURLResponse( url: url, statusCode: failed ? 503 : 200, httpVersion: "HTTP/1.1", headerFields: ["Content-Type": "application/dns-json"])! @@ -83,6 +121,17 @@ private class DNSAnswersProtocol: URLProtocol, @unchecked Sendable { override func stopLoading() {} } -private final class PendingAnswersProtocol: DNSAnswersProtocol, @unchecked Sendable { - override func startLoading() {} +private final class PendingAnswersProtocol: URLProtocol, @unchecked Sendable { + static let observers = Mutex<[String: @Sendable (Bool) -> Void]>([:]) + + override static func canInit(with request: URLRequest) -> Bool { true } + override static func canonicalRequest(for request: URLRequest) -> URLRequest { request } + override func startLoading() { observe(loading: true) } + override func stopLoading() { observe(loading: false) } + + private func observe(loading: Bool) { + guard let host = request.url?.host else { return } + let observer = Self.observers.withLock { $0[host] } + observer?(loading) + } }