fix: only escaping invalid data, keep valid escaping untouched

httpswift · May 12, 2021 · 9c10ca4 · 9c10ca4
1 parent 718f82c
commit 9c10ca4
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 2 deletions.
diff --git a/Xcode/Sources/HttpParser.swift b/Xcode/Sources/HttpParser.swift
@@ -24,7 +24,7 @@ public class HttpParser {
         }
         let request = HttpRequest()
         request.method = statusLineTokens[0]
-        let encodedPath = statusLineTokens[1].addingPercentEncoding(withAllowedCharacters: .urlQueryAllowed) ?? statusLineTokens[1]
+        let encodedPath = Self.escapingInvalidURL(statusLineTokens[1])
         let urlComponents = URLComponents(string: encodedPath)
         request.path = urlComponents?.path ?? ""
         request.queryParams = urlComponents?.queryItems?.map { ($0.name, $0.value ?? "") } ?? []
@@ -38,7 +38,17 @@ public class HttpParser {
             request.body = try readBody(socket, size: contentLengthValue)
         }
         return request
+    }
+
+    /// only escaping invalid chars，valid encodedPath keep untouched
+    static func escapingInvalidURL(_ url: String) -> String {
+        var urlAllowed: CharacterSet {
+            var v = CharacterSet.urlQueryAllowed
+            v.insert(charactersIn: "?#%")
+            return v
         }
+        return url.addingPercentEncoding(withAllowedCharacters: urlAllowed) ?? url
+    }
 
     private func readBody(_ socket: Socket, size: Int) throws -> [UInt8] {
         return try socket.read(length: size)

diff --git a/Xcode/Tests/SwifterTestsHttpParser.swift b/Xcode/Tests/SwifterTestsHttpParser.swift
@@ -175,7 +175,7 @@ class SwifterTestsHttpParser: XCTestCase {
         XCTAssertEqual(resp?.headers["header2"], "2", "Parser should extract multiple headers from the request.")
 
         resp = try? parser.readHttpRequest(TestSocket("GET /some/path?subscript_query[]=1&subscript_query[]=2 HTTP/1.0\nContent-Length: 10\n\n1234567890"))
-        let queryPairs = resp?.queryParams ?? []
+        var queryPairs = resp?.queryParams ?? []
         XCTAssertEqual(queryPairs.count, 2)
         XCTAssertEqual(queryPairs.first?.0, "subscript_query[]")
         XCTAssertEqual(queryPairs.first?.1, "1")
@@ -185,5 +185,19 @@ class SwifterTestsHttpParser: XCTestCase {
         XCTAssertEqual(resp?.path, "/some/path", "Parser should extract HTTP path value from the status line.")
         XCTAssertEqual(resp?.headers["content-length"], "10", "Parser should extract Content-Length header value.")
 
+        resp = try? parser.readHttpRequest(TestSocket("GET /path[]/param?a[]=1&a[]=2&b=%20 HTTP/1.0\r\nContent-Length: 0\r\n\r\n"))
+        queryPairs = resp?.queryParams ?? []
+
+        XCTAssertEqual(resp?.path, "/path[]/param")
+        if queryPairs.count == 3 {
+            XCTAssertEqual(queryPairs[0].0, "a[]")
+            XCTAssertEqual(queryPairs[0].1, "1")
+            XCTAssertEqual(queryPairs[1].0, "a[]")
+            XCTAssertEqual(queryPairs[1].1, "2")
+            XCTAssertEqual(queryPairs[2].0, "b")
+            XCTAssertEqual(queryPairs[2].1, " ")
+        } else {
+            XCTFail("queryPairs count should be 3")
+        }
     }
 }