- Sat Mar 07, 2015 4:40 am
#11520
@prozac I think we're pretty much describing the same thing with just a couple of minor details.
I implemented a system where you declare your cgi handler to be streaming, but I agree with you that there shouldn't really be a difference and that all cgi handlers should operate in the same way. So we would make the current streaming way of handling data as it is in my branch the default. Have I interpreted what you're saying correctly?
Regarding your points:
Point 1: I agree - we should just stream it out to the handler rather than chopping the body as is currently the method
Point 2: Not sure I get your point here - it makes some sense to me when parsing http requests to size the body buffer when we get the header that says how big it should be, and if it's bigger than the max buffer size then it's a streaming request.
Point 3: I'm split on this - on the one hand it's nice to be able to just take the whole packet if we size the buffer appropriately, on the other it makes me uneasy relying on the tcp packets having a fixed maximum size because if there's one thing I've learned it's that things in the real world don't operate according to the specs
So I think going back to 1024 bytes might be a good plan and just chunking the data up. However, this still won't necessarily give you nicely aligned data because, for example, when uploading a new file using multipart form encoding there's a chunk of secondary headers and delimiters that will throw this off, so I was expecting to have our own secondary buffer before writing to flash to deal with this.
Point 4: Agree, though we should rename it because it's no longer necessarily sending the response. Maybe something like dispatchRequest
Point 5: No need to redimension, just send it to dispatchRequest (or whatever it's called) and then start again from zero. For the chunk counter, I had a variable which, rather than chunks, counts the number of bytes that have been sent so far (postReceived) which can be used for the same thing.
Point 6: We could always autodetect this as we're parsing through the body and then set the flag appropriately?
Point 7: Agree - this makes more sense
The version in my branch can already stream any amount of data - I've tested it with a few hundred kB binary file and it seems OK. But I think that the combined approach makes sense to me as there are less special cases and we can treat all post request the same. Any code gratefully received - I was wondering about setting up a dummy pull request over on GitHub so we can take the conversation out of this epic thread - in fact, I'll do that now - check out
https://github.com/bjpirt/esphttpd/pull/3@Sprite_tm as the esp-httpd BDFL, what are your thoughts on moving all POST handling to being streaming?
