From 880732e30c9e461b522595fff21e49c6bbd7cb2a Mon Sep 17 00:00:00 2001 From: Simon Evans Date: Sun, 27 May 2018 17:41:20 +0100 Subject: [PATCH 1/9] SR-7620: Use charset encoding if available for NSString(contentsOf:usedEncoding:) --- .../project.pbxproj | 9 +++++ Foundation.xcodeproj/project.pbxproj | 4 +++ Foundation/NSData.swift | 20 +++++++---- Foundation/NSString.swift | 14 +++++--- Foundation/StringEncodings.swift | 34 +++++++++++++++++++ TestFoundation/HTTPServer.swift | 11 ++++++ .../Resources/NSString-ISO-8859-1-data.txt | 3 ++ TestFoundation/TestNSString.swift | 12 ++++++- build.py | 3 +- 9 files changed, 96 insertions(+), 14 deletions(-) create mode 100644 TestFoundation/Resources/NSString-ISO-8859-1-data.txt diff --git a/DarwinCompatibilityTests.xcodeproj/project.pbxproj b/DarwinCompatibilityTests.xcodeproj/project.pbxproj index 8c6eebfb7d..36ac983860 100644 --- a/DarwinCompatibilityTests.xcodeproj/project.pbxproj +++ b/DarwinCompatibilityTests.xcodeproj/project.pbxproj @@ -7,6 +7,9 @@ objects = { /* Begin PBXBuildFile section */ + B907F36F20BB188800013CBE /* NSString-ISO-8859-1-data.txt in Resources */ = {isa = PBXBuildFile; fileRef = B907F36E20BB188800013CBE /* NSString-ISO-8859-1-data.txt */; }; + B917D32420B0DB9700728EE0 /* Foundation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = B917D32320B0DB9700728EE0 /* Foundation.framework */; }; + B917D32620B0DE2000728EE0 /* main.swift in Sources */ = {isa = PBXBuildFile; fileRef = B917D32520B0DE2000728EE0 /* main.swift */; }; B95788861F6FB9470003EB01 /* TestNSNumberBridging.swift in Sources */ = {isa = PBXBuildFile; fileRef = B95788851F6FB9470003EB01 /* TestNSNumberBridging.swift */; }; B9C89ED21F6BF67C00087AF4 /* XCTest.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = B9C89ED11F6BF67C00087AF4 /* XCTest.framework */; }; B9C89F361F6BF89C00087AF4 /* TestScanner.swift in Sources */ = {isa = PBXBuildFile; fileRef = B9C89EE61F6BF88F00087AF4 /* TestScanner.swift */; }; @@ -123,6 +126,10 @@ /* End PBXCopyFilesBuildPhase section */ /* Begin PBXFileReference section */ + B907F36E20BB188800013CBE /* NSString-ISO-8859-1-data.txt */ = {isa = PBXFileReference; lastKnownFileType = text; name = "NSString-ISO-8859-1-data.txt"; path = "TestFoundation/Resources/NSString-ISO-8859-1-data.txt"; sourceTree = ""; }; + B917D31C20B0DB8B00728EE0 /* xdgTestHelper */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.executable"; includeInIndex = 0; path = xdgTestHelper; sourceTree = BUILT_PRODUCTS_DIR; }; + B917D32320B0DB9700728EE0 /* Foundation.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Foundation.framework; path = System/Library/Frameworks/Foundation.framework; sourceTree = SDKROOT; }; + B917D32520B0DE2000728EE0 /* main.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = main.swift; path = TestFoundation/xdgTestHelper/main.swift; sourceTree = ""; }; B95788851F6FB9470003EB01 /* TestNSNumberBridging.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; name = TestNSNumberBridging.swift; path = TestFoundation/TestNSNumberBridging.swift; sourceTree = ""; }; B9C89EC11F6BF47D00087AF4 /* DarwinCompatibilityTests */ = {isa = PBXFileReference; explicitFileType = "compiled.mach-o.executable"; includeInIndex = 0; path = DarwinCompatibilityTests; sourceTree = BUILT_PRODUCTS_DIR; }; B9C89ED11F6BF67C00087AF4 /* XCTest.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = XCTest.framework; path = Platforms/MacOSX.platform/Developer/Library/Frameworks/XCTest.framework; sourceTree = DEVELOPER_DIR; }; @@ -269,6 +276,7 @@ B9C89FAA1F6DCAE700087AF4 /* NSString-UTF16-LE-data.txt */, B9C89FB01F6DCAE900087AF4 /* NSString-UTF32-BE-data.txt */, B9C89FA51F6DCAE500087AF4 /* NSString-UTF32-LE-data.txt */, + B907F36E20BB188800013CBE /* NSString-ISO-8859-1-data.txt */, B9C89FAE1F6DCAE800087AF4 /* NSStringTestData.txt */, B9C89FB21F6DCAE900087AF4 /* NSURLTestData.plist */, B9C89FB61F6DCAEA00087AF4 /* NSXMLDocumentTestData.xml */, @@ -467,6 +475,7 @@ isa = PBXResourcesBuildPhase; buildActionMask = 2147483647; files = ( + B907F36F20BB188800013CBE /* NSString-ISO-8859-1-data.txt in Resources */, B9C89FBA1F6DCAEB00087AF4 /* NSString-UTF32-LE-data.txt in Resources */, B9C89FBB1F6DCAEB00087AF4 /* NSKeyedUnarchiver-EdgeInsetsTest.plist in Resources */, B9C89FBC1F6DCAEB00087AF4 /* NSKeyedUnarchiver-ConcreteValueTest.plist in Resources */, diff --git a/Foundation.xcodeproj/project.pbxproj b/Foundation.xcodeproj/project.pbxproj index 46817e8365..ccfa16a6a0 100644 --- a/Foundation.xcodeproj/project.pbxproj +++ b/Foundation.xcodeproj/project.pbxproj @@ -330,6 +330,7 @@ 9F0DD3571ECD783500F68030 /* SwiftFoundation.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 5B5D885D1BBC938800234F36 /* SwiftFoundation.framework */; }; A058C2021E529CF100B07AA1 /* TestMassFormatter.swift in Sources */ = {isa = PBXBuildFile; fileRef = A058C2011E529CF100B07AA1 /* TestMassFormatter.swift */; }; AE35A1861CBAC85E0042DB84 /* SwiftFoundation.h in Headers */ = {isa = PBXBuildFile; fileRef = AE35A1851CBAC85E0042DB84 /* SwiftFoundation.h */; settings = {ATTRIBUTES = (Public, ); }; }; + B907F36B20BB07A700013CBE /* NSString-ISO-8859-1-data.txt in Resources */ = {isa = PBXBuildFile; fileRef = B907F36A20BB07A700013CBE /* NSString-ISO-8859-1-data.txt */; }; B90C57BB1EEEEA5A005208AE /* TestFileManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 525AECEB1BF2C96400D15BB0 /* TestFileManager.swift */; }; B90C57BC1EEEEA5A005208AE /* TestThread.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5E5835F31C20C9B500C81317 /* TestThread.swift */; }; B910957A1EEF237800A71930 /* NSString-UTF16-LE-data.txt in Resources */ = {isa = PBXBuildFile; fileRef = B91095781EEF237800A71930 /* NSString-UTF16-LE-data.txt */; }; @@ -814,6 +815,7 @@ A5A34B551C18C85D00FD972B /* TestByteCountFormatter.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TestByteCountFormatter.swift; sourceTree = ""; }; AE35A1851CBAC85E0042DB84 /* SwiftFoundation.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SwiftFoundation.h; sourceTree = ""; }; B167A6641ED7303F0040B09A /* README.md */ = {isa = PBXFileReference; lastKnownFileType = net.daringfireball.markdown; path = README.md; sourceTree = ""; }; + B907F36A20BB07A700013CBE /* NSString-ISO-8859-1-data.txt */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = "NSString-ISO-8859-1-data.txt"; sourceTree = ""; }; B91095781EEF237800A71930 /* NSString-UTF16-LE-data.txt */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = "NSString-UTF16-LE-data.txt"; sourceTree = ""; }; B91095791EEF237800A71930 /* NSString-UTF16-BE-data.txt */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = "NSString-UTF16-BE-data.txt"; sourceTree = ""; }; B933A79C1F3055F600FE6846 /* NSString-UTF32-BE-data.txt */ = {isa = PBXFileReference; lastKnownFileType = text; path = "NSString-UTF32-BE-data.txt"; sourceTree = ""; }; @@ -1460,6 +1462,7 @@ B91095791EEF237800A71930 /* NSString-UTF16-BE-data.txt */, B933A79C1F3055F600FE6846 /* NSString-UTF32-BE-data.txt */, B933A79D1F3055F600FE6846 /* NSString-UTF32-LE-data.txt */, + B907F36A20BB07A700013CBE /* NSString-ISO-8859-1-data.txt */, 528776181BF27D9500CB0090 /* Test.plist */, EA66F63B1BF1619600136161 /* NSURLTestData.plist */, E1A3726E1C31EBFB0023AF4D /* NSXMLDocumentTestData.xml */, @@ -2153,6 +2156,7 @@ D3E8D6D51C36AC0C00295652 /* NSKeyedUnarchiver-RectTest.plist in Resources */, D3A597F81C3415CC00295652 /* NSKeyedUnarchiver-URLTest.plist in Resources */, D3E8D6D31C36982700295652 /* NSKeyedUnarchiver-EdgeInsetsTest.plist in Resources */, + B907F36B20BB07A700013CBE /* NSString-ISO-8859-1-data.txt in Resources */, D370696E1C394FBF00295652 /* NSKeyedUnarchiver-RangeTest.plist in Resources */, D3A597F71C3415CC00295652 /* NSKeyedUnarchiver-ArrayTest.plist in Resources */, CE19A88C1C23AA2300B4CB6A /* NSStringTestData.txt in Resources */, diff --git a/Foundation/NSData.swift b/Foundation/NSData.swift index 176a3d0075..48e9d192db 100644 --- a/Foundation/NSData.swift +++ b/Foundation/NSData.swift @@ -185,24 +185,28 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding { /// Initializes a data object with the data from the location specified by a given URL. public init(contentsOf url: URL, options readOptionsMask: ReadingOptions = []) throws { super.init() - try _contentsOf(url: url, options: readOptionsMask) + let (data, _) = try NSData.contentsOf(url: url, options: readOptionsMask) + _init(bytes: UnsafeMutableRawPointer(mutating: data.bytes), length: data.length, copy: true) } /// Initializes a data object with the data from the location specified by a given URL. public init?(contentsOf url: URL) { super.init() do { - try _contentsOf(url: url) + let (data, _) = try NSData.contentsOf(url: url) + _init(bytes: UnsafeMutableRawPointer(mutating: data.bytes), length: data.length, copy: true) } catch { return nil } } - /// Initializes a data object with the data from the location specified by a given URL. - private func _contentsOf(url: URL, options readOptionsMask: ReadingOptions = []) throws { + internal static func contentsOf(url: URL, options readOptionsMask: ReadingOptions = []) throws -> (NSData, URLResponse?) { + let readResult: NSData + var urlResponse: URLResponse? + if url.isFileURL { - let readResult = try NSData.readBytesFromFileWithExtendedAttributes(url.path, options: readOptionsMask) - _init(bytes: readResult.bytes, length: readResult.length, copy: false, deallocator: readResult.deallocator) + let data = try NSData.readBytesFromFileWithExtendedAttributes(url.path, options: readOptionsMask) + readResult = NSData(bytesNoCopy: data.bytes, length: data.length, deallocator: data.deallocator) } else { let session = URLSession(configuration: URLSessionConfiguration.default) let cond = NSCondition() @@ -210,6 +214,7 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding { var resData: Data? let task = session.dataTask(with: url, completionHandler: { data, response, error in resData = data + urlResponse = response resError = error cond.broadcast() }) @@ -218,8 +223,9 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding { guard let data = resData else { throw resError! } - _init(bytes: UnsafeMutableRawPointer(mutating: data._nsObject.bytes), length: data.count, copy: true) + readResult = NSData(bytes: UnsafeMutableRawPointer(mutating: data._nsObject.bytes), length: data.count) } + return (readResult, urlResponse) } /// Initializes a data object with the given Base64 encoded string. diff --git a/Foundation/NSString.swift b/Foundation/NSString.swift index 95e7822bba..7c19b9736a 100644 --- a/Foundation/NSString.swift +++ b/Foundation/NSString.swift @@ -1254,12 +1254,14 @@ extension NSString { public convenience init(contentsOfFile path: String, encoding enc: UInt) throws { try self.init(contentsOf: URL(fileURLWithPath: path), encoding: enc) } - + public convenience init(contentsOf url: URL, usedEncoding enc: UnsafeMutablePointer?) throws { - let readResult = try NSData(contentsOf: url, options:[]) + let (readResult, urlResponse) = try NSData.contentsOf(url: url) let encoding: UInt let offset: Int + // Look for a BOM (Byte Order Marker) to try and determine the text Encoding, this also skips + // over the bytes. This takes precedence over the textEncoding in the http header let bytePtr = readResult.bytes.bindMemory(to: UInt8.self, capacity:readResult.length) if readResult.length >= 4 && bytePtr[0] == 0xFF && bytePtr[1] == 0xFE && bytePtr[2] == 0x00 && bytePtr[3] == 0x00 { encoding = String.Encoding.utf32LittleEndian.rawValue @@ -1277,14 +1279,15 @@ extension NSString { encoding = String.Encoding.utf32BigEndian.rawValue offset = 4 } - else { + else if let charSet = urlResponse?.textEncodingName, let textEncoding = String.Encoding(charSet: charSet) { + encoding = textEncoding.rawValue + offset = 0 + } else { //Need to work on more conditions. This should be the default encoding = String.Encoding.utf8.rawValue offset = 0 } - enc?.pointee = encoding - // Since the encoding being passed includes the byte order the BOM wont be checked or skipped, so pass offset to // manually skip the BOM header. guard let cf = CFStringCreateWithBytes(kCFAllocatorDefault, bytePtr + offset, readResult.length - offset, @@ -1301,6 +1304,7 @@ extension NSString { "NSDebugDescription" : "Unable to bridge CFString to String." ]) } + enc?.pointee = encoding } public convenience init(contentsOfFile path: String, usedEncoding enc: UnsafeMutablePointer?) throws { diff --git a/Foundation/StringEncodings.swift b/Foundation/StringEncodings.swift index 5c3be90411..e59b48ebc9 100644 --- a/Foundation/StringEncodings.swift +++ b/Foundation/StringEncodings.swift @@ -27,6 +27,39 @@ extension String { public static let utf32 = Encoding(rawValue: 0x8c000100) public static let utf32BigEndian = Encoding(rawValue: 0x98000100) public static let utf32LittleEndian = Encoding(rawValue: 0x9c000100) + + // Map selected IANA character set names to encodings, see + // https://www.iana.org/assignments/character-sets/character-sets.xhtml + internal init?(charSet: String) { + let encoding: Encoding? + + switch charSet.lowercased() { + case "us-ascii": encoding = .ascii + case "utf-8": encoding = .utf8 + case "utf-16": encoding = .utf16 + case "utf-16be": encoding = .utf16BigEndian + case "utf-16le": encoding = .utf16LittleEndian + case "utf-32": encoding = .utf32 + case "utf-32be": encoding = .utf32BigEndian + case "utf-32le": encoding = .utf32LittleEndian + case "iso-8859-1": encoding = .isoLatin1 + case "iso-8859-2": encoding = .isoLatin2 + case "iso-2022-jp": encoding = .iso2022JP + case "windows-1250": encoding = .windowsCP1250 + case "windows-1251": encoding = .windowsCP1251 + case "windows-1252": encoding = .windowsCP1252 + case "windows-1253": encoding = .windowsCP1253 + case "windows-1254": encoding = .windowsCP1254 + case "shift_jis": encoding = .shiftJIS + case "euc-jp": encoding = .japaneseEUC + case "macintosh": encoding = .macOSRoman + default: encoding = nil + } + guard let value = encoding?.rawValue else { + return nil + } + rawValue = value + } } public typealias EncodingConversionOptions = NSString.EncodingConversionOptions @@ -50,6 +83,7 @@ extension String.Encoding : CustomStringConvertible { } } + @available(*, unavailable, renamed: "String.Encoding") public typealias NSStringEncoding = UInt diff --git a/TestFoundation/HTTPServer.swift b/TestFoundation/HTTPServer.swift index 00a22304c9..c240370670 100644 --- a/TestFoundation/HTTPServer.swift +++ b/TestFoundation/HTTPServer.swift @@ -306,6 +306,7 @@ struct _HTTPResponse { enum Response : Int { case OK = 200 case REDIRECT = 302 + case NOTFOUND = 404 } private let responseCode: Response private let headers: String @@ -358,6 +359,16 @@ public class TestURLSessionServer { if req.uri.hasPrefix("/LandOfTheLostCities/") { /* these are all misbehaving servers */ try httpServer.respondWithBrokenResponses(uri: req.uri) + } else if req.uri == "/NSString-ISO-8859-1-data.txt" { + // Serve this directly as binary data to avoid any String encoding conversions. + if let url = testBundle().url(forResource: "NSString-ISO-8859-1-data", withExtension: "txt"), + let content = try? Data(contentsOf: url) { + var responseData = "HTTP/1.1 200 OK\r\nContent-Type: text/html; charset=ISO-8859-1\r\nContent-Length: \(content.count)\r\n\r\n".data(using: .ascii)! + responseData.append(content) + try httpServer.socket.writeRawData(responseData) + } else { + try httpServer.respond(with: _HTTPResponse(response: .NOTFOUND, body: "Not Found")) + } } else { try httpServer.respond(with: process(request: req), startDelay: self.startDelay, sendDelay: self.sendDelay, bodyChunks: self.bodyChunks) } diff --git a/TestFoundation/Resources/NSString-ISO-8859-1-data.txt b/TestFoundation/Resources/NSString-ISO-8859-1-data.txt new file mode 100644 index 0000000000..3d959b1320 --- /dev/null +++ b/TestFoundation/Resources/NSString-ISO-8859-1-data.txt @@ -0,0 +1,3 @@ +This file is encoded as ISO-8859-1 + + diff --git a/TestFoundation/TestNSString.swift b/TestFoundation/TestNSString.swift index eb9184319e..56095b3474 100755 --- a/TestFoundation/TestNSString.swift +++ b/TestFoundation/TestNSString.swift @@ -27,7 +27,7 @@ internal let kCFStringEncodingUTF32LE = CFStringBuiltInEncodings.UTF32LE.rawVal #endif -class TestNSString : XCTestCase { +class TestNSString: LoopbackServerTest { static var allTests: [(String, (TestNSString) -> () throws -> Void)] { return [ @@ -292,6 +292,16 @@ class TestNSString : XCTestCase { } catch { XCTFail("Unable to init NSString from contentsOf:encoding:") } + + let url = URL(string: "http://127.0.0.1:\(TestURLSession.serverPort)/NSString-ISO-8859-1-data.txt")! + var enc: UInt = 0 + let contents = try? NSString(contentsOf: url, usedEncoding: &enc) + + XCTAssertNotNil(contents) + XCTAssertEqual(enc, String.Encoding.isoLatin1.rawValue) + if let contents = contents { + XCTAssertEqual(contents, "This file is encoded as ISO-8859-1\nÀÁÂÃÄÅÿ\n±\n") + } } func test_FromContentOfFileUsedEncodingIgnored() { diff --git a/build.py b/build.py index 43acc3c310..943f369ebb 100755 --- a/build.py +++ b/build.py @@ -444,7 +444,7 @@ 'Foundation/URLSession/NativeProtocol.swift', 'Foundation/URLSession/TransferState.swift', 'Foundation/URLSession/libcurl/libcurlHelpers.swift', - 'Foundation/URLSession/http/HTTPURLProtocol.swift', + 'Foundation/URLSession/http/HTTPURLProtocol.swift', 'Foundation/UserDefaults.swift', 'Foundation/NSUUID.swift', 'Foundation/NSValue.swift', @@ -501,6 +501,7 @@ 'TestFoundation/Resources/NSString-UTF16-LE-data.txt', 'TestFoundation/Resources/NSString-UTF32-BE-data.txt', 'TestFoundation/Resources/NSString-UTF32-LE-data.txt', + 'TestFoundation/Resources/NSString-ISO-8859-1-data.txt', 'TestFoundation/Resources/NSXMLDocumentTestData.xml', 'TestFoundation/Resources/PropertyList-1.0.dtd', 'TestFoundation/Resources/NSXMLDTDTestData.xml', From d7f4af74c7e695b6e990f941eb01b3ac694e3ebd Mon Sep 17 00:00:00 2001 From: Simon Evans Date: Wed, 20 Jun 2018 23:31:19 +0100 Subject: [PATCH 2/9] NSCondition fixes: Ensure correct use of lock()/action/unlock() sequence (#1613) --- Foundation/NSData.swift | 15 +++++++++++++-- Foundation/Process.swift | 2 +- TestFoundation/TestNSLock.swift | 4 ++-- TestFoundation/TestThread.swift | 2 +- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/Foundation/NSData.swift b/Foundation/NSData.swift index 48e9d192db..c28dbdd03d 100644 --- a/Foundation/NSData.swift +++ b/Foundation/NSData.swift @@ -210,16 +210,27 @@ open class NSData : NSObject, NSCopying, NSMutableCopying, NSSecureCoding { } else { let session = URLSession(configuration: URLSessionConfiguration.default) let cond = NSCondition() + cond.lock() + var resError: Error? var resData: Data? + var taskFinished = false let task = session.dataTask(with: url, completionHandler: { data, response, error in + cond.lock() resData = data urlResponse = response resError = error - cond.broadcast() + taskFinished = true + cond.signal() + cond.unlock() }) + task.resume() - cond.wait() + while taskFinished == false { + cond.wait() + } + cond.unlock() + guard let data = resData else { throw resError! } diff --git a/Foundation/Process.swift b/Foundation/Process.swift index 8e5e91ba7e..8100f709ba 100644 --- a/Foundation/Process.swift +++ b/Foundation/Process.swift @@ -484,8 +484,8 @@ open class Process: NSObject { self.processIdentifier = pid - self.processLaunchedCondition.unlock() self.processLaunchedCondition.broadcast() + self.processLaunchedCondition.unlock() } open func interrupt() { NSUnimplemented() } // Not always possible. Sends SIGINT. diff --git a/TestFoundation/TestNSLock.swift b/TestFoundation/TestNSLock.swift index a86365815b..2bca129de1 100644 --- a/TestFoundation/TestNSLock.swift +++ b/TestFoundation/TestNSLock.swift @@ -68,15 +68,15 @@ class TestNSLock: XCTestCase { for t in 0.. Date: Sat, 16 Jun 2018 02:09:09 +0100 Subject: [PATCH 3/9] DarwinCompatibility: HTTPCookie fixes for version 0 cookies - For expiry date, prefer maximum-age over expires-date but only use maximum-age for version 1 cookies. - For version 0 cookies only return the first port number, if available. --- Foundation/HTTPCookie.swift | 61 ++++++++++++++++------------- TestFoundation/TestHTTPCookie.swift | 31 +++++++-------- 2 files changed, 48 insertions(+), 44 deletions(-) diff --git a/Foundation/HTTPCookie.swift b/Foundation/HTTPCookie.swift index 47a92d6e3c..91a1cd4096 100644 --- a/Foundation/HTTPCookie.swift +++ b/Foundation/HTTPCookie.swift @@ -266,54 +266,55 @@ open class HTTPCookie : NSObject { } _version = version - if let portString = properties[.port] as? String, _version == 1 { - _portList = portString.split(separator: ",") + if let portString = properties[.port] as? String { + let portList = portString.split(separator: ",") .compactMap { Int(String($0)) } .map { NSNumber(value: $0) } + if version == 1 { + _portList = portList + } else { + // Version 0 only stores a single port number + _portList = portList.count > 0 ? [portList[0]] : nil + } } else { _portList = nil } - // TODO: factor into a utility function - if version == 0 { + var expDate: Date? = nil + // Maximum-Age is prefered over expires-Date but only version 1 cookies use Maximum-Age + if let maximumAge = properties[.maximumAge] as? String, + let secondsFromNow = Int(maximumAge) { + if version == 1 { + expDate = Date(timeIntervalSinceNow: Double(secondsFromNow)) + } + } else { let expiresProperty = properties[.expires] if let date = expiresProperty as? Date { - _expiresDate = date + expDate = date } else if let dateString = expiresProperty as? String { let formatter = DateFormatter() formatter.locale = Locale(identifier: "en_US_POSIX") formatter.dateFormat = "EEE, dd MMM yyyy HH:mm:ss O" // per RFC 6265 '' let timeZone = TimeZone(abbreviation: "GMT") formatter.timeZone = timeZone - _expiresDate = formatter.date(from: dateString) - } else { - _expiresDate = nil + expDate = formatter.date(from: dateString) } - } else if - let maximumAge = properties[.maximumAge] as? String, - let secondsFromNow = Int(maximumAge), _version == 1 { - _expiresDate = Date(timeIntervalSinceNow: Double(secondsFromNow)) - } else { - _expiresDate = nil } + _expiresDate = expDate if let discardString = properties[.discard] as? String { _sessionOnly = discardString == "TRUE" } else { _sessionOnly = properties[.maximumAge] == nil && version >= 1 } - if version == 0 { - _comment = nil - _commentURL = nil + + _comment = properties[.comment] as? String + if let commentURL = properties[.commentURL] as? URL { + _commentURL = commentURL + } else if let commentURL = properties[.commentURL] as? String { + _commentURL = URL(string: commentURL) } else { - _comment = properties[.comment] as? String - if let commentURL = properties[.commentURL] as? URL { - _commentURL = commentURL - } else if let commentURL = properties[.commentURL] as? String { - _commentURL = URL(string: commentURL) - } else { - _commentURL = nil - } + _commentURL = nil } _HTTPOnly = false @@ -363,7 +364,11 @@ open class HTTPCookie : NSObject { cookieString.removeLast() cookieString.removeLast() } - return ["Cookie": cookieString] + if cookieString == "" { + return [:] + } else { + return ["Cookie": cookieString] + } } /// Return an array of cookies parsed from the specified response header fields and URL. @@ -418,9 +423,9 @@ open class HTTPCookie : NSObject { properties[canonicalize(name)] = value } - //if domain wasn't provided use the URL + // If domain wasn't provided, extract it from the URL if properties[.domain] == nil { - properties[.domain] = url.absoluteString + properties[.domain] = url.host } //the default Path is "/" diff --git a/TestFoundation/TestHTTPCookie.swift b/TestFoundation/TestHTTPCookie.swift index 5ca445fbaf..b46e373087 100644 --- a/TestFoundation/TestHTTPCookie.swift +++ b/TestFoundation/TestHTTPCookie.swift @@ -67,31 +67,29 @@ class TestHTTPCookie: XCTestCase { .domain: "apple.com", .originURL: URL(string: "https://apple.com")!, .comment: "This comment should be nil since this is a v0 cookie.", - .commentURL: URL(string: "https://apple.com")!, + .commentURL: "https://apple.com", .discard: "TRUE", .expires: Date(timeIntervalSince1970: 1000), .maximumAge: "2000", .port: "443,8443", .secure: "YES" ]) - XCTAssertNil(versionZeroCookieWithInvalidVersionOneProps?.comment) - XCTAssertNil(versionZeroCookieWithInvalidVersionOneProps?.commentURL) + XCTAssertEqual(versionZeroCookieWithInvalidVersionOneProps?.version, 0) + XCTAssertNotNil(versionZeroCookieWithInvalidVersionOneProps?.comment) + XCTAssertNotNil(versionZeroCookieWithInvalidVersionOneProps?.commentURL) XCTAssert(versionZeroCookieWithInvalidVersionOneProps?.isSessionOnly == true) // v0 should never use NSHTTPCookieMaximumAge - XCTAssert( - versionZeroCookieWithInvalidVersionOneProps?.expiresDate?.timeIntervalSince1970 == - Date(timeIntervalSince1970: 1000).timeIntervalSince1970 - ) + XCTAssertNil(versionZeroCookieWithInvalidVersionOneProps?.expiresDate?.timeIntervalSince1970) - XCTAssertNil(versionZeroCookieWithInvalidVersionOneProps?.portList) + XCTAssertEqual(versionZeroCookieWithInvalidVersionOneProps?.portList, [NSNumber(value: 443)]) XCTAssert(versionZeroCookieWithInvalidVersionOneProps?.isSecure == true) XCTAssert(versionZeroCookieWithInvalidVersionOneProps?.version == 0) } func test_RequestHeaderFields() { let noCookies: [HTTPCookie] = [] - XCTAssertEqual(HTTPCookie.requestHeaderFields(with: noCookies)["Cookie"], "") + XCTAssertNil(HTTPCookie.requestHeaderFields(with: noCookies)["Cookie"]) let basicCookies: [HTTPCookie] = [ HTTPCookie(properties: [ @@ -117,7 +115,7 @@ class TestHTTPCookie: XCTestCase { "Set-Cookie": "fr=anjd&232; Max-Age=7776000; path=/; domain=.example.com; secure; httponly", "header2":"value2", "header3":"value3"] - let cookies = HTTPCookie.cookies(withResponseHeaderFields: header, for: URL(string: "http://example.com")!) + let cookies = HTTPCookie.cookies(withResponseHeaderFields: header, for: URL(string: "https://example.com")!) XCTAssertEqual(cookies.count, 1) XCTAssertEqual(cookies[0].name, "fr") XCTAssertEqual(cookies[0].value, "anjd&232") @@ -134,7 +132,7 @@ class TestHTTPCookie: XCTestCase { "Set-Cookie": "fr=a&2@#; Max-Age=1186000; path=/; domain=.example.com; secure, xd=plm!@#;path=/;domain=.example2.com", "header2":"value2", "header3":"value3"] - let cookies = HTTPCookie.cookies(withResponseHeaderFields: header, for: URL(string: "http://example.com")!) + let cookies = HTTPCookie.cookies(withResponseHeaderFields: header, for: URL(string: "https://example.com")!) XCTAssertEqual(cookies.count, 2) XCTAssertTrue(cookies[0].isSecure) XCTAssertFalse(cookies[1].isSecure) @@ -142,11 +140,12 @@ class TestHTTPCookie: XCTestCase { func test_cookiesWithResponseHeaderNoDomain() { let header = ["header1":"value1", - "Set-Cookie": "fr=anjd&232; expires=Wed, 21 Sep 2016 05:33:00 GMT; Max-Age=7776000; path=/; secure; httponly", + "Set-Cookie": "fr=anjd&232; expires=Wed, 21 Sep 2016 05:33:00 GMT; path=/; secure; httponly", "header2":"value2", "header3":"value3"] - let cookies = HTTPCookie.cookies(withResponseHeaderFields: header, for: URL(string: "http://example.com")!) - XCTAssertEqual(cookies[0].domain, "http://example.com") + let cookies = HTTPCookie.cookies(withResponseHeaderFields: header, for: URL(string: "https://example.com")!) + XCTAssertEqual(cookies[0].version, 0) + XCTAssertEqual(cookies[0].domain, "example.com") XCTAssertNotNil(cookies[0].expiresDate) let formatter = DateFormatter() @@ -165,8 +164,8 @@ class TestHTTPCookie: XCTestCase { "Set-Cookie": "fr=tx; expires=Wed, 21-Sep-2016 05:33:00 GMT; Max-Age=7776000; secure; httponly", "header2":"value2", "header3":"value3"] - let cookies = HTTPCookie.cookies(withResponseHeaderFields: header, for: URL(string: "http://example.com")!) - XCTAssertEqual(cookies[0].domain, "http://example.com") + let cookies = HTTPCookie.cookies(withResponseHeaderFields: header, for: URL(string: "https://example.com")!) + XCTAssertEqual(cookies[0].domain, "example.com") XCTAssertEqual(cookies[0].path, "/") } } From 7f55f7bea401a25c5d3c87b6368a5f255aa3cb58 Mon Sep 17 00:00:00 2001 From: Simon Evans Date: Tue, 19 Jun 2018 15:48:45 +0100 Subject: [PATCH 4/9] Test HTTPServer.swift fixes: - Set SO_NOSIGPIPE on the client socket (Darwin) or use send() with flag MSG_NOSIGNAL (Linux). - listen() just after bind()ing. - Close the client socket after writing out the response. - When sending a response with a start delay dont use asyncAfter() with a semaphore, just use Thread.sleep(). - Fix parsing the HTTP request so that all of the header lines are read correctly instead of reading the last header line as the body. - When waiting for the server to become ready, wait for the semaphore with a timeout rather than an indefinite wait(). - Add setUp() to be run before each test class. - For test cases that check the respnse body, check the data is not nil before reading it. - Use 1 server for whole class clifetime. - Use shutdown() to interrupt accept() when shutting down the server. --- TestFoundation/HTTPServer.swift | 166 +++++++++++++++++++++----------- 1 file changed, 108 insertions(+), 58 deletions(-) diff --git a/TestFoundation/HTTPServer.swift b/TestFoundation/HTTPServer.swift index c240370670..56f180c4e6 100644 --- a/TestFoundation/HTTPServer.swift +++ b/TestFoundation/HTTPServer.swift @@ -40,7 +40,8 @@ extension UInt16 { } class _TCPSocket { - + + private let sendFlags: CInt private var listenSocket: Int32! private var socketAddress = UnsafeMutablePointer.allocate(capacity: 1) private var connectionSocket: Int32! @@ -55,27 +56,37 @@ class _TCPSocket { private func attempt(_ name: String, file: String = #file, line: UInt = #line, valid: (CInt) -> Bool, _ b: @autoclosure () -> CInt) throws -> CInt { let r = b() - guard valid(r) else { throw ServerError(operation: name, errno: r, file: file, line: line) } + guard valid(r) else { + throw ServerError(operation: name, errno: errno, file: file, line: line) + } return r } public private(set) var port: UInt16 init(port: UInt16?) throws { - #if os(Linux) && !os(Android) +#if canImport(Darwin) + sendFlags = 0 +#else + sendFlags = CInt(MSG_NOSIGNAL) +#endif + +#if os(Linux) && !os(Android) let SOCKSTREAM = Int32(SOCK_STREAM.rawValue) - #else +#else let SOCKSTREAM = SOCK_STREAM - #endif +#endif self.port = port ?? 0 listenSocket = try attempt("socket", valid: isNotNegative, socket(AF_INET, SOCKSTREAM, Int32(IPPROTO_TCP))) - var on: Int = 1 - _ = try attempt("setsockopt", valid: isZero, setsockopt(listenSocket, SOL_SOCKET, SO_REUSEADDR, &on, socklen_t(MemoryLayout.size))) + var on: CInt = 1 + _ = try attempt("setsockopt", valid: isZero, setsockopt(listenSocket, SOL_SOCKET, SO_REUSEADDR, &on, socklen_t(MemoryLayout.size))) + let sa = createSockaddr(port) socketAddress.initialize(to: sa) try socketAddress.withMemoryRebound(to: sockaddr.self, capacity: MemoryLayout.size, { let addr = UnsafePointer($0) _ = try attempt("bind", valid: isZero, bind(listenSocket, addr, socklen_t(MemoryLayout.size))) + _ = try attempt("listen", valid: isZero, listen(listenSocket, SOMAXCONN)) }) var actualSA = sockaddr_in() @@ -104,18 +115,21 @@ class _TCPSocket { } func acceptConnection(notify: ServerSemaphore) throws { - _ = try attempt("listen", valid: isZero, listen(listenSocket, SOMAXCONN)) try socketAddress.withMemoryRebound(to: sockaddr.self, capacity: MemoryLayout.size, { let addr = UnsafeMutablePointer($0) var sockLen = socklen_t(MemoryLayout.size) - notify.signal() connectionSocket = try attempt("accept", valid: isNotNegative, accept(listenSocket, addr, &sockLen)) +#if canImport(Dawin) + // Disable SIGPIPEs when writing to closed sockets + var on: CInt = 1 + _ = try attempt("setsockopt", valid: isZero, setsockopt(connectionSocket, SOL_SOCKET, SO_NOSIGPIPE, &on, socklen_t(MemoryLayout.size))) +#endif }) } func readData() throws -> String { var buffer = [UInt8](repeating: 0, count: 4096) - _ = try attempt("read", valid: isNotNegative, CInt(read(connectionSocket, &buffer, 4096))) + _ = try attempt("read", valid: isNotNegative, CInt(read(connectionSocket, &buffer, buffer.count))) return String(cString: &buffer) } @@ -129,14 +143,14 @@ class _TCPSocket { func writeRawData(_ data: Data) throws { _ = try data.withUnsafeBytes { ptr in - try attempt("write", valid: isNotNegative, CInt(write(connectionSocket, ptr, data.count))) + try attempt("send", valid: isNotNegative, CInt(send(connectionSocket, ptr, data.count, sendFlags))) } } func writeData(header: String, body: String, sendDelay: TimeInterval? = nil, bodyChunks: Int? = nil) throws { - var header = Array(header.utf8) - _ = try attempt("write", valid: isNotNegative, CInt(write(connectionSocket, &header, header.count))) - + var _header = Array(header.utf8) + _ = try attempt("send", valid: isNotNegative, CInt(send(connectionSocket, &_header, _header.count, sendFlags))) + if let sendDelay = sendDelay, let bodyChunks = bodyChunks { let count = max(1, Int(Double(body.utf8.count) / Double(bodyChunks))) let texts = split(body, count) @@ -144,18 +158,24 @@ class _TCPSocket { for item in texts { sleep(UInt32(sendDelay)) var bytes = Array(item.utf8) - _ = try attempt("write", valid: isNotNegative, CInt(write(connectionSocket, &bytes, bytes.count))) + _ = try attempt("send", valid: isNotNegative, CInt(send(connectionSocket, &bytes, bytes.count, sendFlags))) } } else { var bytes = Array(body.utf8) - _ = try attempt("write", valid: isNotNegative, CInt(write(connectionSocket, &bytes, bytes.count))) + _ = try attempt("send", valid: isNotNegative, CInt(send(connectionSocket, &bytes, bytes.count, sendFlags))) } } - func shutdown() { + func closeClient() { if let connectionSocket = self.connectionSocket { close(connectionSocket) + self.connectionSocket = nil } + } + + func shutdownListener() { + closeClient() + shutdown(listenSocket, CInt(SHUT_RDWR)) close(listenSocket) } } @@ -182,7 +202,8 @@ class _HTTPServer { } public func stop() { - socket.shutdown() + socket.closeClient() + socket.shutdownListener() } public func request() throws -> _HTTPRequest { @@ -190,24 +211,14 @@ class _HTTPServer { } public func respond(with response: _HTTPResponse, startDelay: TimeInterval? = nil, sendDelay: TimeInterval? = nil, bodyChunks: Int? = nil) throws { - let semaphore = DispatchSemaphore(value: 0) - let deadlineTime: DispatchTime - - if let startDelay = startDelay { - deadlineTime = .now() + .seconds(Int(startDelay)) - } else { - deadlineTime = .now() + if let delay = startDelay { + Thread.sleep(forTimeInterval: delay) } - - DispatchQueue.global().asyncAfter(deadline: deadlineTime) { - do { - try self.socket.writeData(header: response.header, body: response.body, sendDelay: sendDelay, bodyChunks: bodyChunks) - semaphore.signal() - } catch { } + do { + try self.socket.writeData(header: response.header, body: response.body, sendDelay: sendDelay, bodyChunks: bodyChunks) + } catch { } - semaphore.wait() - - } + } func respondWithBrokenResponses(uri: String) throws { let responseData: Data @@ -285,11 +296,13 @@ struct _HTTPRequest { let headers: [String] public init(request: String) { - let lines = request.components(separatedBy: _HTTPUtils.CRLF2)[0].components(separatedBy: _HTTPUtils.CRLF) - headers = Array(lines[0...lines.count-2]) - method = Method(rawValue: headers[0].components(separatedBy: " ")[0])! - uri = headers[0].components(separatedBy: " ")[1] - body = lines.last! + let headerEnd = (request as NSString).range(of: _HTTPUtils.CRLF2) + let header = (request as NSString).substring(to: headerEnd.location) + headers = header.components(separatedBy: _HTTPUtils.CRLF) + let action = headers[0] + method = Method(rawValue: action.components(separatedBy: " ")[0])! + uri = action.components(separatedBy: " ")[1] + body = (request as NSString).substring(from: headerEnd.location + headerEnd.length) } public func getCommaSeparatedHeaders() -> String { @@ -349,11 +362,7 @@ public class TestURLSessionServer { self.sendDelay = sendDelay self.bodyChunks = bodyChunks } - public func start(started: ServerSemaphore) throws { - started.signal() - try httpServer.listen(notify: started) - } - + public func readAndRespond() throws { let req = try httpServer.request() if req.uri.hasPrefix("/LandOfTheLostCities/") { @@ -400,6 +409,16 @@ public class TestURLSessionServer { return _HTTPResponse(response: .OK, headers: "Content-Length: \(text.data(using: .utf8)!.count)", body: text) } + if uri == "/requestCookies" { + let text = request.getCommaSeparatedHeaders() + return _HTTPResponse(response: .OK, headers: "Content-Length: \(text.data(using: .utf8)!.count)\r\nSet-Cookie: fr=anjd&232; Max-Age=7776000; path=/\r\nSet-Cookie: nm=sddf&232; Max-Age=7776000; path=/; domain=.swift.org; secure; httponly\r\n", body: text) + } + + if uri == "/setCookies" { + let text = request.getCommaSeparatedHeaders() + return _HTTPResponse(response: .OK, headers: "Content-Length: \(text.data(using: .utf8)!.count)", body: text) + } + if uri == "/UnitedStates" { let value = capitals[String(uri.dropFirst())]! let text = request.getCommaSeparatedHeaders() @@ -472,8 +491,8 @@ extension ServerError : CustomStringConvertible { public class ServerSemaphore { let dispatchSemaphore = DispatchSemaphore(value: 0) - public func wait() { - dispatchSemaphore.wait() + public func wait(timeout: DispatchTime) -> DispatchTimeoutResult { + return dispatchSemaphore.wait(timeout: timeout) } public func signal() { @@ -485,6 +504,11 @@ class LoopbackServerTest : XCTestCase { private static let staticSyncQ = DispatchQueue(label: "org.swift.TestFoundation.HTTPServer.StaticSyncQ") private static var _serverPort: Int = -1 + private static let serverReady = ServerSemaphore() + private static var _serverActive = false + private static var testServer: TestURLSessionServer? = nil + + static var serverPort: Int { get { return staticSyncQ.sync { _serverPort } @@ -494,30 +518,56 @@ class LoopbackServerTest : XCTestCase { } } + static var serverActive: Bool { + get { return staticSyncQ.sync { _serverActive } } + set { staticSyncQ.sync { _serverActive = newValue }} + } + + static func terminateServer() { + serverActive = false + testServer?.stop() + testServer = nil + } + override class func setUp() { super.setUp() func runServer(with condition: ServerSemaphore, startDelay: TimeInterval? = nil, sendDelay: TimeInterval? = nil, bodyChunks: Int? = nil) throws { - while true { - let test = try TestURLSessionServer(port: nil, startDelay: startDelay, sendDelay: sendDelay, bodyChunks: bodyChunks) - serverPort = Int(test.port) - try test.start(started: condition) - try test.readAndRespond() - serverPort = -2 - test.stop() + let server = try TestURLSessionServer(port: nil, startDelay: startDelay, sendDelay: sendDelay, bodyChunks: bodyChunks) + testServer = server + serverPort = Int(server.port) + serverReady.signal() + serverActive = true + + while serverActive { + do { + try server.httpServer.listen(notify: condition) + try server.readAndRespond() + server.httpServer.socket.closeClient() + } catch { + } } + serverPort = -2 + } - let serverReady = ServerSemaphore() globalDispatchQueue.async { do { try runServer(with: serverReady) - } catch { - XCTAssertTrue(true) - return } } - serverReady.wait() + let timeout = DispatchTime(uptimeNanoseconds: DispatchTime.now().uptimeNanoseconds + 2_000_000_000) + + while serverPort == -2 { + guard serverReady.wait(timeout: timeout) == .success else { + fatalError("Timedout waiting for server to be ready") + } + } + } + + override class func tearDown() { + super.tearDown() + terminateServer() } } From 4b39b43692727f7ece44e91b1b48771f003cc15f Mon Sep 17 00:00:00 2001 From: Simon Evans Date: Tue, 19 Jun 2018 15:50:16 +0100 Subject: [PATCH 5/9] TestURLSession fixes: - Uncomment failing tests that now work, fixes SR-7723 and SR-5751. - Disable TestURLSession.test_cancelTask() as it fails intermittently because the server can respond before the task is cancelled. - Disable TestURLSession.test_simpleUploadWithDelegate() as the server wont read the entire body successfully. --- TestFoundation/TestURLSession.swift | 135 ++++++++++++++++++++++++++-- 1 file changed, 127 insertions(+), 8 deletions(-) diff --git a/TestFoundation/TestURLSession.swift b/TestFoundation/TestURLSession.swift index 161f9530dd..db1c87d286 100644 --- a/TestFoundation/TestURLSession.swift +++ b/TestFoundation/TestURLSession.swift @@ -28,14 +28,14 @@ class TestURLSession : LoopbackServerTest { ("test_verifyHttpAdditionalHeaders", test_verifyHttpAdditionalHeaders), ("test_timeoutInterval", test_timeoutInterval), ("test_httpRedirectionWithCompleteRelativePath", test_httpRedirectionWithCompleteRelativePath), - //("test_httpRedirectionWithInCompleteRelativePath", test_httpRedirectionWithInCompleteRelativePath), /* temporarily disabled. Needs HTTPServer rework */ - //("test_httpRedirectionTimeout", test_httpRedirectionTimeout), /* temporarily disabled (https://bugs.swift.org/browse/SR-5751) */ + ("test_httpRedirectionWithInCompleteRelativePath", test_httpRedirectionWithInCompleteRelativePath), /* temporarily disabled. Needs HTTPServer rework */ + ("test_httpRedirectionTimeout", test_httpRedirectionTimeout), /* temporarily disabled (https://bugs.swift.org/browse/SR-5751) */ ("test_http0_9SimpleResponses", test_http0_9SimpleResponses), ("test_outOfRangeButCorrectlyFormattedHTTPCode", test_outOfRangeButCorrectlyFormattedHTTPCode), ("test_missingContentLengthButStillABody", test_missingContentLengthButStillABody), ("test_illegalHTTPServerResponses", test_illegalHTTPServerResponses), ("test_dataTaskWithSharedDelegate", test_dataTaskWithSharedDelegate), - ("test_simpleUploadWithDelegate", test_simpleUploadWithDelegate), + // ("test_simpleUploadWithDelegate", test_simpleUploadWithDelegate), - Server needs modification ("test_concurrentRequests", test_concurrentRequests), ] } @@ -219,7 +219,8 @@ class TestURLSession : LoopbackServerTest { XCTAssert(task.isEqual(task.copy())) } - + + // This test is buggy becuase the server could respond before the task is cancelled. func test_cancelTask() { #if os(Android) XCTFail("Intermittent failures on Android") @@ -275,7 +276,8 @@ class TestURLSession : LoopbackServerTest { defer { expect.fulfill() } XCTAssertNotNil(data) XCTAssertNil(error as? URLError, "error = \(error as! URLError)") - let headers = String(data: data!, encoding: String.Encoding.utf8) ?? "" + guard let data = data else { return } + let headers = String(data: data, encoding: .utf8) ?? "" XCTAssertNotNil(headers.range(of: "header1: rvalue1")) XCTAssertNotNil(headers.range(of: "header2: rvalue2")) XCTAssertNotNil(headers.range(of: "header3: svalue3")) @@ -334,7 +336,6 @@ class TestURLSession : LoopbackServerTest { waitForExpectations(timeout: 12) } - /* // temporarily disabled (https://bugs.swift.org/browse/SR-5751) func test_httpRedirectionTimeout() { let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/UnitedStates" @@ -346,7 +347,7 @@ class TestURLSession : LoopbackServerTest { let task = session.dataTask(with: req) { data, response, error in defer { expect.fulfill() } if let e = error as? URLError { - XCTAssertEqual(e.code, .timedOut, "Unexpected error code") + XCTAssertEqual(e.code, .cannotConnectToHost, "Unexpected error code") return } else { XCTFail("test unexpectedly succeeded (response=\(response.debugDescription))") @@ -355,7 +356,6 @@ class TestURLSession : LoopbackServerTest { task.resume() waitForExpectations(timeout: 12) } - */ func test_http0_9SimpleResponses() { for brokenCity in ["Pompeii", "Sodom"] { @@ -516,6 +516,125 @@ class TestURLSession : LoopbackServerTest { } } + func test_disableCookiesStorage() { + let config = URLSessionConfiguration.default + config.timeoutIntervalForRequest = 5 + config.httpCookieAcceptPolicy = HTTPCookie.AcceptPolicy.never + if let storage = config.httpCookieStorage, let cookies = storage.cookies { + for cookie in cookies { + storage.deleteCookie(cookie) + } + } + XCTAssertEqual(config.httpCookieStorage?.cookies?.count, 0) + let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/requestCookies" + let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil) + var expect = expectation(description: "POST \(urlString)") + var req = URLRequest(url: URL(string: urlString)!) + req.httpMethod = "POST" + var task = session.dataTask(with: req) { (data, _, error) -> Void in + defer { expect.fulfill() } + XCTAssertNotNil(data) + XCTAssertNil(error as? URLError, "error = \(error as! URLError)") + } + task.resume() + waitForExpectations(timeout: 30) + let cookies = HTTPCookieStorage.shared.cookies + XCTAssertEqual(cookies?.count, 0) + } + + func test_cookiesStorage() { + let config = URLSessionConfiguration.default + config.timeoutIntervalForRequest = 5 + let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/requestCookies" + let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil) + var expect = expectation(description: "POST \(urlString)") + var req = URLRequest(url: URL(string: urlString)!) + req.httpMethod = "POST" + var task = session.dataTask(with: req) { (data, _, error) -> Void in + defer { expect.fulfill() } + XCTAssertNotNil(data) + XCTAssertNil(error as? URLError, "error = \(error as! URLError)") + } + task.resume() + waitForExpectations(timeout: 30) + let cookies = HTTPCookieStorage.shared.cookies + XCTAssertEqual(cookies?.count, 1) + } + + func test_setCookies() { + let config = URLSessionConfiguration.default + config.timeoutIntervalForRequest = 5 + let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/setCookies" + let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil) + var expect = expectation(description: "POST \(urlString)") + var req = URLRequest(url: URL(string: urlString)!) + req.httpMethod = "POST" + var task = session.dataTask(with: req) { (data, _, error) -> Void in + defer { expect.fulfill() } + XCTAssertNotNil(data) + XCTAssertNil(error as? URLError, "error = \(error as! URLError)") + guard let data = data else { return } + let headers = String(data: data, encoding: String.Encoding.utf8) ?? "" + XCTAssertNotNil(headers.range(of: "Cookie: fr=anjd&232")) + } + task.resume() + waitForExpectations(timeout: 30) + } + + func test_dontSetCookies() { + let config = URLSessionConfiguration.default + config.timeoutIntervalForRequest = 5 + config.httpShouldSetCookies = false + let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/setCookies" + let session = URLSession(configuration: config, delegate: nil, delegateQueue: nil) + var expect = expectation(description: "POST \(urlString)") + var req = URLRequest(url: URL(string: urlString)!) + req.httpMethod = "POST" + var task = session.dataTask(with: req) { (data, _, error) -> Void in + defer { expect.fulfill() } + XCTAssertNotNil(data) + XCTAssertNil(error as? URLError, "error = \(error as! URLError)") + guard let data = data else { return } + let headers = String(data: data, encoding: String.Encoding.utf8) ?? "" + XCTAssertNil(headers.range(of: "Cookie: fr=anjd&232")) + } + task.resume() + waitForExpectations(timeout: 30) + } + + // Validate that the properties are correctly set + func test_initURLSessionConfiguration() { + let config = URLSessionConfiguration.default + config.requestCachePolicy = .useProtocolCachePolicy + config.timeoutIntervalForRequest = 30 + config.timeoutIntervalForResource = 604800 + config.networkServiceType = .default + config.allowsCellularAccess = false + config.isDiscretionary = true + config.httpShouldUsePipelining = true + config.httpShouldSetCookies = true + config.httpCookieAcceptPolicy = .always + config.httpMaximumConnectionsPerHost = 2 + config.httpCookieStorage = HTTPCookieStorage.shared + config.urlCredentialStorage = nil + config.urlCache = nil + config.shouldUseExtendedBackgroundIdleMode = true + + XCTAssertEqual(config.requestCachePolicy, NSURLRequest.CachePolicy.useProtocolCachePolicy) + XCTAssertEqual(config.timeoutIntervalForRequest, 30) + XCTAssertEqual(config.timeoutIntervalForResource, 604800) + XCTAssertEqual(config.networkServiceType, NSURLRequest.NetworkServiceType.default) + XCTAssertEqual(config.allowsCellularAccess, false) + XCTAssertEqual(config.isDiscretionary, true) + XCTAssertEqual(config.httpShouldUsePipelining, true) + XCTAssertEqual(config.httpShouldSetCookies, true) + XCTAssertEqual(config.httpCookieAcceptPolicy, HTTPCookie.AcceptPolicy.always) + XCTAssertEqual(config.httpMaximumConnectionsPerHost, 2) + XCTAssertEqual(config.httpCookieStorage, HTTPCookieStorage.shared) + XCTAssertEqual(config.urlCredentialStorage, nil) + XCTAssertEqual(config.urlCache, nil) + XCTAssertEqual(config.shouldUseExtendedBackgroundIdleMode, true) + } } class SharedDelegate: NSObject { From 998a72b871e4d2f6e51fa6c838a147c868f029cc Mon Sep 17 00:00:00 2001 From: Simon Evans Date: Wed, 20 Jun 2018 14:30:24 +0100 Subject: [PATCH 6/9] TestURLSession: Fix test_cancelTask() - Add a delay into the test server response so that the task will always be cancelled before the server returns the complete response. --- TestFoundation/HTTPServer.swift | 17 +++++++++++++++++ TestFoundation/TestURLSession.swift | 9 +++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/TestFoundation/HTTPServer.swift b/TestFoundation/HTTPServer.swift index 56f180c4e6..bc87cdd09f 100644 --- a/TestFoundation/HTTPServer.swift +++ b/TestFoundation/HTTPServer.swift @@ -313,6 +313,16 @@ struct _HTTPRequest { return allHeaders } + public func getHeader(for key: String) -> String? { + let lookup = key.lowercased() + for header in headers { + let parts = header.components(separatedBy: ":") + if parts[0].lowercased() == lookup { + return parts[1].trimmingCharacters(in: CharacterSet(charactersIn: " ")) + } + } + return nil + } } struct _HTTPResponse { @@ -365,6 +375,13 @@ public class TestURLSessionServer { public func readAndRespond() throws { let req = try httpServer.request() + + if let value = req.getHeader(for: "x-pause") { + if let wait = Double(value), wait > 0 { + Thread.sleep(forTimeInterval: wait) + } + } + if req.uri.hasPrefix("/LandOfTheLostCities/") { /* these are all misbehaving servers */ try httpServer.respondWithBrokenResponses(uri: req.uri) diff --git a/TestFoundation/TestURLSession.swift b/TestFoundation/TestURLSession.swift index db1c87d286..bb23672bdc 100644 --- a/TestFoundation/TestURLSession.swift +++ b/TestFoundation/TestURLSession.swift @@ -28,8 +28,8 @@ class TestURLSession : LoopbackServerTest { ("test_verifyHttpAdditionalHeaders", test_verifyHttpAdditionalHeaders), ("test_timeoutInterval", test_timeoutInterval), ("test_httpRedirectionWithCompleteRelativePath", test_httpRedirectionWithCompleteRelativePath), - ("test_httpRedirectionWithInCompleteRelativePath", test_httpRedirectionWithInCompleteRelativePath), /* temporarily disabled. Needs HTTPServer rework */ - ("test_httpRedirectionTimeout", test_httpRedirectionTimeout), /* temporarily disabled (https://bugs.swift.org/browse/SR-5751) */ + ("test_httpRedirectionWithInCompleteRelativePath", test_httpRedirectionWithInCompleteRelativePath), + ("test_httpRedirectionTimeout", test_httpRedirectionTimeout), ("test_http0_9SimpleResponses", test_http0_9SimpleResponses), ("test_outOfRangeButCorrectlyFormattedHTTPCode", test_outOfRangeButCorrectlyFormattedHTTPCode), ("test_missingContentLengthButStillABody", test_missingContentLengthButStillABody), @@ -226,10 +226,11 @@ class TestURLSession : LoopbackServerTest { XCTFail("Intermittent failures on Android") #else let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/Peru" - let url = URL(string: urlString)! + var urlRequest = URLRequest(url: URL(string: urlString)!) + urlRequest.setValue("2.0", forHTTPHeaderField: "X-Pause") let d = DataTask(with: expectation(description: "GET \(urlString): task cancelation")) d.cancelExpectation = expectation(description: "GET \(urlString): task canceled") - d.run(with: url) + d.run(with: urlRequest) d.cancel() waitForExpectations(timeout: 12) #endif From 34060d62dc589f4a3d5b78bf42ba1af683ecfb12 Mon Sep 17 00:00:00 2001 From: Simon Evans Date: Fri, 22 Jun 2018 22:06:01 +0100 Subject: [PATCH 7/9] TestURLSession: Undo discretionary/isDiscretionary change --- TestFoundation/TestURLSession.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/TestFoundation/TestURLSession.swift b/TestFoundation/TestURLSession.swift index bb23672bdc..4cfab5da4a 100644 --- a/TestFoundation/TestURLSession.swift +++ b/TestFoundation/TestURLSession.swift @@ -611,7 +611,7 @@ class TestURLSession : LoopbackServerTest { config.timeoutIntervalForResource = 604800 config.networkServiceType = .default config.allowsCellularAccess = false - config.isDiscretionary = true + config.discretionary = true config.httpShouldUsePipelining = true config.httpShouldSetCookies = true config.httpCookieAcceptPolicy = .always @@ -626,7 +626,7 @@ class TestURLSession : LoopbackServerTest { XCTAssertEqual(config.timeoutIntervalForResource, 604800) XCTAssertEqual(config.networkServiceType, NSURLRequest.NetworkServiceType.default) XCTAssertEqual(config.allowsCellularAccess, false) - XCTAssertEqual(config.isDiscretionary, true) + XCTAssertEqual(config.discretionary, true) XCTAssertEqual(config.httpShouldUsePipelining, true) XCTAssertEqual(config.httpShouldSetCookies, true) XCTAssertEqual(config.httpCookieAcceptPolicy, HTTPCookie.AcceptPolicy.always) From b2c809283aa9154f5a93e52c78707c52ba179231 Mon Sep 17 00:00:00 2001 From: tid Date: Mon, 2 Apr 2018 02:20:59 +0900 Subject: [PATCH 8/9] [SR-7112] Fixed a case where CFStringGetCStringPtr returns a string without null termination Changed _fastCStringContents (used in CFStringGetCStringPtr) to return NULL if required to return a string containing null termination. Since this fix, if you call CFStringGetCStringPtr with a Swift-based CFString, it returns NULL. Before the change: _fastCStringContents always returned a pointer to an ASCII string containing no NULL termination. This is because the code was using unsafeBitCast to get internal pointer of String of Swift. Swift's string does not guarantee null termination. --- Foundation/NSString.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Foundation/NSString.swift b/Foundation/NSString.swift index 7c19b9736a..dc95e5282d 100644 --- a/Foundation/NSString.swift +++ b/Foundation/NSString.swift @@ -294,6 +294,10 @@ open class NSString : NSObject, NSCopying, NSMutableCopying, NSSecureCoding, NSC } internal func _fastCStringContents(_ nullTerminated: Bool) -> UnsafePointer? { + guard !nullTerminated else { + // There is no way to fastly and safely retrieve a pointer to a null-terminated string from a String of Swift. + return nil + } if type(of: self) == NSString.self || type(of: self) == NSMutableString.self { if _storage._guts._isContiguousASCII { return unsafeBitCast(_storage._guts.startASCII, to: UnsafePointer.self) From a657b2e9f9e8e9dd57c9ece5479f8b0c06e06437 Mon Sep 17 00:00:00 2001 From: Simon Evans Date: Thu, 2 Aug 2018 13:14:01 +0100 Subject: [PATCH 9/9] When redirecting with a relative Location, add the port number to the new URL - If the new location includes a host but no port number, then DONT add the port number. --- Foundation/URLSession/http/HTTPURLProtocol.swift | 9 +++++++++ TestFoundation/HTTPServer.swift | 7 +++++++ TestFoundation/TestURLSession.swift | 14 ++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/Foundation/URLSession/http/HTTPURLProtocol.swift b/Foundation/URLSession/http/HTTPURLProtocol.swift index 53069a00ef..cf58938725 100644 --- a/Foundation/URLSession/http/HTTPURLProtocol.swift +++ b/Foundation/URLSession/http/HTTPURLProtocol.swift @@ -398,10 +398,19 @@ internal extension _HTTPURLProtocol { let scheme = request.url?.scheme let host = request.url?.host + let port = request.url?.port var components = URLComponents() components.scheme = scheme components.host = host + + // Use the original port if the new URL does not contain a host + // ie Location: /foo => :/Foo + // but Location: newhost/foo will ignore the original port + if targetURL.host == nil { + components.port = port + } + //The path must either begin with "/" or be an empty string. if targetURL.relativeString.first != "/" { components.path = "/" + targetURL.relativeString diff --git a/TestFoundation/HTTPServer.swift b/TestFoundation/HTTPServer.swift index bc87cdd09f..608fbda244 100644 --- a/TestFoundation/HTTPServer.swift +++ b/TestFoundation/HTTPServer.swift @@ -480,6 +480,13 @@ public class TestURLSessionServer { let httpResponse = _HTTPResponse(response: .REDIRECT, headers: "Location: \(value)", body: text) return httpResponse } + if uri == "/redirect-with-default-port" { + let text = request.getCommaSeparatedHeaders() + let host = request.headers[1].components(separatedBy: " ")[1] + let ip = host.components(separatedBy: ":")[0] + let httpResponse = _HTTPResponse(response: .REDIRECT, headers: "Location: http://\(ip)/redirected-with-default-port", body: text) + return httpResponse + } return _HTTPResponse(response: .OK, body: capitals[String(uri.dropFirst())]!) } diff --git a/TestFoundation/TestURLSession.swift b/TestFoundation/TestURLSession.swift index 4cfab5da4a..77304dae40 100644 --- a/TestFoundation/TestURLSession.swift +++ b/TestFoundation/TestURLSession.swift @@ -29,6 +29,7 @@ class TestURLSession : LoopbackServerTest { ("test_timeoutInterval", test_timeoutInterval), ("test_httpRedirectionWithCompleteRelativePath", test_httpRedirectionWithCompleteRelativePath), ("test_httpRedirectionWithInCompleteRelativePath", test_httpRedirectionWithInCompleteRelativePath), + ("test_httpRedirectionWithDefaultPort", test_httpRedirectionWithDefaultPort), ("test_httpRedirectionTimeout", test_httpRedirectionTimeout), ("test_http0_9SimpleResponses", test_http0_9SimpleResponses), ("test_outOfRangeButCorrectlyFormattedHTTPCode", test_outOfRangeButCorrectlyFormattedHTTPCode), @@ -337,6 +338,14 @@ class TestURLSession : LoopbackServerTest { waitForExpectations(timeout: 12) } + func test_httpRedirectionWithDefaultPort() { + let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/redirect-with-default-port" + let url = URL(string: urlString)! + let d = HTTPRedirectionDataTask(with: expectation(description: "GET \(urlString): with HTTP redirection")) + d.run(with: url) + waitForExpectations(timeout: 12) + } + // temporarily disabled (https://bugs.swift.org/browse/SR-5751) func test_httpRedirectionTimeout() { let urlString = "http://127.0.0.1:\(TestURLSession.serverPort)/UnitedStates" @@ -893,6 +902,11 @@ extension HTTPRedirectionDataTask : URLSessionTaskDelegate { public func urlSession(_ session: URLSession, task: URLSessionTask, willPerformHTTPRedirection response: HTTPURLResponse, newRequest request: URLRequest, completionHandler: @escaping (URLRequest?) -> Void) { XCTAssertNotNil(response) XCTAssertEqual(302, response.statusCode, "HTTP response code is not 302") + if let url = response.url, url.path.hasSuffix("/redirect-with-default-port") { + XCTAssertEqual(request.url?.absoluteString, "http://127.0.0.1/redirected-with-default-port") + // Dont follow the redirect as the test server is not running on port 80 + return + } completionHandler(request) } }