Skip to content

Commit

Permalink
Fix URI handling with multiple slashes and variable components. (#3143)
Browse files Browse the repository at this point in the history 
* Fix URI handling when at least one component is a variable.
* Use #filePath instead of #file in tests.
  • Loading branch information
@gwynne
gwynne committed Jan 24, 2024
1 parent d5025b3 commit 4942d74
Show file tree
Hide file tree
Showing 12 changed files with 100 additions and 67 deletions.
14 changes: 13 additions & 1 deletion Sources/Vapor/HTTP/Server/HTTPServerRequestDecoder.swift
View file
Expand Up @@ -38,10 +38,22 @@ final class HTTPServerRequestDecoder: ChannelDuplexHandler, RemovableChannelHand
case .head(let head):
switch self.requestState {
case .ready:
/// Note: It is critical that `URI.init(path:)` is used here, _NOT_ `URI.init(string:)`. The following
/// example illustrates why:
///
/// let uri1 = URI(string: "//foo/bar?a#b"), uri2 = URI(path: "//foo/bar?a#b")
///
/// print(uri1.host, uri1.path, uri1.query, uri1.fragment)
/// // Optional(foo) /bar a b
/// print(uri2.host, uri2.path, uri2.query, uri2.fragment)
/// // nil /foo/bar a b
///
/// The latter parse has the correct semantics for an HTTP request's URL (which, in the absence of an
/// accompanying scheme, should never have a host); the former follows strict RFC 3986 rules.
let request = Request(
application: self.application,
method: head.method,
url: .init(string: head.uri),
url: .init(path: head.uri),
version: head.version,
headersNoUpdate: head.headers,
remoteAddress: context.channel.remoteAddress,
Expand Down
17 changes: 12 additions & 5 deletions Sources/Vapor/Utilities/URI.swift
View file
Expand Up @@ -46,6 +46,9 @@ public struct URI: CustomStringConvertible, ExpressibleByStringInterpolation, Ha
try container.encode(self.string)
}

/// Create a ``URI`` by parsing a given string according to the semantics of [RFC 3986].
///
/// [RFC 3986]: https://datatracker.ietf.org/doc/html/rfc3986
public init(string: String = "/") {
self.components = URL(string: string).flatMap { .init(url: $0, resolvingAgainstBaseURL: true) }
}
Expand Down Expand Up @@ -83,11 +86,15 @@ public struct URI: CustomStringConvertible, ExpressibleByStringInterpolation, Ha
/// Percent encoding is added to each component (if necessary) automatically. There is currently no
/// way to change this behavior; use `URLComponents` instead if this is insufficient.
///
/// > Warning: For backwards compatibility reasons, if the `path` component is specified in isolation
/// > (e.g. all other components are `nil`), the path is parsed as if by the ``init(string:)`` initializer.
///
/// > Important: If the `path` does not begin with a `/`, one is prepended. This occurs even if the path
/// > is specified in isolation (as described above).
/// > Important: For backwards compatibility reasons, if the `path` component is specified in isolation
/// > (e.g. all other components are `nil`), the path is parsed as if by the ``init(string:)`` initializer,
/// > _EXCEPT_ that if the path begins with `//`, it will be treated as beginning with `/` instead, thus
/// > parsing the first path component as part of the path rather than as a host component. These semantics
/// > are suitable for parsing URI-like strings which are known to lack an authority component, such as the
/// > URI part of the first line of an HTTP request.
/// >
/// > In all cases, a `/` is prepended to the path if it does not begin with one, irrespective of whether or
/// > not the path has been specified in isolation as described above.
public init(
scheme: Scheme = .init(),
userinfo: String?,
Expand Down
20 changes: 10 additions & 10 deletions Sources/XCTVapor/XCTApplication.swift
View file
Expand Up @@ -128,7 +128,7 @@ extension XCTApplicationTester {
_ path: String,
headers: HTTPHeaders = [:],
body: ByteBuffer? = nil,
file: StaticString = #file,
file: StaticString = #filePath,
line: UInt = #line,
afterResponse: (XCTHTTPResponse) async throws -> ()
) async throws -> XCTApplicationTester {
Expand All @@ -150,7 +150,7 @@ extension XCTApplicationTester {
_ path: String,
headers: HTTPHeaders = [:],
body: ByteBuffer? = nil,
file: StaticString = #file,
file: StaticString = #filePath,
line: UInt = #line,
afterResponse: (XCTHTTPResponse) throws -> ()
) throws -> XCTApplicationTester {
Expand All @@ -172,7 +172,7 @@ extension XCTApplicationTester {
_ path: String,
headers: HTTPHeaders = [:],
body: ByteBuffer? = nil,
file: StaticString = #file,
file: StaticString = #filePath,
line: UInt = #line,
beforeRequest: (inout XCTHTTPRequest) async throws -> () = { _ in },
afterResponse: (XCTHTTPResponse) async throws -> () = { _ in }
Expand All @@ -188,7 +188,7 @@ extension XCTApplicationTester {
let response = try self.performTest(request: request)
try await afterResponse(response)
} catch {
XCTFail("\(error)", file: (file), line: line)
XCTFail("\(error)", file: file, line: line)
throw error
}
return self
Expand All @@ -200,7 +200,7 @@ extension XCTApplicationTester {
_ path: String,
headers: HTTPHeaders = [:],
body: ByteBuffer? = nil,
file: StaticString = #file,
file: StaticString = #filePath,
line: UInt = #line,
beforeRequest: (inout XCTHTTPRequest) throws -> () = { _ in },
afterResponse: (XCTHTTPResponse) throws -> () = { _ in }
Expand All @@ -216,7 +216,7 @@ extension XCTApplicationTester {
let response = try self.performTest(request: request)
try afterResponse(response)
} catch {
XCTFail("\(error)", file: (file), line: line)
XCTFail("\(error)", file: file, line: line)
throw error
}
return self
Expand All @@ -227,7 +227,7 @@ extension XCTApplicationTester {
_ path: String,
headers: HTTPHeaders = [:],
body: ByteBuffer? = nil,
file: StaticString = #file,
file: StaticString = #filePath,
line: UInt = #line,
beforeRequest: (inout XCTHTTPRequest) async throws -> () = { _ in }
) async throws -> XCTHTTPResponse {
Expand All @@ -241,7 +241,7 @@ extension XCTApplicationTester {
do {
return try self.performTest(request: request)
} catch {
XCTFail("\(error)", file: (file), line: line)
XCTFail("\(error)", file: file, line: line)
throw error
}
}
Expand All @@ -251,7 +251,7 @@ extension XCTApplicationTester {
_ path: String,
headers: HTTPHeaders = [:],
body: ByteBuffer? = nil,
file: StaticString = #file,
file: StaticString = #filePath,
line: UInt = #line,
beforeRequest: (inout XCTHTTPRequest) throws -> () = { _ in }
) throws -> XCTHTTPResponse {
Expand All @@ -265,7 +265,7 @@ extension XCTApplicationTester {
do {
return try self.performTest(request: request)
} catch {
XCTFail("\(error)", file: (file), line: line)
XCTFail("\(error)", file: file, line: line)
throw error
}
}
Expand Down
17 changes: 7 additions & 10 deletions Sources/XCTVapor/XCTHTTPResponse.swift
View file
Expand Up @@ -47,15 +47,12 @@ extension Response.Body {
public func XCTAssertContent<D>(
_ type: D.Type,
_ res: XCTHTTPResponse,
file: StaticString = #file,
file: StaticString = #filePath,
line: UInt = #line,
_ closure: (D) throws -> ()
)
rethrows
where D: Decodable
{
) rethrows where D: Decodable {
guard let contentType = res.headers.contentType else {
XCTFail("response does not contain content type", file: (file), line: line)
XCTFail("response does not contain content type", file: file, line: line)
return
}

Expand All @@ -65,14 +62,14 @@ public func XCTAssertContent<D>(
let decoder = try ContentConfiguration.global.requireDecoder(for: contentType)
content = try decoder.decode(D.self, from: res.body, headers: res.headers)
} catch {
XCTFail("could not decode body: \(error)", file: (file), line: line)
XCTFail("could not decode body: \(error)", file: file, line: line)
return
}

try closure(content)
}

public func XCTAssertContains(_ haystack: String?, _ needle: String?, file: StaticString = #file, line: UInt = #line) {
public func XCTAssertContains(_ haystack: String?, _ needle: String?, file: StaticString = #filePath, line: UInt = #line) {
let file = (file)
switch (haystack, needle) {
case (.some(let haystack), .some(let needle)):
Expand All @@ -86,11 +83,11 @@ public func XCTAssertContains(_ haystack: String?, _ needle: String?, file: Stat
}
}

public func XCTAssertEqualJSON<T>(_ data: String?, _ test: T, file: StaticString = #file, line: UInt = #line)
public func XCTAssertEqualJSON<T>(_ data: String?, _ test: T, file: StaticString = #filePath, line: UInt = #line)
where T: Codable & Equatable
{
guard let data = data else {
XCTFail("nil does not equal \(test)", file: (file), line: line)
XCTFail("nil does not equal \(test)", file: file, line: line)
return
}
do {
Expand Down
8 changes: 4 additions & 4 deletions Tests/AsyncTests/AsyncPasswordTests.swift
View file
Expand Up @@ -59,7 +59,7 @@ final class AsyncPasswordTests: XCTestCase {
private func assertAsyncApplicationPasswordVerifies(
_ provider: Application.Passwords.Provider,
on app: Application,
file: StaticString = #file,
file: StaticString = #filePath,
line: UInt = #line
) async throws {
app.passwords.use(provider)
Expand All @@ -72,13 +72,13 @@ final class AsyncPasswordTests: XCTestCase {
.async(on: app.threadPool, hopTo: app.eventLoopGroup.next())
.verify("vapor", created: asyncHash)

XCTAssertTrue(asyncVerifiy, file: (file), line: line)
XCTAssertTrue(asyncVerifiy, file: file, line: line)
}

private func assertAsyncRequestPasswordVerifies(
_ provider: Application.Passwords.Provider,
on app: Application,
file: StaticString = #file,
file: StaticString = #filePath,
line: UInt = #line
) throws {
app.passwords.use(provider)
Expand All @@ -90,7 +90,7 @@ final class AsyncPasswordTests: XCTestCase {
}

try app.test(.GET, "test", afterResponse: { res in
XCTAssertEqual(res.body.string, "true", file: (file), line: line)
XCTAssertEqual(res.body.string, "true", file: file, line: line)
})
}
}
2 changes: 1 addition & 1 deletion Tests/VaporTests/DotEnvTests.swift
View file
Expand Up @@ -10,7 +10,7 @@ final class DotEnvTests: XCTestCase {
let pool = NIOThreadPool(numberOfThreads: 1)
pool.start()
let fileio = NonBlockingFileIO(threadPool: pool)
let folder = #file.split(separator: "/").dropLast().joined(separator: "/")
let folder = #filePath.split(separator: "/").dropLast().joined(separator: "/")
let path = "/" + folder + "/Utilities/test.env"
let file = try DotEnvFile.read(path: path, fileio: fileio, on: elg.next()).wait()
let test = file.lines.map { $0.description }.joined(separator: "\n")
Expand Down
6 changes: 3 additions & 3 deletions Tests/VaporTests/EnvironmentSecretTests.swift
View file
Expand Up @@ -4,7 +4,7 @@ import XCTest

final class EnvironmentSecretTests: XCTestCase {
func testNonExistingSecretFile() throws {
let folder = #file.split(separator: "/").dropLast().joined(separator: "/")
let folder = #filePath.split(separator: "/").dropLast().joined(separator: "/")
let path = "/" + folder + "/Utilities/non-existing-secret"

let app = Application(.testing)
Expand All @@ -16,7 +16,7 @@ final class EnvironmentSecretTests: XCTestCase {
}

func testExistingSecretFile() throws {
let folder = #file.split(separator: "/").dropLast().joined(separator: "/")
let folder = #filePath.split(separator: "/").dropLast().joined(separator: "/")
let path = "/" + folder + "/Utilities/my-secret-env-content"

let app = Application(.testing)
Expand All @@ -28,7 +28,7 @@ final class EnvironmentSecretTests: XCTestCase {
}

func testExistingSecretFileFromEnvironmentKey() throws {
let folder = #file.split(separator: "/").dropLast().joined(separator: "/")
let folder = #filePath.split(separator: "/").dropLast().joined(separator: "/")
let path = "/" + folder + "/Utilities/my-secret-env-content"

let key = "MY_ENVIRONMENT_SECRET"
Expand Down
2 changes: 1 addition & 1 deletion Tests/VaporTests/ErrorTests.swift
View file
Expand Up @@ -99,7 +99,7 @@ final class ErrorTests: XCTestCase {
func XCTAssertContains(
_ haystack: String?,
_ needle: String,
file: StaticString = #file,
file: StaticString = #filePath,
line: UInt = #line
) {
let file = (file)
Expand Down

0 comments on commit 4942d74

Please sign in to comment.