arangodb · jsteemann · Jun 11, 2021 · Jun 11, 2021
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,6 +1,10 @@
 devel
 -----
 
+* Fix URL request parsing in case data is handed in in small chunks.
+  Previously the URL could be cut off if the chunk size was smaller than
+  the URL size.
+
 * Backport bugfix from upstream rocksdb repository for calculating the
   free disk space for the database directory. Before the bugfix, rocksdb
   could overestimate the amount of free space when the arangod process

diff --git a/arangod/GeneralServer/HttpCommTask.cpp b/arangod/GeneralServer/HttpCommTask.cpp
@@ -77,6 +77,7 @@ int HttpCommTask<T>::on_message_began(llhttp_t* p) {
   me->_lastHeaderField.clear();
   me->_lastHeaderValue.clear();
   me->_origin.clear();
+  me->_url.clear();
   me->_request = std::make_unique<HttpRequest>(me->_connectionInfo, /*messageId*/ 1,
                                                me->_allowMethodOverride);
   me->_response.reset();
@@ -93,16 +94,15 @@ int HttpCommTask<T>::on_message_began(llhttp_t* p) {
 template <SocketType T>
 int HttpCommTask<T>::on_url(llhttp_t* p, const char* at, size_t len) {
   HttpCommTask<T>* me = static_cast<HttpCommTask<T>*>(p->data);
-  me->_request->parseUrl(at, len);
   me->_request->setRequestType(llhttpToRequestType(p));
   if (me->_request->requestType() == RequestType::ILLEGAL) {
     me->sendSimpleResponse(rest::ResponseCode::METHOD_NOT_ALLOWED,
                            rest::ContentType::UNSET, 1, VPackBuffer<uint8_t>());
     return HPE_USER;
   }
-
   me->statistics(1UL).SET_REQUEST_TYPE(me->_request->requestType());
 
+  me->_url.append(at, len);
   return HPE_OK;
 }
 
@@ -193,6 +193,7 @@ int HttpCommTask<T>::on_body(llhttp_t* p, const char* at, size_t len) {
 template <SocketType T>
 int HttpCommTask<T>::on_message_complete(llhttp_t* p) {
   HttpCommTask<T>* me = static_cast<HttpCommTask<T>*>(p->data);
+  me->_request->parseUrl(me->_url.data(), me->_url.size());
 
   me->statistics(1UL).SET_READ_END();
   me->_messageDone = true;
@@ -245,15 +246,28 @@ bool HttpCommTask<T>::readCallback(asio_ns::error_code ec) {
     size_t nparsed = 0;
     for (auto const& buffer : this->_protocol->buffer.data()) {
       const char* data = reinterpret_cast<const char*>(buffer.data());
-
-      err = llhttp_execute(&_parser, data, buffer.size());
-      if (err != HPE_OK) {
-        ptrdiff_t diff = llhttp_get_error_pos(&_parser) - data;
-        TRI_ASSERT(diff >= 0);
-        nparsed += static_cast<size_t>(diff);
-        break;
-      }
-      nparsed += buffer.size();
+      const char* end = data + buffer.size();
+      do {
+        size_t datasize = end - data;
+
+        TRI_IF_FAILURE("HttpCommTask<T>::readCallback_in_small_chunks") {
+          // we had an issue that URLs were cut off because the url data was handed
+          // in in multiple buffers.
+          // To cover this case, we simulate that data fed to the parser in small chunks.
+          constexpr size_t chunksize = 5;
+          datasize = std::min<size_t>(datasize, chunksize);
+        }
+
+        err = llhttp_execute(&_parser, data, datasize);
+        if (err != HPE_OK) {
+          ptrdiff_t diff = llhttp_get_error_pos(&_parser) - data;
+          TRI_ASSERT(diff >= 0);
+          nparsed += static_cast<size_t>(diff);
+          break;
+        }
+        nparsed += datasize;
+        data += datasize;
+      } while (ADB_UNLIKELY(data < end));
     }
 
     TRI_ASSERT(nparsed < std::numeric_limits<size_t>::max());

diff --git a/arangod/GeneralServer/HttpCommTask.h b/arangod/GeneralServer/HttpCommTask.h
@@ -86,6 +86,7 @@ class HttpCommTask final : public GeneralCommTask<T> {
   std::string _lastHeaderField;
   std::string _lastHeaderValue;
   std::string _origin;  // value of the HTTP origin header the client sent
+  std::string _url;
   std::unique_ptr<HttpRequest> _request;
   std::unique_ptr<basics::StringBuffer> _response;
   bool _lastHeaderWasValue;

diff --git a/tests/js/client/shell/shell-request.js b/tests/js/client/shell/shell-request.js
@@ -34,14 +34,16 @@ var request = require('@arangodb/request');
 var url = require('url');
 var querystring = require('querystring');
 var qs = require('qs');
+const deriveTestSuite = require('@arangodb/test-helper').deriveTestSuite;
+const internal = require("internal");
 
+'use strict';
 
 //////////////////////////////////////////////////////////////////
8000
//////////////
 /// @brief test suite
 ////////////////////////////////////////////////////////////////////////////////
 
-function RequestSuite () {
-  'use strict';
+function BaseRequestSuite () {
   var buildUrl = function (append, base) {
     base = base === false ? '' : '/_admin/echo';
     append = append || '';
@@ -395,12 +397,33 @@ function RequestSuite () {
   };
 }
 
+function RequestSuite () {
+  let suite = {};
+  deriveTestSuite(BaseRequestSuite(), suite, '');
+  return suite;
+}
+
+function RequestSuiteWithSmallChunks () {
+  let suite = {
+    setUpAll: function() {
+      internal.debugSetFailAt("HttpCommTask<T>::readCallback_in_small_chunks");
+    },
+
+    tearDownAll: function() {
+      internal.debugClearFailAt();
+    },
+  };
+
+  deriveTestSuite(BaseRequestSuite(), suite, '_SmallChunks');
+  return suite;
+}
 
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief executes the test suite
 ////////////////////////////////////////////////////////////////////////////////
 
 jsunity.run(RequestSuite);
+jsunity.run(RequestSuiteWithSmallChunks);
 
 return jsunity.done();