drogonframework / drogon

Drogon: A C++14/17/20 based HTTP web application framework running on Linux/macOS/Unix/Windows
MIT License
11.44k stars 1.1k forks source link

`MultiPartParser::parse` returns -1 on forms #1653

Closed Mis1eader-dev closed 8 months ago

Mis1eader-dev commented 1 year ago

Describe the bug Using MultiPartParser::parse on a request form returns -1, and is unable to parse. This is the case with and without <form> tags.

To Reproduce The drogon side of the code is as follows:

void LoginCtrl::asyncHandleHttpRequest(const HttpRequestPtr& req, std::function<void (const HttpResponsePtr&)>&& callback)
{
    MultiPartParser parser;
    std::cout << parser.parse(req) << '\n';
    const auto& params = parser.getParameters();

    const auto unameFind = params.find("u");
    if(unameFind == params.end())
    {
        auto resp = HttpResponse::newHttpResponse();
        resp->setStatusCode(k401Unauthorized);
        callback(resp);
        return;
    }

    const auto passFind = params.find("p");
    if(passFind == params.end())
    {
        auto resp = HttpResponse::newHttpResponse();
        resp->setStatusCode(k401Unauthorized);
        callback(resp);
        return;
    }

    const string_view
        uname = unameFind->second,
        pass = passFind->second;

    std::cout << uname << '\n' << pass << '\n';
    callback(HttpResponse::newHttpResponse());
}

The form is submitted via JavaScript:

const
    uname = "admin",
    pass = "admin",
    form = new FormData();
form.append("u", uname);
form.append("p", pass);
fetch("/api/login", {
    method: "POST",
    body: form,
    headers: {
        "Content-Type": "multipart/form-data",
    },
});

Expected behavior It must parse the request form.

Desktop:

Additional context If the req->body() is printed, the form content can be seen. This is an issue with the MultiPartParser.

Mis1eader-dev commented 1 year ago

I had used a different approach right after making this issue, and thought this may help people who may stumble upon this.

For the time being until this is solved, users have to use normal HTTP requests and make a protocol for it, for my case I made a byte array with the following format:

[1 byte: uname length] [<uname length> bytes: uname] [1 byte: pass length] [<pass length> bytes: pass]

If you need more length, make length bytes more than 1.

On the server side, it is as easy as casting the lengths to appropriate size type pointers and dereferencing them:

const std::string_view body = req->body();

const uint8_t unameLen = body[0];
const std::string_view uname = body.substr(1, unameLen);
// etc.

Of course you must do boundary and length checks yourself when retrieving each piece of information from the body, requests from outside must never be trusted and should be validated so you don't encounter out of bounds errors or buffer overflows.

Mis1eader-dev commented 8 months ago

The MultiPartParser works if the header in the frontend is not set manually like in the example. Do we need to fix anything? Or have MultiPartParser parse requests with Content-Type: multipart/form-data as well?

If the the content type header is not set, the frontend sends: multipart/form-data; boundary=----WebKitFormBoundarybRHe2TiCOeJdlhGA

an-tao commented 8 months ago

I don't understand what's the problem. according to the protocol, the Content-type header of multipart/form-data should be set with the boundary string.

Mis1eader-dev commented 8 months ago

If we set the header in the frontend:

const form = new FormData();
form.append("key", "value");
fetch("/api/form", {
    method: "POST",
    body: form,
    headers: {
        "Content-Type": "multipart/form-data", // note this
    },
});

Then the http request's content type header looks like:

Content-Type: multipart/form-data

And the returned value of MultiPartParser::parse(req) will be -1.


However, if we don't set the content type header:

const form = new FormData();
form.append("key", "value");
fetch("/api/form", {
    method: "POST",
    body: form,
    /*headers: {
        "Content-Type": "multipart/form-data"
    },*/
});

Then the http request's content type header looks like:

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

And the returned value of MultiPartParser::parse(req) will be 0 and parses as expected.


I am not familiar with the HTTP form spec, is not having a boundary illegal? If so, I will close this issue

an-tao commented 8 months ago

Please print the body in the first case to see if there are boundaries in the body.

Mis1eader-dev commented 8 months ago

I don't think this is a bug, if the frontend client sets its Content-Type header to something without a boundary, it may be violating the specification.