Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accept multipart-formdata without CRLF in the last line #1619

Closed
mccare opened this issue Jul 13, 2023 · 5 comments
Closed

Accept multipart-formdata without CRLF in the last line #1619

mccare opened this issue Jul 13, 2023 · 5 comments

Comments

@mccare
Copy link

mccare commented Jul 13, 2023

Hi,

I'm sending multipart-formdata to a server that is using cpp-httplib internally. The problem I'm running into is, that cpp-httplib requires the last line of the request to end in a CRLF. But the client library I'm using (undici from node) does not send a CRLF after the last boundary of a multipart formdata POST request. cpp-httplib will respond with a 400 error. If I use curl or axios, it will work, since they both append a CRLF after the last boundary

Sample last boundary (accepted by cpp-httplib)

--------------------------da314b32b9ea03ee--\r\n

Sample last boundary (producing 400 by cpp-httplib)

--------------------------da314b32b9ea03ee--

I haven't found a 100% answer if a CRLF is required after the last line on a multipart-formdata request, my guess is it is not required (the spec says something about CRLF between body parts but nothing about the last line). Message body from HTTP is also not required to end in a CRLF.

I've tried adding a case for the boundary parsing where it checks for dash_ but it didn't really work out. Since my C++ knowledge is rather limited, happy to have some pointers here. Is cpp-httpd relying on CRLF ending in some other place, too?

Here what I tried:

      case 4: { // Boundary
        if (crlf_.size() > buf_size()) { return true; }
        if (buf_start_with(crlf_)) {
          buf_erase(crlf_.size());
          state_ = 1;
        } else {
          if (dash_crlf_.size() > buf_size()) { return true; }
          if (buf_start_with(dash_crlf_)) {
            buf_erase(dash_crlf_.size());
            is_valid_ = true;
            buf_erase(buf_size()); // Remove epilogue
          } else if (buf_start_with(dash_)) {
            // case where the last line will end in -- without a CRLF
            buf_erase(dash_.size());
            is_valid_ = true;
            buf_erase(buf_size()); // Remove epilogue
          } else {
            return true;
          }
        }
        break;
@yhirose
Copy link
Owner

yhirose commented Jul 22, 2023

@mccare thank you for the feedback. Could you check with the valid RFC to see if a CRLF is required after the last line? If the spec doesn't require it, I'll take a look at it. Thanks for your help!

@mccare
Copy link
Author

mccare commented Jul 24, 2023

I'm looking at https://datatracker.ietf.org/doc/html/rfc2046#section-5.1.1 which is referenced from https://datatracker.ietf.org/doc/html/rfc7578#section-4.1

Below in section 5.1.1 it states the following format for the multipart-body. So the multipart body ends with a close-delimiter, which is a delimiter plus "--" and a delimiter is a CRLF with a dash-boundary. In the RFC it states that the CRLF before the boundary belongs conceptually to the boundary:

NOTE:  The CRLF preceding the boundary delimiter line is conceptually
   attached to the boundary so that it is possible to have a part that
   does not end with a CRLF (line  break).

Here the BNF of a multipart body:

multipart-body := [preamble CRLF]
                       dash-boundary transport-padding CRLF
                       body-part *encapsulation
                       close-delimiter transport-padding
                       [CRLF epilogue]

encapsulation := delimiter transport-padding
                      CRLF body-part

delimiter := CRLF dash-boundary

close-delimiter := delimiter "--"

From this I would argue that the last boundary does not have to end in a CRLF.

@yhirose
Copy link
Owner

yhirose commented Jul 29, 2023

@mccare the latest master should fix this problem. Could you try it on your environment? Thanks!

@mccare
Copy link
Author

mccare commented Jul 30, 2023

Thanks a lot, I will try it and report back.

@mccare
Copy link
Author

mccare commented Aug 4, 2023

Just tested it and it works now with undici as client sending multipart requests to cpp-httplib server. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants