Skip to content

Commit b6898c7

Browse files
Pushkar N Kulkarniparkera
Pushkar N Kulkarni
authored andcommitted
Cherry-pick important fixes for 3.0.1 (#654)
* Test case to test TimeZone.localizedName * Allow httpAdditionalHeaders to contain Strings or NSStrings (#625) * Switch to Error from NSError for API conformance * Fix for a URLSession crash (SR-2630) * If present, pass URLRequest.httpBody to the dataTask (#633) * Set Content-Type to application/x-www-form-urlencoded for POST requests * Update NSURLSession.swift * Fix for SR-2631 and implementation of URLSession.finishTasksAndInvalidate * Allow numeric types in NSNumberFormatter.string(for:)[SR-2477] (#643) * Fix for [SR-2486] Crash in NSDictionary.description
1 parent 2eccd19 commit b6898c7

10 files changed

+142
-31
lines changed

Foundation/NSNumberFormatter.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,8 @@ open class NumberFormatter : Formatter {
8585
open func objectValue(_ string: String, range: inout NSRange) throws -> Any? { NSUnimplemented() }
8686

8787
open override func string(for obj: Any) -> String? {
88-
guard let number = obj as? NSNumber else { return nil }
88+
//we need to allow Swift's numeric types here - Int, Double et al.
89+
guard let number = _SwiftValue.store(obj) as? NSNumber else { return nil }
8990
return string(from: number)
9091
}
9192

Foundation/NSURLSession/Configuration.swift

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,17 @@ private func convertToStringString(dictionary: [AnyHashable:Any]) -> [String: St
130130
// C.f. <https://github.com/apple/swift-corelibs-foundation/pull/287>
131131
var r: [String: String] = [:]
132132
dictionary.forEach {
133-
let k = String(describing: $0.key as! NSString)
134-
let v = String(describing: $0.value as! NSString)
133+
let k = getString(from: $0.key)
134+
let v = getString(from: $0.value)
135135
r[k] = v
136136
}
137137
return r
138138
}
139+
140+
private func getString(from obj: Any) -> String {
141+
if let string = obj as? String {
142+
return string
143+
} else {
144+
return String(describing: obj as! NSString)
145+
}
146+
}

Foundation/NSURLSession/NSURLSession.swift

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,9 @@ open class URLSession : NSObject {
189189
/// This queue is used to make public attributes on `URLSessionTask` instances thread safe.
190190
/// - Note: It's a **concurrent** queue.
191191
internal let taskAttributesIsolation: DispatchQueue
192-
fileprivate let taskRegistry = URLSession._TaskRegistry()
192+
internal let taskRegistry = URLSession._TaskRegistry()
193193
fileprivate let identifier: Int32
194+
fileprivate var invalidated = false
194195

195196
/*
196197
* The shared session uses the currently set global NSURLCache,
@@ -237,7 +238,7 @@ open class URLSession : NSObject {
237238
}
238239

239240
open let delegateQueue: OperationQueue
240-
open let delegate: URLSessionDelegate?
241+
open var delegate: URLSessionDelegate?
241242
open let configuration: URLSessionConfiguration
242243

243244
/*
@@ -258,7 +259,27 @@ open class URLSession : NSObject {
258259
* session with the same identifier until URLSession:didBecomeInvalidWithError: has
259260
* been issued.
260261
*/
261-
open func finishTasksAndInvalidate() { NSUnimplemented() }
262+
open func finishTasksAndInvalidate() {
263+
//we need to return immediately
264+
workQueue.async {
265+
//don't allow creation of new tasks from this point onwards
266+
self.invalidated = true
267+
268+
//wait for running tasks to finish
269+
if !self.taskRegistry.isEmpty {
270+
let tasksCompletion = DispatchSemaphore(value: 0)
271+
self.taskRegistry.notify(on: tasksCompletion)
272+
tasksCompletion.wait()
273+
}
274+
275+
//invoke the delegate method and break the delegate link
276+
guard let sessionDelegate = self.delegate else { return }
277+
self.delegateQueue.addOperation {
278+
sessionDelegate.urlSession(self, didBecomeInvalidWithError: nil)
279+
self.delegate = nil
280+
}
281+
}
282+
}
262283

263284
/* -invalidateAndCancel acts as -finishTasksAndInvalidate, but issues
264285
* -cancel to all outstanding tasks for this session. Note task
@@ -297,7 +318,7 @@ open class URLSession : NSObject {
297318
}
298319

299320
/* Creates an upload task with the given request. The body of the request is provided from the bodyData. */
300-
open func uploadTask(with request: URLRequest, fromData bodyData: Data) -> URLSessionUploadTask {
321+
open func uploadTask(with request: URLRequest, from bodyData: Data) -> URLSessionUploadTask {
301322
let r = URLSession._Request(request)
302323
return uploadTask(with: r, body: .data(createDispatchData(bodyData)), behaviour: .callDelegate)
303324
}
@@ -367,6 +388,7 @@ fileprivate extension URLSession {
367388
///
368389
/// All public methods funnel into this one.
369390
func dataTask(with request: _Request, behaviour: _TaskRegistry._Behaviour) -> URLSessionDataTask {
391+
guard !self.invalidated else { fatalError("Session invalidated") }
370392
let r = createConfiguredRequest(from: request)
371393
let i = createNextTaskIdentifier()
372394
let task = URLSessionDataTask(session: self, request: r, taskIdentifier: i)
@@ -380,6 +402,7 @@ fileprivate extension URLSession {
380402
///
381403
/// All public methods funnel into this one.
382404
func uploadTask(with request: _Request, body: URLSessionTask._Body, behaviour: _TaskRegistry._Behaviour) -> URLSessionUploadTask {
405+
guard !self.invalidated else { fatalError("Session invalidated") }
383406
let r = createConfiguredRequest(from: request)
384407
let i = createNextTaskIdentifier()
385408
let task = URLSessionUploadTask(session: self, request: r, taskIdentifier: i, body: body)
@@ -391,6 +414,7 @@ fileprivate extension URLSession {
391414

392415
/// Create a download task
393416
func downloadTask(with request: _Request, behavior: _TaskRegistry._Behaviour) -> URLSessionDownloadTask {
417+
guard !self.invalidated else { fatalError("Session invalidated") }
394418
let r = createConfiguredRequest(from: request)
395419
let i = createNextTaskIdentifier()
396420
let task = URLSessionDownloadTask(session: self, request: r, taskIdentifier: i)
@@ -433,10 +457,10 @@ extension URLSession {
433457
*/
434458
open func uploadTask(with request: URLRequest, fromFile fileURL: URL, completionHandler: @escaping (Data?, URLResponse?, NSError?) -> Void) -> URLSessionUploadTask {
435459
let fileData = try! Data(contentsOf: fileURL)
436-
return uploadTask(with: request, fromData: fileData, completionHandler: completionHandler)
460+
return uploadTask(with: request, from: fileData, completionHandler: completionHandler)
437461
}
438462

439-
open func uploadTask(with request: URLRequest, fromData bodyData: Data?, completionHandler: @escaping (Data?, URLResponse?, NSError?) -> Void) -> URLSessionUploadTask {
463+
open func uploadTask(with request: URLRequest, from bodyData: Data?, completionHandler: @escaping (Data?, URLResponse?, NSError?) -> Void) -> URLSessionUploadTask {
440464
return uploadTask(with: _Request(request), body: .data(createDispatchData(bodyData!)), behaviour: .dataCompletionHandler(completionHandler))
441465
}
442466

Foundation/NSURLSession/NSURLSessionDelegate.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ public protocol URLSessionDelegate : NSObjectProtocol {
5353
* invalid because of a systemic error or when it has been
5454
* explicitly invalidated, in which case the error parameter will be nil.
5555
*/
56-
func urlSession(_ session: URLSession, didBecomeInvalidWithError error: NSError?)
56+
func urlSession(_ session: URLSession, didBecomeInvalidWithError error: Error?)
5757

5858
/* If implemented, when a connection level authentication challenge
5959
* has occurred, this delegate will be given the opportunity to
@@ -68,7 +68,7 @@ public protocol URLSessionDelegate : NSObjectProtocol {
6868
}
6969

7070
extension URLSessionDelegate {
71-
public func urlSession(_ session: URLSession, didBecomeInvalidWithError error: NSError?) { }
71+
public func urlSession(_ session: URLSession, didBecomeInvalidWithError error: Error?) { }
7272
public func urlSession(_ session: URLSession, didReceive challenge: URLAuthenticationChallenge, completionHandler: @escaping (URLSession.AuthChallengeDisposition, URLCredential?) -> Void) { }
7373
}
7474

@@ -118,7 +118,7 @@ public protocol URLSessionTaskDelegate : URLSessionDelegate {
118118
/* Sent as the last message related to a specific task. Error may be
119119
* nil, which implies that no error occurred and this task is complete.
120120
*/
121-
func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: NSError?)
121+
func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?)
122122
}
123123

124124
extension URLSessionTaskDelegate {
@@ -136,7 +136,7 @@ extension URLSessionTaskDelegate {
136136

137137
public func urlSession(_ session: URLSession, task: URLSessionTask, didSendBodyData bytesSent: Int64, totalBytesSent: Int64, totalBytesExpectedToSend: Int64) { }
138138

139-
public func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: NSError?) { }
139+
public func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) { }
140140
}
141141

142142
/*

Foundation/NSURLSession/NSURLSessionTask.swift

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ open class URLSessionTask : NSObject, NSCopying {
2929
fileprivate var suspendCount = 1
3030
fileprivate var easyHandle: _EasyHandle!
3131
fileprivate var totalDownloaded = 0
32-
fileprivate unowned let session: URLSessionProtocol
32+
fileprivate var session: URLSessionProtocol! //change to nil when task completes
3333
fileprivate let body: _Body
3434
fileprivate let tempFileURL: URL
3535

@@ -62,6 +62,10 @@ open class URLSessionTask : NSObject, NSCopying {
6262
}
6363
if case .taskCompleted = internalState {
6464
updateTaskState()
65+
guard let s = session as? URLSession else { fatalError() }
66+
s.workQueue.async {
67+
s.taskRegistry.remove(self)
68+
}
6569
}
6670
}
6771
}
@@ -90,9 +94,13 @@ open class URLSessionTask : NSObject, NSCopying {
9094
self.tempFileURL = URL(fileURLWithPath: fileName)
9195
super.init()
9296
}
93-
/// Create a data task, i.e. with no body
97+
/// Create a data task. If there is a httpBody in the URLRequest, use that as a parameter
9498
internal convenience init(session: URLSession, request: URLRequest, taskIdentifier: Int) {
95-
self.init(session: session, request: request, taskIdentifier: taskIdentifier, body: .none)
99+
if let bodyData = request.httpBody {
100+
self.init(session: session, request: request, taskIdentifier: taskIdentifier, body: _Body.data(createDispatchData(bodyData)))
101+
} else {
102+
self.init(session: session, request: request, taskIdentifier: taskIdentifier, body: .none)
103+
}
96104
}
97105
internal init(session: URLSession, request: URLRequest, taskIdentifier: Int, body: _Body) {
98106
self.session = session
@@ -568,6 +576,8 @@ fileprivate extension URLSessionTask {
568576
easyHandle.set(requestMethod: request.httpMethod ?? "GET")
569577
if request.httpMethod == "HEAD" {
570578
easyHandle.set(noBody: true)
579+
} else if ((request.httpMethod == "POST") && (request.value(forHTTPHeaderField: "Content-Type") == nil)) {
580+
easyHandle.set(customHeaders: ["Content-Type:application/x-www-form-urlencoded"])
571581
}
572582
}
573583
}
@@ -819,27 +829,35 @@ extension URLSessionTask {
819829
guard case .transferCompleted(response: let response, bodyDataDrain: let bodyDataDrain) = internalState else {
820830
fatalError("Trying to complete the task, but its transfer isn't complete.")
821831
}
822-
internalState = .taskCompleted
823832
self.response = response
833+
834+
//because we deregister the task with the session on internalState being set to taskCompleted
835+
//we need to do the latter after the delegate/handler was notified/invoked
824836
switch session.behaviour(for: self) {
825837
case .taskDelegate(let delegate):
826838
guard let s = session as? URLSession else { fatalError() }
827839
s.delegateQueue.addOperation {
828840
delegate.urlSession(s, task: self, didCompleteWithError: nil)
841+
self.internalState = .taskCompleted
829842
}
830843
case .noDelegate:
831-
break
844+
internalState = .taskCompleted
832845
case .dataCompletionHandler(let completion):
833846
guard case .inMemory(let bodyData) = bodyDataDrain else {
834847
fatalError("Task has data completion handler, but data drain is not in-memory.")
835848
}
849+
836850
guard let s = session as? URLSession else { fatalError() }
837-
838-
var data = Data(capacity: bodyData!.length)
839-
data.append(Data(bytes: bodyData!.bytes, count: bodyData!.length))
851+
852+
var data = Data()
853+
if let body = bodyData {
854+
data = Data(bytes: body.bytes, count: body.length)
855+
}
840856

841857
s.delegateQueue.addOperation {
842858
completion(data, response, nil)
859+
self.internalState = .taskCompleted
860+
self.session = nil
843861
}
844862
case .downloadCompletionHandler(let completion):
845863
guard case .toFile(let url, let fileHandle?) = bodyDataDrain else {
@@ -852,6 +870,8 @@ extension URLSessionTask {
852870

853871
s.delegateQueue.addOperation {
854872
completion(url, response, nil)
873+
self.internalState = .taskCompleted
874+
self.session = nil
855875
}
856876

857877
}
@@ -860,24 +880,26 @@ extension URLSessionTask {
860880
guard case .transferFailed = internalState else {
861881
fatalError("Trying to complete the task, but its transfer isn't complete / failed.")
862882
}
863-
internalState = .taskCompleted
864883
switch session.behaviour(for: self) {
865884
case .taskDelegate(let delegate):
866885
guard let s = session as? URLSession else { fatalError() }
867886
s.delegateQueue.addOperation {
868-
delegate.urlSession(s, task: self, didCompleteWithError: error)
887+
delegate.urlSession(s, task: self, didCompleteWithError: error as Error)
888+
self.internalState = .taskCompleted
869889
}
870890
case .noDelegate:
871-
break
891+
internalState = .taskCompleted
872892
case .dataCompletionHandler(let completion):
873893
guard let s = session as? URLSession else { fatalError() }
874894
s.delegateQueue.addOperation {
875895
completion(nil, nil, error)
896+
self.internalState = .taskCompleted
876897
}
877898
case .downloadCompletionHandler(let completion):
878899
guard let s = session as? URLSession else { fatalError() }
879900
s.delegateQueue.addOperation {
880901
completion(nil, nil, error)
902+
self.internalState = .taskCompleted
881903
}
882904
}
883905
}

Foundation/NSURLSession/TaskRegistry.swift

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
// -----------------------------------------------------------------------------
1818

1919
import CoreFoundation
20-
20+
import Dispatch
2121

2222
extension URLSession {
2323
/// This helper class keeps track of all tasks, and their behaviours.
@@ -45,6 +45,7 @@ extension URLSession {
4545

4646
fileprivate var tasks: [Int: URLSessionTask] = [:]
4747
fileprivate var behaviours: [Int: _Behaviour] = [:]
48+
fileprivate var tasksFinished: DispatchSemaphore?
4849
}
4950
}
5051

@@ -79,6 +80,19 @@ extension URLSession._TaskRegistry {
7980
fatalError("Trying to remove task's behaviour, but it's not in the registry.")
8081
}
8182
behaviours.remove(at: behaviourIdx)
83+
84+
guard let allTasksFinished = tasksFinished else { return }
85+
if self.isEmpty {
86+
allTasksFinished.signal()
87+
}
88+
}
89+
90+
func notify(on semaphore: DispatchSemaphore) {
91+
tasksFinished = semaphore
92+
}
93+
94+
var isEmpty: Bool {
95+
return tasks.count == 0
8296
}
8397
}
8498
extension URLSession._TaskRegistry {

Foundation/Set.swift

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,13 @@ extension Set : _ObjectTypeBridgeable {
4040
if let o = obj as? Element {
4141
set.insert(o)
4242
} else {
43-
failedConversion = true
44-
stop.pointee = true
43+
// here obj must be a swift type
44+
if let nsObject = _SwiftValue.store(obj) as? Element {
45+
set.insert(nsObject)
46+
} else {
47+
failedConversion = true
48+
stop.pointee = true
49+
}
4550
}
4651
}
4752
} else if type(of: source) == _NSCFSet.self {

TestFoundation/TestNSNumberFormatter.swift

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ class TestNSNumberFormatter: XCTestCase {
5252
("test_currencyGroupingSeparator", test_currencyGroupingSeparator),
5353
("test_lenient", test_lenient),
5454
("test_minimumSignificantDigits", test_minimumSignificantDigits),
55-
("test_maximumSignificantDigits", test_maximumSignificantDigits)
55+
("test_maximumSignificantDigits", test_maximumSignificantDigits),
56+
("test_stringFor", test_stringFor)
5657
]
5758
}
5859

@@ -341,5 +342,18 @@ class TestNSNumberFormatter: XCTestCase {
341342
let formattedString = numberFormatter.string(from: 42.42424242)
342343
XCTAssertEqual(formattedString, "42.4")
343344
}
345+
346+
func test_stringFor() {
347+
let numberFormatter = NumberFormatter()
348+
XCTAssertEqual(numberFormatter.string(for: 10)!, "10")
349+
XCTAssertEqual(numberFormatter.string(for: 3.14285714285714)!, "3")
350+
XCTAssertEqual(numberFormatter.string(for: true)!, "1")
351+
XCTAssertEqual(numberFormatter.string(for: false)!, "0")
352+
XCTAssertNil(numberFormatter.string(for: [1,2]))
353+
XCTAssertEqual(numberFormatter.string(for: NSNumber(value: 99.1))!, "99")
354+
XCTAssertNil(numberFormatter.string(for: "NaN"))
355+
XCTAssertNil(numberFormatter.string(for: NSString(string: "NaN")))
356+
}
357+
344358
}
345359

TestFoundation/TestNSSet.swift

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ class TestNSSet : XCTestCase {
3535
("test_CountedSetObjectCount", test_CountedSetObjectCount),
3636
("test_CountedSetAddObject", test_CountedSetAddObject),
3737
("test_CountedSetRemoveObject", test_CountedSetRemoveObject),
38-
("test_CountedSetCopying", test_CountedSetCopying)
38+
("test_CountedSetCopying", test_CountedSetCopying),
39+
("test_mutablesetWithDictionary", test_mutablesetWithDictionary),
3940
]
4041
}
4142

@@ -226,5 +227,13 @@ class TestNSSet : XCTestCase {
226227
XCTAssertTrue(NSArray(array: setMutableCopy.allObjects).index(of: entry) != NSNotFound)
227228
}
228229
}
229-
230+
231+
func test_mutablesetWithDictionary() {
232+
let aSet = NSMutableSet()
233+
let dictionary = NSMutableDictionary()
234+
let key = NSString(string: "Hello")
235+
aSet.add(["world": "again"])
236+
dictionary.setObject(aSet, forKey: key)
237+
XCTAssertNotNil(dictionary.description) //should not crash
238+
}
230239
}

0 commit comments

Comments
 (0)