Skip to content

Commit f812a85

Browse files
CoeurCoeur
authored
suggestedFilename always returns a valid filename (#5019)
* suggestedFilename always returns a valid filename * re-sanitizing for supportsSecureCoding * code review: prefer `if let` to catch null case
1 parent 1acc48b commit f812a85

File tree

3 files changed

+83
-33
lines changed

3 files changed

+83
-33
lines changed

‎Sources/FoundationNetworking/URLResponse.swift‎

Lines changed: 40 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ open class URLResponse : NSObject, NSSecureCoding, NSCopying, @unchecked Sendabl
3535
guard let nsurl = aDecoder.decodeObject(of: NSURL.self, forKey: "NS.url") else { return nil }
3636
self.url = nsurl as URL
3737

38-
3938
if let mimetype = aDecoder.decodeObject(of: NSString.self, forKey: "NS.mimeType") {
4039
self.mimeType = mimetype as String
4140
}
@@ -46,8 +45,11 @@ open class URLResponse : NSObject, NSSecureCoding, NSCopying, @unchecked Sendabl
4645
self.textEncodingName = encodedEncodingName as String
4746
}
4847

49-
if let encodedFilename = aDecoder.decodeObject(of: NSString.self, forKey: "NS.suggestedFilename") {
50-
self.suggestedFilename = encodedFilename as String
48+
// re-sanitizing with lastPathComponent because of supportsSecureCoding
49+
if let encodedFilename = aDecoder.decodeObject(of: NSString.self, forKey: "NS.suggestedFilename")?.lastPathComponent, !encodedFilename.isEmpty {
50+
self.suggestedFilename = encodedFilename
51+
} else {
52+
self.suggestedFilename = "Unknown"
5153
}
5254
}
5355

@@ -177,6 +179,25 @@ open class URLResponse : NSObject, NSSecureCoding, NSCopying, @unchecked Sendabl
177179
/// protocol responses.
178180
open class HTTPURLResponse : URLResponse, @unchecked Sendable {
179181

182+
private static func sanitize(headerFields: [String: String]?) -> [String: String] {
183+
// Canonicalize the header fields by capitalizing the field names, but not X- Headers
184+
// This matches the behaviour of Darwin.
185+
guard let headerFields = headerFields else { return [:] }
186+
var canonicalizedFields: [String: String] = [:]
187+
188+
for (key, value) in headerFields {
189+
if key.isEmpty { continue }
190+
if key.hasPrefix("x-") || key.hasPrefix("X-") {
191+
canonicalizedFields[key] = value
192+
} else if key.caseInsensitiveCompare("WWW-Authenticate") == .orderedSame {
193+
canonicalizedFields["WWW-Authenticate"] = value
194+
} else {
195+
canonicalizedFields[key.capitalized] = value
196+
}
197+
}
198+
return canonicalizedFields
199+
}
200+
180201
/// Initializer for HTTPURLResponse objects.
181202
///
182203
/// - Parameter url: the URL from which the response was generated.
@@ -186,30 +207,13 @@ open class HTTPURLResponse : URLResponse, @unchecked Sendable {
186207
/// - Returns: the instance of the object, or `nil` if an error occurred during initialization.
187208
public init?(url: URL, statusCode: Int, httpVersion: String?, headerFields: [String : String]?) {
188209
self.statusCode = statusCode
189-
190-
self._allHeaderFields = {
191-
// Canonicalize the header fields by capitalizing the field names, but not X- Headers
192-
// This matches the behaviour of Darwin.
193-
guard let headerFields = headerFields else { return [:] }
194-
var canonicalizedFields: [String: String] = [:]
195-
196-
for (key, value) in headerFields {
197-
if key.isEmpty { continue }
198-
if key.hasPrefix("x-") || key.hasPrefix("X-") {
199-
canonicalizedFields[key] = value
200-
} else if key.caseInsensitiveCompare("WWW-Authenticate") == .orderedSame {
201-
canonicalizedFields["WWW-Authenticate"] = value
202-
} else {
203-
canonicalizedFields[key.capitalized] = value
204-
}
205-
}
206-
return canonicalizedFields
207-
}()
208-
210+
211+
self._allHeaderFields = HTTPURLResponse.sanitize(headerFields: headerFields)
212+
209213
super.init(url: url, mimeType: nil, expectedContentLength: 0, textEncodingName: nil)
210-
expectedContentLength = getExpectedContentLength(fromHeaderFields: headerFields) ?? -1
211-
suggestedFilename = getSuggestedFilename(fromHeaderFields: headerFields) ?? "Unknown"
212-
if let type = ContentTypeComponents(headerFields: headerFields) {
214+
expectedContentLength = getExpectedContentLength(fromHeaderFields: _allHeaderFields) ?? -1
215+
suggestedFilename = getSuggestedFilename(fromHeaderFields: _allHeaderFields) ?? "Unknown"
216+
if let type = ContentTypeComponents(headerFields: _allHeaderFields) {
213217
mimeType = type.mimeType.lowercased()
214218
textEncodingName = type.textEncoding?.lowercased()
215219
}
@@ -222,13 +226,18 @@ open class HTTPURLResponse : URLResponse, @unchecked Sendable {
222226

223227
self.statusCode = aDecoder.decodeInteger(forKey: "NS.statusCode")
224228

225-
if aDecoder.containsValue(forKey: "NS.allHeaderFields") {
226-
self._allHeaderFields = aDecoder.decodeObject(of: NSDictionary.self, forKey: "NS.allHeaderFields") as! [String: String]
227-
} else {
228-
self._allHeaderFields = [:]
229-
}
229+
// re-sanitizing dictionary because of supportsSecureCoding
230+
self._allHeaderFields = HTTPURLResponse.sanitize(headerFields: aDecoder.decodeObject(of: NSDictionary.self, forKey: "NS.allHeaderFields") as? [String: String])
230231

231232
super.init(coder: aDecoder)
233+
234+
// re-sanitizing from _allHeaderFields because of supportsSecureCoding
235+
expectedContentLength = getExpectedContentLength(fromHeaderFields: _allHeaderFields) ?? -1
236+
suggestedFilename = getSuggestedFilename(fromHeaderFields: _allHeaderFields) ?? "Unknown"
237+
if let type = ContentTypeComponents(headerFields: _allHeaderFields) {
238+
mimeType = type.mimeType.lowercased()
239+
textEncodingName = type.textEncoding?.lowercased()
240+
}
232241
}
233242

234243
open override func encode(with aCoder: NSCoder) {

‎Tests/Foundation/TestHTTPURLResponse.swift‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,8 @@ class TestHTTPURLResponse: XCTestCase {
228228

229229
func test_NSCoding() {
230230
let url = URL(string: "https://apple.com")!
231-
let f = ["Content-Type": "text/HTML; charset=ISO-8859-4"]
231+
let f = ["Content-Type": "text/HTML; charset=ISO-8859-4",
232+
"Content-Disposition": "attachment; filename=fname.ext"]
232233

233234
let responseA = HTTPURLResponse(url: url, statusCode: 200, httpVersion: "HTTP/1.1", headerFields: f)!
234235
let responseB = NSKeyedUnarchiver.unarchiveObject(with: NSKeyedArchiver.archivedData(withRootObject: responseA)) as! HTTPURLResponse

‎Tests/Foundation/TestURLResponse.swift‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ class TestURLResponse : XCTestCase {
8181
func test_NSCoding() {
8282
let url = URL(string: "https://apple.com")!
8383
let responseA = URLResponse(url: url, mimeType: "txt", expectedContentLength: 0, textEncodingName: nil)
84-
let responseB = NSKeyedUnarchiver.unarchiveObject(with: NSKeyedArchiver.archivedData(withRootObject: responseA)) as! URLResponse
84+
let responseB = try! NSKeyedUnarchiver.unarchivedObject(ofClass: URLResponse.self, from: NSKeyedArchiver.archivedData(withRootObject: responseA))!
8585

8686
//On macOS unarchived Archived then unarchived `URLResponse` is not equal.
8787
XCTAssertEqual(responseA.url, responseB.url, "Archived then unarchived url response must be equal.")
@@ -91,6 +91,46 @@ class TestURLResponse : XCTestCase {
9191
XCTAssertEqual(responseA.suggestedFilename, responseB.suggestedFilename, "Archived then unarchived url response must be equal.")
9292
}
9393

94+
func test_NSCodingEmptySuggestedFilename() {
95+
let url = URL(string: "https://apple.com")!
96+
let responseA = URLResponse(url: url, mimeType: "txt", expectedContentLength: 0, textEncodingName: nil)
97+
98+
// archiving in xml format
99+
let archiver = NSKeyedArchiver(requiringSecureCoding: false)
100+
archiver.outputFormat = .xml
101+
archiver.encode(responseA, forKey: NSKeyedArchiveRootObjectKey)
102+
var plist = String(data: archiver.encodedData, encoding: .utf8)!
103+
104+
// clearing the filename in the archive
105+
plist = plist.replacingOccurrences(of: "Unknown", with: "")
106+
let data = plist.data(using: .utf8)!
107+
108+
// unarchiving
109+
let responseB = try! NSKeyedUnarchiver.unarchivedObject(ofClass: URLResponse.self, from: data)!
110+
111+
XCTAssertEqual(responseB.suggestedFilename, "Unknown", "Unarchived filename must be valid.")
112+
}
113+
114+
func test_NSCodingInvalidSuggestedFilename() {
115+
let url = URL(string: "https://apple.com")!
116+
let responseA = URLResponse(url: url, mimeType: "txt", expectedContentLength: 0, textEncodingName: nil)
117+
118+
// archiving in xml format
119+
let archiver = NSKeyedArchiver(requiringSecureCoding: false)
120+
archiver.outputFormat = .xml
121+
archiver.encode(responseA, forKey: NSKeyedArchiveRootObjectKey)
122+
var plist = String(data: archiver.encodedData, encoding: .utf8)!
123+
124+
// invalidating the filename in the archive
125+
plist = plist.replacingOccurrences(of: "Unknown", with: "invalid/valid")
126+
let data = plist.data(using: .utf8)!
127+
128+
// unarchiving
129+
let responseB = try! NSKeyedUnarchiver.unarchivedObject(ofClass: URLResponse.self, from: data)!
130+
131+
XCTAssertEqual(responseB.suggestedFilename, "valid", "Unarchived filename must be valid.")
132+
}
133+
94134
func test_equalWithTheSameInstance() throws {
95135
let url = try XCTUnwrap(URL(string: "http://example.com/"))
96136
let response = URLResponse(url: url, mimeType: nil, expectedContentLength: -1, textEncodingName: nil)

0 commit comments

Comments
 (0)