Skip to content

Commit c6f7b0c

Browse files
committed
- Adopt feedback.
- TestURLCredentialStorage was never built by Xcode and never executed on either build system. Fix that, and remove spurious tests.
1 parent a440a93 commit c6f7b0c

File tree

6 files changed

+76
-43
lines changed

6 files changed

+76
-43
lines changed

Diff for: Foundation.xcodeproj/project.pbxproj

+4
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
153E951120111DC500F250BE /* CFKnownLocations.h in Headers */ = {isa = PBXBuildFile; fileRef = 153E950F20111DC500F250BE /* CFKnownLocations.h */; settings = {ATTRIBUTES = (Private, ); }; };
1818
153E951220111DC500F250BE /* CFKnownLocations.c in Sources */ = {isa = PBXBuildFile; fileRef = 153E951020111DC500F250BE /* CFKnownLocations.c */; };
1919
15496CF1212CAEBA00450F5A /* CFAttributedStringPriv.h in Headers */ = {isa = PBXBuildFile; fileRef = 15496CF0212CAEBA00450F5A /* CFAttributedStringPriv.h */; };
20+
155B77AC22E63D2D00D901DE /* TestURLCredentialStorage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 155B77AB22E63D2D00D901DE /* TestURLCredentialStorage.swift */; };
2021
155D3BBC22401D1100B0D38E /* FixtureValues.swift in Sources */ = {isa = PBXBuildFile; fileRef = 155D3BBB22401D1100B0D38E /* FixtureValues.swift */; };
2122
1569BFA12187D04C009518FA /* CFCalendar_Enumerate.c in Sources */ = {isa = PBXBuildFile; fileRef = 1569BF9F2187D003009518FA /* CFCalendar_Enumerate.c */; };
2223
1569BFA22187D04F009518FA /* CFCalendar_Internal.h in Headers */ = {isa = PBXBuildFile; fileRef = 1569BF9D2187CFFF009518FA /* CFCalendar_Internal.h */; settings = {ATTRIBUTES = (Private, ); }; };
@@ -638,6 +639,7 @@
638639
153E950F20111DC500F250BE /* CFKnownLocations.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = CFKnownLocations.h; sourceTree = "<group>"; };
639640
153E951020111DC500F250BE /* CFKnownLocations.c */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.c; path = CFKnownLocations.c; sourceTree = "<group>"; };
640641
15496CF0212CAEBA00450F5A /* CFAttributedStringPriv.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CFAttributedStringPriv.h; sourceTree = "<group>"; };
642+
155B77AB22E63D2D00D901DE /* TestURLCredentialStorage.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TestURLCredentialStorage.swift; sourceTree = "<group>"; };
641643
155D3BBB22401D1100B0D38E /* FixtureValues.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FixtureValues.swift; sourceTree = "<group>"; };
642644
1569BF9D2187CFFF009518FA /* CFCalendar_Internal.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CFCalendar_Internal.h; sourceTree = "<group>"; };
643645
1569BF9F2187D003009518FA /* CFCalendar_Enumerate.c */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.c; path = CFCalendar_Enumerate.c; sourceTree = "<group>"; };
@@ -1779,6 +1781,7 @@
17791781
DAA79BD820D42C07004AF044 /* TestURLProtectionSpace.swift */,
17801782
7A7D6FBA1C16439400957E2E /* TestURLResponse.swift */,
17811783
5B1FD9E21D6D17B80080E83C /* TestURLSession.swift */,
1784+
155B77AB22E63D2D00D901DE /* TestURLCredentialStorage.swift */,
17821785
555683BC1C1250E70041D4C6 /* TestUserDefaults.swift */,
17831786
C2A9D75B1C15C08B00993803 /* TestNSUUID.swift */,
17841787
D3047AEB1C38BC3300295652 /* TestNSValue.swift */,
@@ -2912,6 +2915,7 @@
29122915
684C79011F62B611005BD73E /* TestNSNumberBridging.swift in Sources */,
29132916
DAA79BD920D42C07004AF044 /* TestURLProtectionSpace.swift in Sources */,
29142917
616068F5225DE606004FCC54 /* TestURLSessionFTP.swift in Sources */,
2918+
155B77AC22E63D2D00D901DE /* TestURLCredentialStorage.swift in Sources */,
29152919
B951B5EC1F4E2A2000D8B332 /* TestNSLock.swift in Sources */,
29162920
5B13B33A1C582D4C00651CE2 /* TestNSNumber.swift in Sources */,
29172921
5B13B3521C582D4C00651CE2 /* TestNSValue.swift in Sources */,

Diff for: Foundation/URLCredentialStorage.swift

+6
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,12 @@ open class URLCredentialStorage: NSObject {
3838
_defaultCredentials = [:]
3939
}
4040

41+
convenience init(ephemeral: Bool) {
42+
// Some URLCredentialStorages must be ephemeral, to support ephemeral URLSessions. They should not write anything to persistent storage.
43+
// All URLCredentialStorage instances are _currently_ ephemeral, so there's no need to record the value of 'ephemeral' here, but if we implement persistent storage in the future using platform secure storage, implementers of that functionality will have to heed this flag here.
44+
self.init()
45+
}
46+
4147
/*!
4248
@method credentialsForProtectionSpace:
4349
@abstract Get a dictionary mapping usernames to credentials for the specified protection space.

Diff for: Foundation/URLSession/URLSessionConfiguration.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ open class URLSessionConfiguration : NSObject, NSCopying {
150150
open class var ephemeral: URLSessionConfiguration {
151151
let ephemeralConfiguration = URLSessionConfiguration.default.copy() as! URLSessionConfiguration
152152
ephemeralConfiguration.httpCookieStorage = .ephemeralStorage()
153-
ephemeralConfiguration.urlCredentialStorage = URLCredentialStorage() // All credential storage in s-c-f are ephemeral.
153+
ephemeralConfiguration.urlCredentialStorage = URLCredentialStorage(ephemeral: true)
154154
ephemeralConfiguration.urlCache = URLCache(memoryCapacity: 4 * 1024 * 1024, diskCapacity: 0, diskPath: nil)
155155
return ephemeralConfiguration
156156
}

Diff for: Foundation/URLSession/URLSessionTask.swift

+1-2
Original file line numberDiff line numberDiff line change
@@ -721,8 +721,7 @@ extension _ProtocolClient : URLProtocolClient {
721721
}
722722

723723
if let storage = session.configuration.urlCredentialStorage,
724-
let last = task._protocolLock.performLocked({ task._lastCredentialUsedFromStorageDuringAuthentication }),
725-
last.credential.persistence != .none && last.credential.persistence != .forSession {
724+
let last = task._protocolLock.performLocked({ task._lastCredentialUsedFromStorageDuringAuthentication }) {
726725
storage.set(last.credential, for: last.protectionSpace, task: task)
727726
}
728727

Diff for: TestFoundation/TestURLCredentialStorage.swift

+63-40
Original file line numberDiff line numberDiff line change
@@ -9,39 +9,6 @@
99

1010
class TestURLCredentialStorage : XCTestCase {
1111

12-
static var allTests: [(String, (TestURLCredentialStorage) -> () throws -> Void)] {
13-
return [
14-
("test_storageStartsEmpty", test_storageStartsEmpty),
15-
("test_sessionCredentialGetsReturnedForTheRightProtectionSpace", test_sessionCredentialGetsReturnedForTheRightProtectionSpace),
16-
("test_sessionCredentialDoesNotGetReturnedForTheWrongProtectionSpace", test_sessionCredentialDoesNotGetReturnedForTheWrongProtectionSpace),
17-
("test_sessionCredentialBecomesDefaultForProtectionSpace", test_sessionCredentialBecomesDefaultForProtectionSpace),
18-
("test_sessionCredentialGetsReturnedAsDefaultIfSetAsDefaultForSpace", test_sessionCredentialGetsReturnedAsDefaultIfSetAsDefaultForSpace),
19-
("test_sessionCredentialGetsReturnedIfSetAsDefaultForSpace", test_sessionCredentialGetsReturnedIfSetAsDefaultForSpace),
20-
("test_sessionCredentialDoesNotGetReturnedIfSetAsDefaultForOtherSpace", test_sessionCredentialDoesNotGetReturnedIfSetAsDefaultForOtherSpace),
21-
("test_sessionCredentialDoesNotGetReturnedWhenNotAddedAsDefault", test_sessionCredentialDoesNotGetReturnedWhenNotAddedAsDefault),
22-
("test_sessionCredentialCanBeRemovedFromSpace", test_sessionCredentialCanBeRemovedFromSpace),
23-
("test_sessionDefaultCredentialCanBeRemovedFromSpace", test_sessionDefaultCredentialCanBeRemovedFromSpace),
24-
("test_synchronizableCredentialCanBeRemovedWithRightOptions", test_synchronizableCredentialCanBeRemovedWithRightOptions),
25-
("test_synchronizableCredentialWillNotBeRemovedWithoutRightOptions", test_synchronizableCredentialWillNotBeRemovedWithoutRightOptions),
26-
("test_storageCanRemoveArbitraryCredentialWithoutFailing", test_storageCanRemoveArbitraryCredentialWithoutFailing),
27-
("test_storageWillNotSaveCredentialsWithoutPersistence", test_storageWillNotSaveCredentialsWithoutPersistence),
28-
("test_storageWillSendNotificationWhenAddingNewCredential", test_storageWillSendNotificationWhenAddingNewCredential),
29-
("test_storageWillSendNotificationWhenAddingExistingCredentialToDifferentSpace", test_storageWillSendNotificationWhenAddingExistingCredentialToDifferentSpace),
30-
("test_storageWillNotSendNotificationWhenAddingExistingCredential", test_storageWillNotSendNotificationWhenAddingExistingCredential),
31-
("test_storageWillSendNotificationWhenAddingNewDefaultCredential", test_storageWillSendNotificationWhenAddingNewDefaultCredential),
32-
("test_storageWillNotSendNotificationWhenAddingExistingDefaultCredential", test_storageWillNotSendNotificationWhenAddingExistingDefaultCredential),
33-
("test_storageWillSendNotificationWhenAddingDifferentDefaultCredential", test_storageWillSendNotificationWhenAddingDifferentDefaultCredential),
34-
("test_storageWillSendNotificationWhenRemovingExistingCredential", test_storageWillSendNotificationWhenRemovingExistingCredential),
35-
("test_storageWillNotSendNotificationWhenRemovingExistingCredentialInOtherSpace", test_storageWillNotSendNotificationWhenRemovingExistingCredentialInOtherSpace),
36-
("test_storageWillSendNotificationWhenRemovingDefaultNotification", test_storageWillSendNotificationWhenRemovingDefaultNotification),
37-
("test_taskBasedGetCredentialsReturnsCredentialsForSpace", test_taskBasedGetCredentialsReturnsCredentialsForSpace),
38-
("test_taskBasedSetCredentialStoresGivenCredentials", test_taskBasedSetCredentialStoresGivenCredentials),
39-
("test_taskBasedRemoveCredentialDeletesCredentialsFromSpace", test_taskBasedRemoveCredentialDeletesCredentialsFromSpace),
40-
("test_taskBasedGetDefaultCredentialReturnsTheDefaultCredential", test_taskBasedGetDefaultCredentialReturnsTheDefaultCredential),
41-
("test_taskBasedSetDefaultCredentialStoresTheDefaultCredential", test_taskBasedSetDefaultCredentialStoresTheDefaultCredential),
42-
]
43-
}
44-
4512
func test_storageStartsEmpty() {
4613
let storage = URLCredentialStorage.shared
4714
XCTAssertEqual(storage.allCredentials.count, 0)
@@ -185,6 +152,12 @@ class TestURLCredentialStorage : XCTestCase {
185152
storage.remove(credential2, for: space)
186153
}
187154

155+
#if NS_FOUNDATION_NETWORKING_URLCREDENTIALSTORAGE_SYNCHRONIZABLE_ALLOWED
156+
/*
157+
swift-corelibs-foundation does not support synchronizable credentials, refusing to save them much like Darwin when logged out of iCloud.
158+
Thus, these tests cannot succeed — there is never a credential to remove.
159+
If we ever implement synchronizable credentials, uncomment this.
160+
*/
188161
func test_synchronizableCredentialCanBeRemovedWithRightOptions() {
189162
let storage = URLCredentialStorage.shared
190163

@@ -196,7 +169,7 @@ class TestURLCredentialStorage : XCTestCase {
196169
storage.remove(credential, for: space, options: [NSURLCredentialStorageRemoveSynchronizableCredentials: NSNumber(value: true)])
197170
}
198171
}
199-
172+
200173
func test_synchronizableCredentialWillNotBeRemovedWithoutRightOptions() {
201174
let storage = URLCredentialStorage.shared
202175

@@ -223,7 +196,8 @@ class TestURLCredentialStorage : XCTestCase {
223196

224197
storage.remove(credential, for: space, options: [NSURLCredentialStorageRemoveSynchronizableCredentials: NSNumber(value: true)])
225198
}
226-
199+
#endif
200+
227201
func test_storageCanRemoveArbitraryCredentialWithoutFailing() {
228202
let storage = URLCredentialStorage.shared
229203

@@ -481,13 +455,20 @@ class TestURLCredentialStorage : XCTestCase {
481455

482456
storage.set(credential, for: space, task: task)
483457

484-
guard let credentials = storage.credentials(for: space),
485-
let recovered = credentials[try credential.user.unwrapped()] else {
486-
XCTFail("Credential not found in storage")
487-
return
458+
let expectation = self.expectation(description: "Done")
459+
460+
storage.getCredentials(for: space, task: task) { credentials in
461+
guard let credentials = credentials,
462+
let user = credential.user,
463+
let recovered = credentials[user] else {
464+
XCTFail("Credential not found in storage")
465+
return
466+
}
467+
XCTAssertEqual(recovered, credential)
468+
expectation.fulfill()
488469
}
489-
XCTAssertEqual(recovered, credential)
490470

471+
waitForExpectations(timeout: 10)
491472
storage.remove(credential, for: space)
492473
}
493474

@@ -546,4 +527,46 @@ class TestURLCredentialStorage : XCTestCase {
546527

547528
storage.remove(credential, for: space)
548529
}
530+
531+
532+
static var allTests: [(String, (TestURLCredentialStorage) -> () throws -> Void)] {
533+
var tests: [(String, (TestURLCredentialStorage) -> () throws -> Void)] = [
534+
("test_storageStartsEmpty", test_storageStartsEmpty),
535+
("test_sessionCredentialGetsReturnedForTheRightProtectionSpace", test_sessionCredentialGetsReturnedForTheRightProtectionSpace),
536+
("test_sessionCredentialDoesNotGetReturnedForTheWrongProtectionSpace", test_sessionCredentialDoesNotGetReturnedForTheWrongProtectionSpace),
537+
("test_sessionCredentialBecomesDefaultForProtectionSpace", test_sessionCredentialBecomesDefaultForProtectionSpace),
538+
("test_sessionCredentialGetsReturnedAsDefaultIfSetAsDefaultForSpace", test_sessionCredentialGetsReturnedAsDefaultIfSetAsDefaultForSpace),
539+
("test_sessionCredentialGetsReturnedIfSetAsDefaultForSpace", test_sessionCredentialGetsReturnedIfSetAsDefaultForSpace),
540+
("test_sessionCredentialDoesNotGetReturnedIfSetAsDefaultForOtherSpace", test_sessionCredentialDoesNotGetReturnedIfSetAsDefaultForOtherSpace),
541+
("test_sessionCredentialDoesNotGetReturnedWhenNotAddedAsDefault", test_sessionCredentialDoesNotGetReturnedWhenNotAddedAsDefault),
542+
("test_sessionCredentialCanBeRemovedFromSpace", test_sessionCredentialCanBeRemovedFromSpace),
543+
("test_sessionDefaultCredentialCanBeRemovedFromSpace", test_sessionDefaultCredentialCanBeRemovedFromSpace),
544+
("test_storageCanRemoveArbitraryCredentialWithoutFailing", test_storageCanRemoveArbitraryCredentialWithoutFailing),
545+
("test_storageWillNotSaveCredentialsWithoutPersistence", test_storageWillNotSaveCredentialsWithoutPersistence),
546+
("test_storageWillSendNotificationWhenAddingNewCredential", test_storageWillSendNotificationWhenAddingNewCredential),
547+
("test_storageWillSendNotificationWhenAddingExistingCredentialToDifferentSpace", test_storageWillSendNotificationWhenAddingExistingCredentialToDifferentSpace),
548+
("test_storageWillNotSendNotificationWhenAddingExistingCredential", test_storageWillNotSendNotificationWhenAddingExistingCredential),
549+
("test_storageWillSendNotificationWhenAddingNewDefaultCredential", test_storageWillSendNotificationWhenAddingNewDefaultCredential),
550+
("test_storageWillNotSendNotificationWhenAddingExistingDefaultCredential", test_storageWillNotSendNotificationWhenAddingExistingDefaultCredential),
551+
("test_storageWillSendNotificationWhenAddingDifferentDefaultCredential", test_storageWillSendNotificationWhenAddingDifferentDefaultCredential),
552+
("test_storageWillSendNotificationWhenRemovingExistingCredential", test_storageWillSendNotificationWhenRemovingExistingCredential),
553+
("test_storageWillNotSendNotificationWhenRemovingExistingCredentialInOtherSpace", test_storageWillNotSendNotificationWhenRemovingExistingCredentialInOtherSpace),
554+
("test_storageWillSendNotificationWhenRemovingDefaultNotification", test_storageWillSendNotificationWhenRemovingDefaultNotification),
555+
("test_taskBasedGetCredentialsReturnsCredentialsForSpace", test_taskBasedGetCredentialsReturnsCredentialsForSpace),
556+
("test_taskBasedSetCredentialStoresGivenCredentials", test_taskBasedSetCredentialStoresGivenCredentials),
557+
("test_taskBasedRemoveCredentialDeletesCredentialsFromSpace", test_taskBasedRemoveCredentialDeletesCredentialsFromSpace),
558+
("test_taskBasedGetDefaultCredentialReturnsTheDefaultCredential", test_taskBasedGetDefaultCredentialReturnsTheDefaultCredential),
559+
("test_taskBasedSetDefaultCredentialStoresTheDefaultCredential", test_taskBasedSetDefaultCredentialStoresTheDefaultCredential),
560+
]
561+
562+
#if NS_FOUNDATION_NETWORKING_URLCREDENTIALSTORAGE_SYNCHRONIZABLE_ALLOWED
563+
// See these test methods for why they aren't added by default.
564+
tests.append(contentsOf: [
565+
("test_synchronizableCredentialCanBeRemovedWithRightOptions", test_synchronizableCredentialCanBeRemovedWithRightOptions),
566+
("test_synchronizableCredentialWillNotBeRemovedWithoutRightOptions", test_synchronizableCredentialWillNotBeRemovedWithoutRightOptions),
567+
])
568+
#endif
569+
570+
return tests
571+
}
549572
}

Diff for: TestFoundation/main.swift

+1
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@ var allTestCases = [
8585
testCase(TestURLCache.allTests),
8686
testCase(TestURLComponents.allTests),
8787
testCase(TestURLCredential.allTests),
88+
testCase(TestURLCredentialStorage.allTests),
8889
testCase(TestURLProtectionSpace.allTests),
8990
testCase(TestURLProtocol.allTests),
9091
testCase(TestNSURLRequest.allTests),

0 commit comments

Comments
 (0)