search

eidheim / Simple-Web-Server

A very simple, fast, multithreaded, platform independent HTTP and HTTPS server and client library implemented using C++11 and Boost.Asio. Created to be an easy way to make REST resources available from C++ applications.
MIT License
2.61k stars 751 forks source link

Client<HTTPS> providing truncated body, may be fault of server #203

Closed moodboom closed 6 years ago

moodboom commented 6 years ago

I'm getting a truncated body using the client, but I am suspicious that it is malformed data from the server.

google.com works

Client<HTTPS> client("www.google.com");
auto response = client.request("GET","/");
string body = response->content.string(); // "<!doctype html><html itemscope=\"\" ...

but for a marketwatch.com request, the body is prefixed with junk and gets truncated

Client<HTTPS> client("www.marketwatch.com");
auto response = client.request("GET","/investing/stock/goog");
string body = response->content.string(); // "00006000\r\n\n<!DOCTYPE html>\n .... (truncated)

The good response hits asio's read_until_delim_string_op full match (~ln 584, "// Full match. We're done.") four times, while the bad response only hits it once, but that's as far as I've gotten so far in tracking it down.

I suspect that browsers are dealing with malformed headers in a more-forgiving fashion perhaps? But haven't tracked down the details yet.

eidheim commented 6 years ago

Thank you for reporting and looking into this. There seems to be an extra space in marketwatch's Transfer-Encoding header field value. It worked when I made the following change just to test:

diff --git a/client_http.hpp b/client_http.hpp
index 6803afc..1fd2f1a 100644
--- a/client_http.hpp
+++ b/client_http.hpp
@@ -503,7 +503,7 @@ namespace SimpleWeb {
             else
               session->callback(session->connection, ec);
           }
-          else if((header_it = session->response->header.find("Transfer-Encoding")) != session->response->header.end() && header_it->second == "chunked") {
+          else if((header_it = session->response->header.find("Transfer-Encoding")) != session->response->header.end() && header_it->second == " chunked") {
             auto chunks_streambuf = std::make_shared<asio::streambuf>(this->config.max_response_streambuf_size);
             this->read_chunked_transfer_encoded(session, chunks_streambuf);
           }

We'll have to change HttpHeader::parse to take into accounts such errors I guess.

moodboom commented 6 years ago

Awesome nice find. Here's a fix in HttpHeader::parse that's working for me on a couple rogue sites I found. Sorry I didn't get to the bottom first!

diff --git a/utility.hpp b/utility.hpp
index 76fc7d5..766c400 100644
--- a/utility.hpp
+++ b/utility.hpp
@@ -142,13 +142,10 @@ namespace SimpleWeb {
       std::size_t param_end;
       while((param_end = line.find(':')) != std::string::npos) {
         std::size_t value_start = param_end + 1;
-        if(value_start < line.size()) {
-          if(line[value_start] == ' ')
-            value_start++;
-          if(value_start < line.size())
-            result.emplace(line.substr(0, param_end), line.substr(value_start, line.size() - value_start - 1));
-        }
-
+        while(value_start < line.size() && line[value_start] == ' ')
+          value_start++;
+        if(value_start < line.size())
+          result.emplace(line.substr(0, param_end), line.substr(value_start, line.size() - value_start - 1));
         getline(stream, line);
       }
       return result;
eidheim commented 6 years ago

Thank you, your diff looks pretty much the same as the commit that closed this issue:) I also added some additional tests for this issue.

moodboom commented 6 years ago

Sweet thank you good sir. :-)

Natulux commented 6 years ago

Hey guys I also happened to get several "stream truncated" errors which eventually break my code.

I start the server part of the HTTPS example and use the match example:

server.resource["^/match/([0-9]+)$"]["GET"] = [](shared_ptr response, shared_ptr request) { cout << "Got match: " << request->path_match[1] << endl; response->write(request->path_match[1]); };

Using Chrome version 63.0.3239.132 https://localhost:8082/match/2

I get a lot of truncated errors in the error handling, even though it eventually works: asio.ssl.stream:1 stream truncated asio.ssl.stream:1 stream truncated asio.ssl.stream:1 stream truncated Got match: 2

My appllication which I enriched by the webserver even happens to crash here. Any hint, how to handle this on the server side?

Natulux commented 6 years ago

Nevermind, actually. The stream truncated error didn't break the code, it was just my way of displaying the error causing text, which had an issue. And if I don't pass wrong values to the webserver, I get almost no errors.