Rename and clarify the body.collect(upTo:)
API which gets used in an insecure way all the time
#745
Labels
api-design
Issues related to API design
We see code similar to this a lot
The above code doesn't make any sense because it essentially says
This of course can lead to very bad scenarios because the
content-length
is controlled by the attacker. Furthermore, it's a lot of code to spell the ~equivalent ofAnd just to be clear, we do not have to check
content-length
here. The content length is correct, or else NIO would've spotted that and sent an error instead (like illegal EOF condition if it ended prematurely or so).But let's get to fixing this:
Goals:
Ideas:
body.collect()
would be safecollect(...)
should not be used. Only for JSON/protobuf/image/... decoding that needs to happen in memory we should use itInt
use a new type that can be constructed with integer literals or (and thanks to @hamzahrmalik for the idea) using some.init(watchOutWhatYourAreDoingIsDangerous: Int)
Maybe
body.collectIntoMemory()
-- safe: up to 4 MBbody.collectIntoMemory(maximumSize: .megabytes(16))
-- safe: up to 16 MBbody.collectIntoMemory(maximumSize: .bytes(contentLength))
-- compiler error: expected typeBodySize
, actual typeInt
body.collectIntoMemory(maximumSize: .bytes(BodySize(dangerousCalculatedSizeInBytes: contentLength))
-- unsafe but compilesThe text was updated successfully, but these errors were encountered: