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

ProtocolException unsupported HTTP version #101

Open
KingDuckZ opened this issue Aug 16, 2020 · 13 comments
Open

ProtocolException unsupported HTTP version #101

KingDuckZ opened this issue Aug 16, 2020 · 13 comments
Assignees
Labels

Comments

@KingDuckZ
Copy link

Hello, I'm trying to connect to a remote server and my program fails with this exception:

terminate called after throwing an instance of 'restc_cpp::ProtocolException'
  what():  ReadHeaders(): unsupported HTTP version: 0%0D%0A%0D%0AHTTP/1.1
fish: './orotool' terminated by signal SIGABRT (Abort)

Is the upstream server misconfigured, or should this library be able to clean up the extra characters before HTTP?

@KingDuckZ
Copy link
Author

KingDuckZ commented Aug 17, 2020

I've been looking into this one a bit, unfortunately I don't think I know how to fix it. With the debugger I got to this point:

restc-cpp/restc-cpp/src/IoReaderImpl.cpp:31

and the state of some variables:

(gdb) p bytes
$23 = 5
(gdb) p buffer_
$24 = {_M_elems = "0\r\n\r\n", '\000' <repeats 16378 times>}
(gdb) p bytes
$25 = 5
(gdb) p *this
$26 = {<restc_cpp::DataReader> = {_vptr.DataReader = 0x7ffff7fb4e98 <vtable for restc_cpp::IoReaderImpl+16>}, ctx_ = @0x7ffff0403cd0, 
  connection_ = std::weak_ptr<restc_cpp::Connection> (use count 3, weak count 5) = {get() = 0x7ffff0046df0}, cfg_ = {msReadTimeout = 21000}, buffer_ = {
    _M_elems = "0\r\n\r\n", '\000' <repeats 16378 times>}}

I got into this function twice before this one and data in buffer_ looked fine (a valid string starting with HTTP). What could be going wrong here? Is it a lib or a server problem?

This is a blocker for me as it's preventing my program from running correctly with the real server, please help!

Edit I've been debugging a bit more, it's boost itself that fills the buffer like that, so it doesn't look like some memory corruption or anything of the sort to me, but I'm still clueless on what the cause might be

Edit 2 on a second thought it could be some kind of body length? meaning empty? I can't see that happening in curl, but it could be something that curl just skips over or it's only caused by the way this lib/boost communicate with the server? I think it's weird to have that even before the HTTP string, but it's consistent and curl just works

Edit 3 ok I'll stop looking now, but I'm starting to think it might be something like the server sending chunked replies without explicitly saying so? could that be? so the wrong reader gets instantiated and it chokes on the chunk sizes? this part leaves me a bit confused: "For version 1.1 of the HTTP protocol, the chunked transfer mechanism is considered to be always and anyway acceptable, even if not listed in the TE (transfer encoding) request header field" - anyways, I'm just guessing here!

@KingDuckZ
Copy link
Author

KingDuckZ commented Aug 21, 2020

Any chance to get some comment on this please? Like I said, this is a blocker for me, it stalled my whole project

@buntin
Copy link

buntin commented Oct 29, 2020

Did you find a solution for this issue? I've run up against it in my project.

@jgaa
Copy link
Owner

jgaa commented Oct 30, 2020

I'm working on it.
I'll probably have a fix within 24 hours

(I can only reproduce it under Windows).

@jgaa jgaa self-assigned this Oct 30, 2020
@jgaa jgaa added the bug label Oct 30, 2020
@buntin
Copy link

buntin commented Oct 30, 2020

only reproduce it under Windows

yes, I'm having the issue with Windows and VS2019.

@RavilN
Copy link

RavilN commented Oct 30, 2020

Hi,
I wonder, is this the same issue which I had, and was fixed locally this way (file src/DataReaderStream.cpp, line 95):

if (ciEqLibC()(value, http_1_1)) {
    ; // Do nothing HTTP 1.1 is the default value
} else if (value.find(http_1_1) == std::string::npos) {
    throw ProtocolException(
        string("ReadHeaders(): unsupported HTTP version: ")
            + url_encode(value));
}

jgaa added a commit that referenced this issue Oct 30, 2020
Merging recent changes to master. 
All tests are passing under Debian and Ubuntu.

Issue #101 is reproducible under Windows. 

Working on issue #101, #102, #104, #105
@jgaa jgaa closed this as completed in 7bb995e Oct 31, 2020
@KingDuckZ
Copy link
Author

It's been a long time and since that problem was a hard blocker for me I simply ditched restc-cpp and implemented my own solution using libcurl and simdjson. My program's been running for a few months now collecting 1.3 GiB of compressed JSON. It only quits every few days/weeks due to unrelated bugs.

For those interested my code is here. Specifically you want to look at quick_rest.hpp, the interface is very simple and it enables me to do everything I need. Result is a string with the received http code attached and some other fun stuff. Json parsing is done outside, by the function that invokes fetch(). Code is under GPL3+.

To be exact I put restc-cpp facing code inside ifdefs so I could go back to it. I can still give the latest version of restc-cpp a try, but iirc there was also the problem of getting the appropriate http code in case of bad response by the server, which prevents me from reacting properly to errors.

@jepessen
Copy link

I'm obtaining the same error. This happens when I this piece of code:

try {
    string address{ GetProductCandlesRequest };
    replace_all(address, ProductIDPlaceholder, id);
    auto startTimes = TimeConversion::GetISO8601TimeStringFrom(startTime);
    auto endTimes = TimeConversion::GetISO8601TimeStringFrom(endTime);
    m_logger->debug("Retrieving coinbase candles for symbol " + string(id) + " from " + startTimes + " to " + endTimes);
    auto contextFunction = [&](Context& ctx) {
      auto reply = RequestBuilder(ctx)
        .Get(address)
        .Argument(GranularityParameter, candleWidth.count() * 60)
        .Argument(StartParameter, startTimes)
        .Argument(EndParameter, endTimes)
        .Header(AcceptHeader, AcceptHeaderValue)
        .Header(UserAgentHeader, UserAgentHeaderValue)
        .Execute();

      auto json = reply->GetBodyAsString();
      m_logger->info("JSON IS: " + json);
    };
    m_client->ProcessWithPromise(contextFunction).get();
  }

I can obtain two type of body strings. One is an empty array

[]

The other response is something like this

[[1618160400,0.0000958,0.0000983,0.000096,0.0000976,14170.94],[1618156800,0.0000956,0.000098,0.0000956,0.0000959,12175.8],[1618153200,0.0000948,0.0000964,0.0000953,0.0000956,2815.79],[1618149600,0.0000948,0.0000957,0.0000948,0.0000951,1451.8],[1618146000,0.0000943,0.0000953,0.0000951,0.0000947,2114.07],[1618142400,0.0000934,0.0000954,0.0000942,0.0000954,3783.13]]

When I receive this, I obtain the exception, but if I comment the auto json = reply->GetBodyAsString(); the error disappear.

@jgaa
Copy link
Owner

jgaa commented Aug 28, 2022

It says unsupported http version. Can you enable logging or use the debugger to see what http version the server is using in the http reply ?

@jepessen
Copy link

As I've said, it's the same error. The problem occurs because in the new piece of code

if (ciEqLibC()(value, http_1_1)) {
    ; // Do nothing HTTP 1.1 is the default value
} else if (value.find(http_1_1) == std::string::npos) {
    throw ProtocolException(
        string("ReadHeaders(): unsupported HTTP version: ")
            + url_encode(value));
}

value is 0\r\n\r\nHTTP/1.1 instead of HTTP/1.1. It does not happen if I comment the GetBodyAsString method during parsing, so I suppose that's something in there.
At the moment I cannot enable logging because I'm using the vcpkg version that's not updated to latest, I can use only boost logging and if I try to add it I obtain linking errors. I'll try to use the repository version.

@jepessen
Copy link

I've replaced GetBodyAsString with restc_cpp::SerializeFromJson in deserialization of the message, and the problem disappeared. I've some problem on enabling logs due to boost that mixes static and dynamic linking in my project. I'll try to create a clean log if possible.

@jgaa
Copy link
Owner

jgaa commented Aug 30, 2022

Weird. The string contains a zero, which indicate a bug at a lower level in the code parsing the HTTP response. I think I better try to reproduce the problem here.
What is the URL you are connecting to, and what operating system are you using?

@jgaa jgaa reopened this Aug 30, 2022
@jepessen
Copy link

jepessen commented Sep 7, 2022

This is an extract of the piece of code where I had problems:

constexpr auto GetAllKnownTradingPairsRequest{ "https://api.exchange.coinbase.com/products/" };
constexpr auto GetProductCandlesRequest{ "https://api.exchange.coinbase.com/products/{product_id}/candles" };
constexpr auto ProductIDPlaceholder{ "{product_id}" };
constexpr auto AcceptHeader{ "Accept" };
constexpr auto AcceptHeaderValue{ "application/json" };
constexpr auto UserAgentHeader{ "User-Agent" };
constexpr auto UserAgentHeaderValue{ "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.0.0 Safari/537.36" };
constexpr auto GranularityParameter{ "granularity" };
constexpr auto StartParameter{ "start" };
constexpr auto EndParameter{ "end" };

using TimePoint = std::chrono::time_point<std::chrono::system_clock>;
using Second = std::chrono::seconds;

string TimeConversion::GetISO8601TimeStringFrom(const TimePoint& ms) {
  TimePoint dt(ms);  
  auto x = date::format("%FT%T%z", dt);
  return x;
}

REST::REST(const Connection& connection, Logger::Interface::Ptr logger) :
  m_connection(connection),
  m_logger(logger) {
  auto tls_ctx = std::make_shared<boost::asio::ssl::context>(boost::asio::ssl::context::tlsv12_client);
  tls_ctx->set_options(
    boost::asio::ssl::context::default_workarounds
  | boost::asio::ssl::context::no_compression
  );
  m_client = RestClient::Create(tls_ctx);

}

vector<CandleData> REST::getProductCandles(string_view id, const TimePoint& startTime, const TimePoint& endTime, const Second& candleWidth) {
  vector<CandleData> candles;
  try {
    string address{ GetProductCandlesRequest };
    replace_all(address, ProductIDPlaceholder, id);
    // address is something like "https://api.exchange.coinbase.com/products/BTC-EUR/candles"
    auto startTimes = TimeConversion::GetISO8601TimeStringFrom(startTime);
    auto endTimes = TimeConversion::GetISO8601TimeStringFrom(endTime);
    m_logger->debug("Retrieving coinbase candles for symbol " + string(id) + " from " + startTimes + " to " + endTimes);
    auto contextFunction = [&](Context& ctx) {
      auto reply = RequestBuilder(ctx)
        .Get(address)
        .Argument(GranularityParameter, candleWidth.count() * 60)
        .Argument(StartParameter, startTimes)
        .Argument(EndParameter, endTimes)
        .Header(AcceptHeader, AcceptHeaderValue)
        .Header(UserAgentHeader, UserAgentHeaderValue)
        .Execute();
      
      // this works, the problem with leading zeros was with
      //  auto json = reply->GetBodyAsString();
      // m_logger->info("JSON IS: " + json);
      std::vector<std::vector<double>> jdata;
      restc_cpp::SerializeFromJson(jdata, *reply);
      for (const auto& it : jdata) {
        CandleData candle;
        candle.timestamp = static_cast<decltype(candle.timestamp)>(it.at(0));
        candle.low = it.at(1);
        candle.high = it.at(2);
        candle.open = it.at(3);
        candle.close = it.at(4);
        candle. Volume = it.at(5);
        candles.emplace_back(std::move(candle));
      }
    };
    m_client->ProcessWithPromise(contextFunction).get();
  }
  catch (std::exception& e) {
    m_logger->error(string("Error when retrieving candles from coinbase: ") + e.what());
    candles.clear();
  }
  std::sort(candles.begin(), candles.end(), [](const auto& lhs, const auto& rhs) -> bool { return lhs.timestamp < rhs.timestamp; });
  return candles;
}

I'm using Visual Studio 2022 (17.3.3) with Windows 11 64 bit

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

No branches or pull requests

5 participants