MultipartKit V5

[ptoffy]

No description provided.

[adam-fowler]

Some initial comments.

[ptoffy]

@adam-fowler Do you want to take a look at this again and see if stuff makes more sense now? I added in some binary data (which even contains hex-CRLF 😄) to the tests so it should be able to parse anything now

[ptoffy]

@Joannis @simonjbeaumont @czechboy0 pulling you into this so you can take a look if you want

[Joannis]

This makes a copy from string, can't we use withContiguousMemoryIfAvailable?

[ptoffy]

Mhh I don't think that would work any better because we need the Collection there. We can't pass in the raw bytes and if we're copying them to an array we're still making a copy at that point right?

[Joannis]

Since you're doing an .append on the parser you're right that you're already making a copy. Except right now you're making two copies of the same data, and each time you're also allocating space for that data.

[ptoffy]

No, I mean we can't pass the raw pointer to the decode method because it expects the collection of bytes, so this can't be done

try string.utf8.withContiguousStorageIfAvailable { bytes in
    decode(D.self, from: bytes, boundary: boundary)
} ?? decode(D.self, from: Array(string.utf8), boundary: boundary)

And if we were to do something like

if let bytes = string.utf8.withContiguousStorageIfAvailable(Array.init) {
    try decode(D.self, from: bytes, boundary: boundary)
} else {
    try decode(D.self, from: Array(string.utf8), boundary: boundary)
}

we're still initialising an array with the raw bytes so I think this doesn't really save us a copy.
Unless you mean a different way of using withContiguousStorageIfAvailable

[Joannis]

Is it necessary to copy data out before parsing?

[ptoffy]

This way we avoid a whole bunch of needMoreData returns from the parser

[Joannis]

Could it be restructured to be parseOrAppend then?

if parserBuffer.isEmpty {
  let result = parser.parse(data)
  if result == .needMoreData {
    parser.append(data)
  }
} else {
  parser.append(data)
  parser.parse()
}

[ptoffy]

But why? We're just doing that once at the beginning, this is the sync parse

[adam-fowler]

Generic parameter changes look good

[adam-fowler]

As I understand it the user can receive a body as multiple MultipartSections if the underlying AsyncSequence has broken that body up. This is great as we don't want to pay the memory for large bodies if we can. But there are situations where we want to ensure we have a complete body section eg a block of data we want to run a JSON decode on. Is it possible to add a helper function to the Iterator to do this? eg Iterator.collectBody(upTo: memoryLimit) which returns a header, complete body section

[ptoffy]

There's currently a parse method on MultipartParser which loads the input all at once. I'm guessing you mean something of a middle ground between this and stream parsing? E.g just for one part of the message

[adam-fowler]

Yes just one part of the message eg I have a Multipart message with a zip file in there plus some metadata. I want to save the zip files to disk (using the least amount of memory possible ie streaming it) but parse the metadata with Codable so need the whole of it in memory.