ossia / libossia

A modern C++, cross-environment distributed object model for creative coding and interaction scoring
https://ossia.io
GNU Lesser General Public License v3.0
206 stars 33 forks source link

asio async_read issues with http replies #533

Closed pchdev closed 4 years ago

pchdev commented 4 years ago

hello guys!

in network/oscquery/detail/http_client.hpp, there seems to be an issue with the asio::async_read_until calls (starting on line 111), reading in some cases the entire http reply messages in one go, including its header and contents, thus discarding the initial until "\r\n" condition and consequently the next function calls (it never reaches the necessary handle_read_headers() & handle_read_content() callbacks, and consequently, never reaches parsing as well).

I read somewhere that in some cases, the async_read_until call have the habit to go a little bit further than expected, which maybe would explain why the next callbacks are not processed.

I've managed to make a quick fix with the following piece of code (on line 146), but there might be some bad consequences to this as well.. So, if anyone has experience with asio (which I certainly don't), and have any ideas to fix the problem at its root, it would be much better !

      while (!response_stream.eof())
      {
          std::getline(response_stream, status_message);
          // if 'blank' line, we expect the next one to be
          // the reply's contents.
          if (status_message.size() == 1)
          {
              // if we reach eof after blank line, just break.
              if (response_stream.eof())
                  break;
              // if contents, get the rest of the stream.
              // send and parse contents through m_fun;
              status_message = std::string(std::istreambuf_iterator<char>(response_stream), {});
              std::string contents;
              contents.reserve(status_message.size()+16);
              contents.assign(status_message);
              m_fun(*this, contents);
              return;
          }
      }
jcelerier commented 4 years ago

oho, that's a very interesting one. @thibaudk mentioned issues with the critical attribute, is that the cause ? Thanks for the fix in any case, testing asap

pchdev commented 4 years ago

critical attribute is a different issue, and a really simpler one, I will push a fix quickly. Thank you!

pchdev commented 4 years ago

@jcelerier I can't really see another cleaner type of fix for this one at the moment, since asio doesn't really seem to have other ways to deal with this except to leave it to the user to parse it the old fashioned way. I'll make a PR, if you want to test it a bit first or if you have other ideas meanwhile.

pchdev commented 4 years ago

closing issue, bottom line is: there's nothing really wrong with the asio code, but it imperatively needs the http reply to be immediately followed by a closed connection in order to reach the proper callbacks and end of file.