Two handy extensions to work with HTTPClientResponse body

[makoni]

Two extensions for HTTPClientResponse to avoid boilerplate code.

Reading a response data currently:

let expectedBytes = headers.first(name: "content-length").flatMap(Int.init) ?? 1024 * 1024
var bytes = try await body.collect(upTo: expectedBytes)
if let data = bytes.readData(length: bytes.readableBytes) {
    // handle data
}

With the extension:

if let data = try await response.data(upTo: 1024*1024) {
    // handle data
}

Additionally an extension for bytes. Currently:

let expectedBytes = headers.first(name: "content-length").flatMap(Int.init) ?? 1024 * 1024
var bytes = try await body.collect(upTo: expectedBytes)

With the extension:

var bytes = try await response.bytes(upTo: 1024*1024)

[Lukasa]

Generally these look really good! Can you add a couple of tests for them as well?

[Lukasa]

I like this helper, but I think it might be a bit better as a function. In particular I'm very reluctant to let users avoid providing the default maximum body size: just hard-coding this to 1MB is probably going to cause some confusion.

[makoni]

I'm not really sure about a default value, but at the same time I found that sometimes there's not "content-length" header. I'm working on a small client lib for CouchDB which has HTTP API. And some of its responses doesn't have that header. But with some default value it works fine.

Do you have any other ideas how to avoid hard-coding?

[makoni]

I think I found a solution:

public func bytes(upTo maxBytes: Int? = nil) async throws -> ByteBuffer {
    if let expectedBytes = maxBytes ?? self.headers.first(name: "content-length").flatMap(Int.init) {
        return try await self.body.collect(upTo: expectedBytes)
    }

    var data = [UInt8]()
    for try await var buffer in self.body {
        data = data + (buffer.readBytes(length: buffer.readableBytes) ?? [])
    }

    return ByteBuffer(bytes: data)
}

[makoni]

Added 2 new tests

[fabianfett]

I'm not sure we should automatically depend on "content-length" here. A malicious client may send just peta bytes of data, which can bring down the client.

[makoni]

If you're getting an image from some URL you probably don't know is its size 1mb or 20mb. If you'll pass too small upTo value there will be a crash.

Here's an example that gives Swift/ErrorType.swift:200: Fatal error: Error raised at top level: NIOCore.NIOTooManyBytesError() with 1.21.0:

let request = HTTPClientRequest(url: "https://github.githubassets.com/images/modules/logos_page/GitHub-Mark.png")
let response = try await HTTPClient.shared.execute(request, timeout: .seconds(30))
let buffer = try await response.body.collect(upTo: 10)

If you don't know the data size it makes sense to provide some big value (like 1012 * 1012 * 100 for 100MB for example), or to use "content-length" header that tells you the size of the data.

If you don't trust the source it's still possible to provide upTo param in the public func bytes(upTo maxBytes: Int? = nil) async throws -> ByteBuffer method above.

Do you have a better idea? 🙂

[Lukasa]

A better version might be to provide a maximum value the user is willing to accept here, and then use content-length or the maximum value. We can then throw errors if the content-length is larger than the max.

[makoni]

Thanks Lukasa. I've updated the PR, made upTo param required for both helper functions as you suggested. If the content-length is larger than the max there will be an error thrown. Also updated the tests.

[Lukasa]

Mild nit: our style is to use self. wherever relevant.

[makoni]

Done

[Lukasa]

This shouldn't just blindly trust the content-length: it should reject values of the content length above the upTo value.

[Lukasa]

Same note here.