RFC 6455 section 4.2.1 prescribes which headers are mandatory for a WebSocket handshake request to be correct. It also details what each of these headers should contain. This PR brings WebSocketHTTPHander up to spec (which has a typo by the way, but I didn't change that to keep this PR focussed).

I have documented the new code in a way that makes it easy to match up each check with its corresponding RFC rule (see WebSocketHTTPHander.verifyHandshakeRequestHeaders).

There is one conundrum that I ran into while implementing the checks, which is that rule 1 mandates that websocket connections must begin as GET requests, however in FlyingFox the method for websocket endpoints is up to users of the framework. We could add some kind of check to appendRoute, or we could just leave it given that developers will probably realise pretty quickly that their websocket won't work as a POST endpoint. Another option would be creating a separate method for hosting websocket endpoints such as appendWebSocketRoute which takes only a path and doesn't let developers specify the method.

Create a function to enable add batch handlers. Use case:

server.appendRoutes([
            "/fish/accepted" => ClosureHTTPHandler { _ in
                HTTPResponse.make(statusCode: .accepted)
            },
            "/fish/*" => .file(named: "fish.json", in: .module)
        ])

The issue was caused by incorrect handling of data slices with non-zero start indices and could lead to arbitrary memory being leaked to the socket. See #26 for details.

As per RFC-3986 5.2.4 (Remove Dot Segments), we want to remove ..s. We can use Foundation's URL.standardized features for that.

However, I am sleepy, so this may have missed something obvious.

Fixes swhitty/FlyingFox/issues/24

I'd like to use this server in some tests. The tests might be executed in parallel, so I'm not able to hard-code specific port numbers.

If you bind a socket address with port number 0, the system will assign a unique port number for you, which is ideal for these situations. Unfortunately, AFAICT, there is no way to get the port number or socket from HTTPServer, so I wouldn't be able to actually make requests to that server instance.

What is a path traversal vulnerability?

A path traversal vulnerability occurs when paths aren't properly normalised by the server, allowing attackers to access any file on the server, not just files within the served directory.

Proof of concept

The following code snippet for creating a simple file server is vulnerable to path traversal (and so is any other code that uses DirectoryHTTPHandler).

let directoryHandler = DirectoryHTTPHandler(root: URL(fileURLWithPath: "."), serverPath: "/")
let server = HTTPServer(port: 80)
await server.appendRoute("GET *", to: directoryHandler)
try await server.start()

To observe the effect of this vulnerability, run the code snippet above and then run the following command in terminal to see that the server has exposed the contents of your machine's /etc/passwd file (and all other files for that matter):

curl --path-as-is http://127.0.0.1/../../../../../../../../../../../etc/passwd

Any user that can access the web server can run a similar command on their machine to access any file on the server (within the limits of the privileges that the server is running with).

Mitigation

Option 1 (easy, but isn't as nice as option 2)

Abort any requests to the directory handler that contain ../ in their path. This is how vapor's file server middleware avoids path traversal (the following snippet is taken from vapor source). This only fixes the issue for the built-in directory handler, but the check could be applied to all requests (not just those handled by the directory handler) to fix that.

// protect against relative paths
guard !path.contains("../") else {
    return request.eventLoop.makeFailedFuture(Abort(.forbidden))
}

I don't like this solution because it feels like a bit of a quick fix, but another way of looking is that its simplicity makes it very hard to mess up.

Option 2

Normalize request paths in HTTPDecoder to remove any .. components. Refer to the RFC specification on removing dot components (https://www.rfc-editor.org/rfc/rfc3986#section-5.2.4).

This solution is a bit more involved, but it results in nicer behaviour in my opinion, and I think it's the approach that most http servers take.

Conclusion

Let me know which solution you prefer. If you would like me to implement the fix, I'd be happy to implement either solution. Personally I prefer the behaviour of the second solution but the simplicity of the first, so it's up to you.

If I try to use FlyingFox and embed some HTML with CSS in my app and then load the content via a WebKit browser instance, the HTML content does not load correctly because the CSS does not get loaded.

It appears that the makeContentType method in FileHTTPHandler does not return the right type for CSS files. If you add the following case to the method, the HTML content renders correctly:

case "css":
	return "text/css"

The new stop method on HTTPServer will close the socket so that no new connections are handled, but waits for the existing requests to finish before start returns.

This is similar to SwiftNIO's Quiescing capability available in apple/swift-nio-extras

I was testing FlyingFox on Xcode 14 (planning on offering/contributing some new features, love this project :) ) and found that the tests crashed on some unsafePointer stuff, and traced it to this PR: https://github.com/apple/swift/pull/42002

Most likely the usage of the new inout APIs will be considered wrong, so wanted to send in this to help in the mean time.

Feel free to reject on the basis that Xcode 14 is beta and that you only support mainline builds of Xcode, but I think the unsafePointer changes are safe to include, on the basis that it enforces the usage of the UnsafePointer API and not the inout version.

The convenience removal was also a warning in Xcode 14 beta 4 (at least), unclear if that will be an issue in previous versions of Xcode (personal computer only has the latest edge macOS and Xcode versions :p )

Overview

If the data passed to Socket.write is a slice with a non-zero startIndex, memory after the end of the data buffer will be leaked to the recipient.

Cause

The issue is on line 229 of Socket.swift:

let sent = try write(buffer.baseAddress! + index, length: data.endIndex - index)

The code assumes that buffer.baseAddress! + index correctly gets the byte at index in the data, however baseAddress points to the byte at startIndex not at index 0. For example, consider the following code:

let data = "abcd".data(using: .utf8)!
let slice = data[2...] // contains "cd"

let index = slice.startIndex
try slice.withUnsafeBytes { buffer in               
    _ = try write(buffer.baseAddress! + index, length: data.endIndex - index)
    //                   (1)            (2)            (3)
    //
    // 1. baseAddress points to "c"
    // 2. after adding startIndex (which is 2), the pointer points to the byte after the end of the buffer
    // 3. length is 2 (4 - 2)
    //
    // In this example scenario, the server accidentally sends two bytes of
    // the memory after the end of the data buffer to the client, which could
    // lead to sensitive data being leaked in certain setups. It could also potentially
    // be combined with certain other types of vulnerabilities to execute arbitrary code.
}

Proof of concept

First, run the following command to start a tcp listener that simply prints out any data it receives.

nc -l 8080

Next, run this code snippet with swift run for the highest chance of reproducing, because that's how I ran it:

@main
struct FlyingFoxDataTest {
    static func main() async throws {
        // Generate some dummy data and make a slice
        let data = String(repeating: "abcd", count: 32).data(using: .utf8)!
        let slice = data[64...]
        
        // This length of string seems to work consistently on my laptop
        let secretPassword = "thisismyverylongandsecuresecretpasswordthisismyverylongandsecuresecretpasswordthisismyverylongandsecuresecretpassworditissogood!".data(using: .utf8)!
        
        // Attempt to send the slice through the socket
        let socket = try await AsyncSocket.connected(to: .inet(ip4: "127.0.0.1", port: 8080), pool: .polling)
        try await socket.write(slice)
    }
}

When I run that snippet, I get the following output:

$ nc -l 8080
thisismyverylongandsecuresecretpasswordthisismyverylongandsecure

As you can see, it sent the first half of secretPassword instead of the contents of the data slice. This bug could have pretty bad side-effects if it appeared in any unfortunate situations.

Mitigation

Make the following change:

- let sent = try write(buffer.baseAddress! + index, length: data.endIndex - index)
+ let sent = try write(buffer.baseAddress! + index - data.startIndex, length: data.endIndex - index)

I'll make a PR to fix this soon.

And I know this full on bug report might be a bit overkill, but I had fun getting that proof of concept to work, so I did it anyway.

Hi Simon,

I'm running into an issue with FlyingFox when building my project. The problem only arises when I specifically direct Swift to build it for macOS 12.0 (the reason I'm doing that is because I'm installing my tool through mint, which automatically targets the machine's os version). Here's the error:

$ swift build -c release -Xswiftc -target -Xswiftc x86_64-apple-macosx12.0
Building for production...
/Users/stackotter/Desktop/Projects/Swift/Scute/.build/checkouts/FlyingFox/Sources/Handlers/ProxyHTTPHandler.swift:51:42: error: ambiguous use of 'data'
        let (data, response) = try await session.data(for: req)
                                         ^
/Users/stackotter/Desktop/Projects/Swift/Scute/.build/checkouts/FlyingFox/Sources/URLSession+Async.swift:42:10: note: found this candidate
    func data(for request: URLRequest, forceFallback: Bool = false) async throws -> (Data, URLResponse) {
         ^
Foundation.URLSession:3:17: note: found this candidate
    public func data(for request: URLRequest, delegate: URLSessionTaskDelegate? = nil) async throws -> (Data, URLResponse)
                ^
[3/7] Compiling FlyingFox AsyncSequence+Extensions.swift

It seems like you may have to rename your method or change the function signature in some other way so that it doesn't clash with the macOS 12.0 API additions,

Cheers, stackotter

When multiple endpoints are defined, one ends up with:

await server.appendRoute(HTTPRoute(.GET, "/cat"), to: MyCatGetHandler())
await server.appendRoute(HTTPRoute(.GET, "/dog"), to: MyDogGetHandler())
await server.appendRoute(HTTPRoute(.GET, "/fish"), to: MyFishGetHandler())

and then implementations contains only the handler:

struct MyCatGetHandler : HTTPHandler {
    func handleRequest(_ request: FlyingFox.HTTPRequest) async throws -> FlyingFox.HTTPResponse { }
}

struct MyDogGetHandler : HTTPHandler {
    func handleRequest(_ request: FlyingFox.HTTPRequest) async throws -> FlyingFox.HTTPResponse { }
}

struct MyFishGetHandler : HTTPHandler {
    func handleRequest(_ request: FlyingFox.HTTPRequest) async throws -> FlyingFox.HTTPResponse { }
}

Maybe it's my personal taste, but I find it better to have both the route and the handler in the same file, so by adding those extensions:

protocol RouteHandler : HTTPHandler {
    func route() -> HTTPRoute
}

extension HTTPServer {
    func appendRoutes(_ routeHandlers: RouteHandler...) {
        routeHandlers.forEach {
            appendRoute($0.route(), to: $0)
        }
    }
}

I'm able to write each handler like this (which I find convenient, specially when the route has placeholders, so I can write a single common function and define shared constants between the route and the request handler):

struct MyCatGetHandler : RouteHandler {

    func route() -> HTTPRoute {
        HTTPRoute(method: .GET, path: "/cat")
    }

    func handleRequest(_ request: FlyingFox.HTTPRequest) async throws -> FlyingFox.HTTPResponse {
        // implementation
    }
}

and also be able to add them all like this:

await server.appendRoutes(
    MyCatGetHandler(),
    MyDogGetHandler(),
    MyFishGetHandler()
)

If you think it's a valuable addition, feel free to add it in the library 🍻

This is my third proposal so far (#50 #49), and the fact I was able to write everything I needed as an extension denotes really good design choices on your side, so kudos for the great work! 💯

JSON responses are common for a web server. With this extension (serializing dates as millis by default to ease JS clients):

extension HTTPResponse {
    static let jsonTimestampMillisEncoder: JSONEncoder = {
        let jsonEncoder = JSONEncoder()
        jsonEncoder.dateEncodingStrategy = .millisecondsSince1970

        return jsonEncoder
    }()

    static func json(_ encodable: Encodable, jsonEncoder: JSONEncoder = jsonTimestampMillisEncoder) throws -> HTTPResponse {
        HTTPResponse(statusCode: .ok, body: try jsonEncoder.encode(encodable))
    }
}

one can easily return a JSON ready to be consumed by another application:

struct Cat: Codable {
    let birthDate: Date
    let name: String
}

struct MyHandler : HTTPHandler {
    func handleRequest(_ request: HTTPRequest) async throws -> HTTPResponse {
        try .json(Cat(birthDate: Date(), name: "Loris"))
    }
}

If you think it's a valuable addition, feel free to add it in the library 🍻

First of all, thanks for this really good library!

With this setup:

let server = HTTPServer(port: 8080)

let root = URL(fileURLWithPath: "/Users/myUser/myStaticDir")

let route = HTTPRoute(method: .GET, path: "/static/*")
let handler = DirectoryHTTPHandler(root: root, serverPath: "static")

await server.appendRoute(route, to: handler)

try await server.start()

When I perform:

curl -L -vv http://localhost:8080/static/index.html

I'm always getting a 404.

That's because: https://github.com/swhitty/FlyingFox/blob/8ef9eddda8f4ea370233454eded2ceded1fb76ee/FlyingFox/Sources/Handlers/DirectoryHTTPHandler.swift#L51 never matches, because request.path will always be /static/index.html, so I find this example to be wrong: https://github.com/swhitty/FlyingFox#directoryhttphandler

To make it work I had to write the handler like this:

let handler = DirectoryHTTPHandler(root: root, serverPath: "/static/")

But then I found out that if I want to also handle the default file to be index.html when not explicitly specifying it:

curl -L -vv http://localhost:8080/static/

or

curl -L -vv http://localhost:8080/static

I have to also add:

let fileRoute = HTTPRoute(method: .GET, path: "/static")
let fileHandler = FileHTTPHandler(path: root.appendingPathComponent("index.html"), contentType: "text/html")
await server.appendRoute(fileRoute, to: fileHandler)

Then I thought this is a very common scenario one would expect to work with minimal work, so I written this extension:

extension HTTPServer {
    func appendStaticRoute(_ route: String, root: URL, defaultFileName: String = "index.html", withContentType contentType: String = "text/html") {
        appendRoute(HTTPRoute(method: .GET, path: "/\(route)/*"), to: DirectoryHTTPHandler(root: root, serverPath: "/\(route)/"))
        appendRoute(HTTPRoute(method: .GET, path: "/\(route)/"), to: FileHTTPHandler(path: root.appendingPathComponent(defaultFileName), contentType: contentType))
        appendRoute(HTTPRoute(method: .GET, path: "/\(route)"), to: .redirect(to: "/\(route)/"))
    }

    func appendStaticRoute(_ route: String, bundle: Bundle = .main, defaultFileName: String = "index.html", withContentType contentType: String = "text/html") throws {
        guard let root = bundle.resourceURL else { throw HTTPUnhandledError() }
        appendStaticRoute(route, root: root, defaultFileName: defaultFileName, withContentType: contentType)
    }
}

Which allows you to achieve the same with only:

let server = HTTPServer(port: 8080)

let root = URL(fileURLWithPath: "/Users/myUser/myStaticDir")

await server.appendStaticRoute("static", root: root)

try await server.start()

And so here I am, posting this. If you think it's a valuable addition, feel free to add it in the library 🍻

First of all thanks for developing this amazing package!

I wanted to ask if it is possible to use http 2 with this package as I see the server is currently using HTTP/1.1.