All good @bew ?
My use case:

• https://github.com/spider-gazelle/action-controller/blob/master/src/action-controller/body_parser.cr#L61
• https://github.com/spider-gazelle/action-controller/blob/master/spec/body_parser_spec.cr

I don't have enough knowledge on how HTTP is implemented and how this might be used to validate the relevance & implementation of your PR, but language-wise it looks better yes 😃

Actually, an added spec or two would be nice!

The error message actually already states that it should work with bot form-data and file yet the implementation only accepts the former.

The change is correct, but it should be supplemented by a spec.

This is incorrect as per the RFC. Look at the example:

   Content-Type: multipart/form-data; boundary=AaB03x

   --AaB03x
   Content-Disposition: form-data; name="submit-name"

   Larry
   --AaB03x
   Content-Disposition: form-data; name="files"
   Content-Type: multipart/mixed; boundary=BbC04y

   --BbC04y
   Content-Disposition: file; filename="file1.txt"
   Content-Type: text/plain

   ... contents of file1.txt ...
   --BbC04y
   Content-Disposition: file; filename="file2.gif"
   Content-Type: image/gif
   Content-Transfer-Encoding: binary

   ...contents of file2.gif...
   --BbC04y--
   --AaB03x--

This is a multipart/mixed with each part containing a Content-Disposition: file header contained inside a multipart/form-data.

We should discuss how we want to handle this, but this is absolutely the incorrect fix and will not parse the above data.

To make it clear what I mean, let me indent:

Content-Type: multipart/form-data; boundary=AaB03x

--AaB03x
Content-Disposition: form-data; name="submit-name"

Larry
--AaB03x
Content-Disposition: form-data; name="files"
Content-Type: multipart/mixed; boundary=BbC04y

   --BbC04y
   Content-Disposition: file; filename="file1.txt"
   Content-Type: text/plain

   ... contents of file1.txt ...
   --BbC04y
   Content-Disposition: file; filename="file2.gif"
   Content-Type: image/gif
   Content-Transfer-Encoding: binary

   ...contents of file2.gif...
   --BbC04y--
--AaB03x--

@RX14 Good catch. From the initial description of the method and the error message it seemed obvious to be the intended behaviour. But when you look at the context of how it's used, it is clear that it won't work this way.

There needs to be extra effort to support multiple file payloads.

I think most actual browsers don't do this, they just send multiple fields with the same name, instead of nesting multipart/*

@RX14 to your point, I am extracting the indented data and then wanting to parse it. Please see: https://github.com/spider-gazelle/action-controller/blob/master/src/action-controller/body_parser.cr#L61

So this minor modification solves the issue. Otherwise I have to have an independent copy of this parsing code for something covered by the specification.

i.e. even if in the future if effort is taken to support multiple file payloads you can use this parsing code to extract those files.

Supporting the two types of Content-Dispositions makes sense at this level in the process.

This class is for parsing multipart/form-data though, you're trying to parse multipart/mixed so you should use HTTP::Multipart. So this error is correct: you're trying to parse the incorrect format.

We should have some way to parse a Content-Disposition: file header, I entirely agree. However I think the way we want to do that should be discusses in a new issue, and this PR closed.

The HTTP::FormData parser is actually a tiny wrapper around HTTP::Multipart so it should be easy to switch.

Thanks for the tip @RX14 - I'll re-factor using HTTP::Multipart

@stakach You'll still have to parse the Content-Disposition header yourself. This is absolutely non-ideal. Please would you submit a feature-request for a way to parse Content-Dispostion: file headers? Thanks.