Stiffstream / restinio

Cross-platform, efficient, customizable, and robust asynchronous HTTP(S)/WebSocket server C++ library with the right balance between performance and ease of use
Other
1.16k stars 93 forks source link

Http server cuts response body #220

Closed ShipilovDmitry closed 4 months ago

ShipilovDmitry commented 4 months ago

If there is a standard unhandled exception in the user code, it is intercepted by the http server restinio. If the response is large enough (more than 65,000 bytes), the http server will return code 200 and an incomplete (truncated response) to the client.

I would like either not to return the 200 code to the user, or to return the full body.

Currently, the http server does not send all packets, but only a part of them.

Sample code for version 0.6.18

#include <restinio/all.hpp>
#include <string>

void start() {
  restinio::run(
      restinio::on_thread_pool(1).port(8000).address("0.0.0.0").request_handler(
          [](restinio::request_handle_t const &req) {
            constexpr size_t outputBytes = 100'000;
            std::string bigOutput;
            bigOutput.reserve(outputBytes);

            for (size_t i = 0; i < outputBytes; ++i) {
              bigOutput.push_back('a');
            }

            bigOutput.push_back('}'); // it is absent, when throw
            std::string_view constexpr kAppJson = "application/json";

            req->create_response()
                .append_header(restinio::http_field::version,
                               std::to_string(req->header().http_major()))
                .append_header(restinio::http_field::content_type,
                               kAppJson.data())
                .append_header(restinio::http_field::status_uri,
                               std::to_string(200))
                .set_body(bigOutput)
                .done();

            throw std::logic_error{"error"};

            return restinio::request_handling_status_t::accepted;
          }));
}

int main() {
  start();
  return 0;
}
eao197 commented 4 months ago

Hi! Thanks for reporting.

I'll take a look at it after the weekend.

eao197 commented 4 months ago

Hi, @ShipilovDmitry

Let's take a look from this point of view:

There is a "contract" between RESTinio and a programmer. When RESTinio accepts and reads a request it calls programmer's request handler. And the programmer takes responsibility of a new request by returning one of the following values:

If a request handler throws then RESTinio doesn't know what the status of the request and what to do with it. So RESTinio assumes that request handling wasn't completed, the request is in some undefined state and doesn't know what to do with it. Because of that RESTinio just shuts the connection down and closes it.

Because of the asynchronous nature of RESTinio it's possible that actual sending of data to the client may be started before the completion of the request handler. That is why you see that some part of the response is delivered to the client. But it's not guaranteed, there maybe cases when create_response()...done(); won't send a byte to the client, the whole response may be discarded.

It seems to me that this approach is a quite logical and understandable.

If we try to change the behavior of RESTinio to complete a response started just because an exception from request handler then we can find ourselves in a case where some garbage is being delivered to the client. It's just because we can trust data prepared in a terminated request handler.

So, if a programmer brokes the "contract" described above and throws an exception instead of returning one of expected values, then there is no guarantees from the RESTinio.

ShipilovDmitry commented 4 months ago

Hi, @eao197! Thanks for quick response!

I understand and support your assumption about "contract". But it's not clear for me why exception in user's code means "success" for request (200 code).

eao197 commented 4 months ago

@ShipilovDmitry let's take a look at your code example:

            req->create_response()                                                      // (0)
                .append_header(restinio::http_field::version,
                               std::to_string(req->header().http_major()))
                .append_header(restinio::http_field::content_type,
                               kAppJson.data())
                .append_header(restinio::http_field::status_uri,
                               std::to_string(200))                                     // (1)
                .set_body(bigOutput)
                .done();                                                                // (2)

            throw std::logic_error{"error"};                                            // (3)

At the point (0) you start to create a response for incoming request. At the point (1) you set "success" status code. At the point (2) you tell RESTinio that response is completed. At this point RESTinio may initiate sending of the response back to the client. The response will contain "success" status code set at the point (1).

At the point (3) an exception is thrown. If RESTinio has already started sending of the response then some part of this response (including status line) may already be sent into the connection. And this part will be delivered to the client even if RESTinio closes the connection because an exception at the point (3).

ShipilovDmitry commented 4 months ago

Thank you!