Chunked encoding in requests

[MitchBradley]

I have implemented a WebDAV server in our FluidNC CNC firmware. It works well with Windows and curl clients, but uploads fail with MacOS because Mac's WebDAVFS code sends PUT data with Transfer-Encoding: Chunked which ESPAsyncWebServer does not handle. Has anyone looked into supporting chunking for requests too?

[mathieucarbou]

Hello,

Hi @MitchBradley, the server does indeed not support chunk encoding for requests.

But that's a valid use case and it would be nice to support it indeed.

Did you try to setup an upload handler for now ? What is happening ? I guess the upload handler receives all the raw data (chunk headers plus data and separators) right ?

As I understand the spec, all recipients should in theory support all encodings like gzip, chunked, etc but that being said, MCU having limitations, some could be expensive to support (like gzip).

For chunk, and since this is asynchronous, data can arrive in a non deterministic way (i.e. a chunk header could arrive in 2 separate packets). But I think only some states and counters would be required to parse that, as long as all the data is sent to the upload callback after.

If you have some MRE / sample that are easy to use and isolated to provide some use case or any starting point, or want to start a PR, feel free to do it / provide them. I think this is a kind of work that could be started in a branch and be worked on / completed as the time allows it.

@vortigont @me-no-dev @willmmiles WDYT ?

• https://greenbytes.de/tech/webdav/draft-ietf-httpbis-p1-messaging-26.html#transfer.codings
• http://www.webdav.org/specs/rfc2616.pdf
• https://evertpot.com/260/

[MitchBradley]

Since chunked encoding has no Content-Length header, WebRequest.cpp line 683 causes the body to be skipped. My current approach to fixing it involves a _chunkedRequest bool that is set when Transfer-Encoding: includes "chunked", with subsequent decoding of the body and calls to handleBody. I just started working on it so I am not very far along.

I think it will be easier for me to fix the problem than to create an MRE, as my current test code is deeply embedded in a large app - but at some point I will probably need a simple test case to validate a future PR.

[MitchBradley]

I suppose that it could handled by a middleware, but since chunked is a core requirement of HTTP/1.1, maybe it should be in the core code.

[mathieucarbou]

@MitchBradley if you are ready to have a solution then that's even better! Go for it, send a PR in one of your branch so that we (team) could also add commits in case you need help. I agree that the place you saw might be a good place to switch to this decoding if chunk is used.

not sure a middle ware would help because it does not have access to the request internals and especially would not be able to process the body and send data to the upload callback.

I agree it would be better to have that implemented internally and correctly supported.

This usage is also larger than just webdav... Any large request could benefit of that so that would be a nice addition.

[MitchBradley]

I have something basically working, but before I create a PR I would like advice on a few matters of taste and style:

1. Would it be okay to use a class enum for the chunked parser state, e.g.

enum class Chunk : uint8_t {
   None = 0,
   Length = 1,
  ...
};

or would you prefer a plain enum like

enum {
  CHUNK_NONE = 0,
  CHUNK_LENGTH = 1,
  ...
};

2. For each block of data decoded from the chunk stream (which does not necessarily align with chunk boundaries due to buffering concerns), I call handleBody(this, buf, len, index_into_overall_stream, 0) so the client can append to the file or whatever. The question is what to do at the end. One possibility is to call handleBody with 0 len, so it knows that the stream is done and can clean up. Another is to let handleRequest() take care of the cleanup, since it is going to be called anyway at the end.

[mathieucarbou]

1. All enums in cpp files are declared like this:

enum {
  PARSE_REQ_START = 0,
  PARSE_REQ_HEADERS = 1,
  PARSE_REQ_BODY = 2,
  PARSE_REQ_END = 3,
  PARSE_REQ_FAIL = 4
};

and all enums in headers are declared like this:

typedef enum {
  WS_DISCONNECTED,
  WS_CONNECTED,
  WS_DISCONNECTING
} AwsClientStatus;

So you could keep this pattern 😃

2. I think handleUpload() would be more appropriate because this is the handler to use that is accumulating data during an upload (see the examples). The on body is mostly used to handle form post or related post content types but not uploads. handleUpload() also has a final boolean flag to tell the callback when this is the end. If you follow the existing code which is handling an upload, there are consecutive calls to handleUpload() then one final, and then the final handleRequest() call is made, from where the response has to be created.

[MitchBradley]

I will try with handleUpload() to see if there are any gotchas. One concern is that it seems like a bit of an API break, since the existing code handles non-chunked PUT bodies via handleBody(). Some WebDAV clients send Content-Length framed PUT bodies, while MacOS WebDAVFS chunks the body. There is no inherent reason that the server could not use both Body and Upload, but it does feel a bit strange.

[mathieucarbou]

Ah! I didn't notice this part, sorry!

My thinking was that WebDAV post seems to be more familiar with how an upload handler works of you look at the examples, where all bytes are received after headers are processed and final bool flag tells de callback when it ends.

[MitchBradley]

Okay, so there is one head-scratcher issue with using handleUpload() - the filename parameter. For POSTs, that filename comes from the multipart form data "name" field. For PUT, the filename is inferred by decoding the URL according to the way that the filesystem is mounted. I suppose that the filename could be the verbatim URL in the PUT case.

[mathieucarbou]

Yes I would say so also...

[mathieucarbou]

@MitchBradley thank you for doing this effort. I see there are several design questions, so for now i would propose to work on te PR like you do in order to come up with a draft.

It will be easier to iterate / refine / discuss with a first draft.

Please do not forget to add a little example (you can copy/paste/adapt an existing one).

Starting a list of things to discuss:

• onBody or onUpload ?
• filename wrt PUT/POST

🙏👍