Bug: POST from client code with incorrect header (no content-length / bad data written in request) crashes server

[riyadshauk]

If I post without a Content-Length in the header or write data as a UTF-8 string that doesn't conform to the Content-Type, the webserver crashes due to an assertion error. Using the auth-template as a starting point, although the client is first logged in as an authorized user in order to be able to make the POST, client code shouldn't be able to crash the back-end. Specifically, see Line 75 of HTTPChunkedStream.swift:

public func write(_ chunk: HTTPChunkedStreamResult) -> Future<Void> {
    if case .end = chunk {
        self.isClosed = true
    }

    if let handler = handler {
        return handler(chunk, self)
    } else {
        let promise = eventLoop.newPromise(Void.self)
        assert(waiting == nil)
        waiting = (chunk, promise)
        return promise.futureResult
    }
}

Corresponding to the following Error message: [ERROR] [HTTP] HTTPResponse sent while HTTPRequest had unconsumed chunked data.:

[vzsg]

If I post without a Content-Length in the header or write data as a UTF-8 string that doesn't conform to the Content-Type

Could you share some examples on how you perform either of these? They would make nice test cases.

[riyadshauk]

Sorry, I just noticed this response. It's kind of late right now, but maybe when I have more time. However, here's how you can recreate the issue:

$ git clone https://github.com/riyadshauk/coupon-retailer-shopper-webserver.git
$ cd coupon-retailer-shopper-webserver
$ # comment out line 314 of https://github.com/riyadshauk/coupon-retailer-shopper-webserver/blob/master/ClientAPI/clientAPI.ts (locally), the line with 'Content-Length'
$ vapor run
$ cd ClientAPI
$ npm run build1 && npm run pop # this will crash the server
$ # uncomment that line, and all works fine.

No guarantees that this repo is stable / will be up forever, but it'll be up for at least the next few days, and possibly just forever anyways. I might actually write some test cases for it later, though. Don't really have much time at the current moment to do that now.

[tanner0101]

I don't see a real problem with this behavior. The assert is there to help you find these problems early during development. When your app is compiled with -c release, the assert will be skipped.

[riyadshauk]

Ah, I see (good documentation). Sorry for my ignorance. I'll try to research a bit next time before worrying.