Request body as `io::Read`

[Ddystopia]

Hello,

we use picoserve for updating firmware and configurations on embedded devices. Our updates are large, and storing the entire body in RAM is resource-intensive.

We suggest enhancing request.body() to implement io::Read instead of holding the entire payload in memory. Our devices support async writes, and streaming the body directly would greatly reduce RAM usage, benefiting not only us but anyone with similar embedded device constraints.

This change would streamline operations for large payloads, making picoserve more efficient for embedded systems.

[Ddystopia]

I guess it contradicts #19

[Ddystopia]

I made some version of it, like that:

• Removed body from Request struct
• Removed Connection from Response writer
• Later on passing Connection to types that actually require it
• Before it passing reader down to handlers

Right now I modified FromRequest trait to accept a reader too, but I guess it doesn't make a lot of sense. I guess better solution would be to let it as it is, so implementors would now not have access to the body and add new trait that would get a reader. Than in that huge triangular macro it can impl for an additional type of handler, with that trait. So 0 or 1 types that implement that new trait that has access to the body reader.

Also I'm not sure how to deal with content length. I guess a reader handed to that trait implementation could limit read bytes to content length and also count how many bytes were read, so that if implemetor did not consume body, some other code would consume it and discard.

[Ddystopia]

What is your opinion about it? Would you consider merging if I made pull request?

[sammhicks]

I'm definitely interested. I'm currently also working on what is hopefully a solution, but a different solution as well will highlight stuff that I've missed or not thought of, and the final solution can be the best of both approaches. Thanks for the help :)

[Ddystopia]

Okay. Right now it is not in a beautiful state with generics splattered all over the code (I already see that in .*write.* traits/methods it could be rolled back), but it was without any design thought, just to get it working. Also I made request::Reader to read only by 1 byte so that it does not consume body, but BodyReader could have a slice to already read body and return it first before using underlying reader.

[sammhicks]

Thanks for the code share, it's been a great help! Your design of having the routing methods be generic over the connection read type is much neater than the spaghetti that I ended up with which included the read type being part of the trait themselves. I also liked the mechanism of having a body reader that you finalize and then pass to the ResponseWriter :)

I've put the initial design in the branch request-body-read, and have put an example of streaming data called huge_requests. Roughly speaking, it has the following new features:

• Request Bodies are no longer automatically read and passed as a slice of bytes, but passed as a RequestBody structure
  • RequestBody has two methods
  • read_all will read the entire body into the remaining space in request buffer, effectively the same as the current functionality. Useful for small requests
  • reader with return a RequestBodyReader, which implements embedded_io_async::Read. When reading from it, it starts by copying across the data that was buffered while reading the headers, then reads directly from the network connection, bypassing the buffer. This allows it to potentially process huge files, such as the firmware upload that you've previously mentioned. RequestBodyReader automatically reports EOF when it's at the end of the request.
• If a connection is upgraded, it's always closed once it's been handled.
• Similar to axum, I've separated FromRequest into FromRequestParts (which has access to path, headers, etc, but not the body) and FromRequest (which also has access to the body). The last parameter of a handler function must be the one which implements FromRequest.

TODO:

• Currently if the client sends data to an upgraded connection before waiting for a response (as might happen in tests for example), data might be lost. I'm planning of having a similar reader as to the request body reader which starts by copying across data in the request buffer.
• I'm thinking of giving the Connection mutable access to the remaining area of the request buffer. For example, an upgraded WebSocket connection might use the remaining area to read messages into instead of allocating a separate buffer, thus being more memory-efficient.
• Given how complex the reading code now is, it's high time that I created some tests to verify functionality.

[sammhicks]

Update: I've found several bugs with the new implementation, currently investigating and fixing...

[Ddystopia]

Thank you!