[BUG] http_request::get_arg[_flat] result truncated at around 15553 characters

jcphill commented 3 months ago

Prerequisites

[X] Put an X between the brackets on this line if you have checked that your issue isn't already filed: https://github.com/search?l=&q=repo%3Aetr%2Flibhttpserver&type=Issues

Description

Upgrading from 0.18.2 to 0.19.0 I find that large POST payloads that worked fine before are now truncated at around 15553 characters (that is the offset the JSON parser reports).

Steps to Reproduce

POST argument of 18K or larger.
Access via get_arg or get_arg_flat and convert to std::string.
Observe that it is truncated.

Expected behavior: [What you expect to happen]

Data is not truncated.

Actual behavior: [What actually happens]

Data is truncated at around 15553 characters.

Reproduces how often: [What percentage of the time does it reproduce?]

100%

Versions

Linux XXXX 4.18.0-477.51.1.el8_8.x86_64 1 SMP Fri Mar 1 11:21:44 EST 2024 x86_64 x86_64 x86_64 GNU/Linux
libhttpserver version 0.19.0, compiled
libmicrohttpd version 0.9.71, compiled

Additional Information

Sorry I don't have a simple reproducer. I'll see if I can make a test that fails.

jcphill commented 3 months ago

No luck on a simple reproducer. I am launching the server with start_method(http::http_utils::THREAD_PER_CONNECTION) if that matters.

jcphill commented 3 months ago

I can confirm that large multipart/form-data values are in fact being split into multiple http_arg_value values.

jcphill commented 3 months ago

The problem was likely introduced in #302 where the following was added to webserver::post_iterator in webserver.cpp:

    if (!filename) {
        // There is no actual file, just set the arg key/value and return.
        mr->dhr->set_arg(key, std::string(data, size));
        return MHD_YES;
    }

This adds a new value for each chunk with the same key rather than appending to the same value.

One possible fix would be to not ignore the passed-in offset value but rather in the case of a non-zero offset append the new data to the last value for the key.

In the meantime I can work around this issue on the client side by wrapping the string data in a Blob, which sets the filename field.

etr commented 3 months ago

You are right. In reality, what that method should do is checking whether there is an offset, and if there is, it should append the value to the last matching recorded argument in the request.

Definitely a missing test that should have been there, sorry for that - I'll try to fix it asap (unless, of course, you want to take a stub yourself).

jcphill commented 3 months ago

Thanks for the reply. I'm going to stick to my workaround and let you figure out a proper fix.

etr commented 2 months ago

Fixed: https://github.com/etr/libhttpserver/pull/337

etr / libhttpserver