8000 Fix URL request parsing. (#14357) · arangodb/arangodb@1d82f5c · GitHub
[go: up one dir, main page]
Skip to content

Commit 1d82f5c

Browse files
mpoetermpoeter
authored
Fix URL request parsing. (#14357)
1 parent d5be537 commit 1d82f5c

File tree

4 files changed

+55
-13
lines changed

4 files changed

+55
-13
lines changed

‎CHANGELOG

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
devel
22
-----
33

4+
* Fix URL request parsing in case data is handed in in small chunks.
5+
Previously the URL could be cut off if the chunk size was smaller than
6+
the URL size.
7+
48
* Backport bugfix from upstream rocksdb repository for calculating the
59
free disk space for the database directory. Before the bugfix, rocksdb
610
could overestimate the amount of free space when the arangod process

‎arangod/GeneralServer/HttpCommTask.cpp

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ int HttpCommTask<T>::on_message_began(llhttp_t* p) {
7777
me->_lastHeaderField.clear();
7878
me->_lastHeaderValue.clear();
7979
me->_origin.clear();
80+
me->_url.clear();
8081
me->_request = std::make_unique<HttpRequest>(me->_connectionInfo, /*messageId*/ 1,
8182
me->_allowMethodOverride);
8283
me->_response.reset();
@@ -93,16 +94,15 @@ int HttpCommTask<T>::on_message_began(llhttp_t* p) {
9394
template <SocketType T>
9495
int HttpCommTask<T>::on_url(llhttp_t* p, const char* at, size_t len) {
9596
HttpCommTask<T>* me = static_cast<HttpCommTask<T>*>(p->data);
96-
me->_request->parseUrl(at, len);
9797
me->_request->setRequestType(llhttpToRequestType(p));
9898
if (me->_request->requestType() == RequestType::ILLEGAL) {
9999
me->sendSimpleResponse(rest::ResponseCode::METHOD_NOT_ALLOWED,
100100
rest::ContentType::UNSET, 1, VPackBuffer<uint8_t>());
101101
return HPE_USER;
102102
}
103-
104103
me->statistics(1UL).SET_REQUEST_TYPE(me->_request->requestType());
105104

105+
me->_url.append(at, len);
106106
return HPE_OK;
107107
}
108108

@@ -193,6 +193,7 @@ int HttpCommTask<T>::on_body(llhttp_t* p, const char* at, size_t len) {
193193
template <SocketType T>
194194
int HttpCommTask<T>::on_message_complete(llhttp_t* p) {
195195
HttpCommTask<T>* me = static_cast<HttpCommTask<T>*>(p->data);
196+
me->_request->parseUrl(me->_url.data(), me->_url.size());
196197

197198
me->statistics(1UL).SET_READ_END();
198199
me->_messageDone = true;
@@ -245,15 +246,28 @@ bool HttpCommTask<T>::readCallback(asio_ns::error_code ec) {
245246
size_t nparsed = 0;
246247
for (auto const& buffer : this->_protocol->buffer.data()) {
247248
const char* data = reinterpret_cast<const char*>(buffer.data());
248-
249-
err = llhttp_execute(&_parser, data, buffer.size());
250-
if (err != HPE_OK) {
251-
ptrdiff_t diff = llhttp_get_error_pos(&_parser) - data;
252-
TRI_ASSERT(diff >= 0);
253-
nparsed += static_cast<size_t>(diff);
254-
break;
255-
}
256-
nparsed += buffer.size();
249+
const char* end = data + buffer.size();
250+
do {
251+
size_t datasize = end - data;
252+
253+
TRI_IF_FAILURE("HttpCommTask<T>::readCallback_in_small_chunks") {
254+
// we had an issue that URLs were cut off because the url data was handed
255+
// in in multiple buffers.
256+
// To cover this case, we simulate that data fed to the parser in small chunks.
257+
constexpr size_t chunksize = 5;
258+
datasize = std::min<size_t>(datasize, chunksize);
259+
}
260+
261+
err = llhttp_execute(&_parser, data, datasize);
262+
if (err != HPE_OK) {
263+
ptrdiff_t diff = llhttp_get_error_pos(&_parser) - data;
264+
TRI_ASSERT(diff >= 0);
265+
nparsed += static_cast<size_t>(diff);
266+
break;
267+
}
268+
nparsed += datasize;
269+
data += datasize;
270+
} while (ADB_UNLIKELY(data < end));
257271
}
258272

259273
TRI_ASSERT(nparsed < std::numeric_limits<size_t>::max());

‎arangod/GeneralServer/HttpCommTask.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ class HttpCommTask final : public GeneralCommTask<T> {
8686
std::string _lastHeaderField;
8787
std::string _lastHeaderValue;
8888
std::string _origin; // value of the HTTP origin header the client sent
89+
std::string _url;
8990
std::unique_ptr<HttpRequest> _request;
9091
std::unique_ptr<basics::StringBuffer> _response;
9192
bool _lastHeaderWasValue;

‎tests/js/client/shell/shell-request.js

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,16 @@ var request = require('@arangodb/request');
3434
var url = require('url');
3535
var querystring = require('querystring');
3636
var qs = require('qs');
37+
const deriveTestSuite = require('@arangodb/test-helper').deriveTestSuite;
38+
const internal = require("internal");
3739

40+
'use strict';
3841

3942
////////////////////////////////////////////////////////////////////////////////
4043
/// @brief test suite
4144
////////////////////////////////////////////////////////////////////////////////
4245

43-
function RequestSuite () {
44-
'use strict';
46+
function BaseRequestSuite () {
4547
var buildUrl = function (append, base) {
4648
base = base === false ? '' : '/_admin/echo';
4749
append = append || '';
@@ -395,12 +397,33 @@ function RequestSuite () {
395397
};
396398
}
397399

400+
function RequestSuite () {
401+
let suite = {};
402+
deriveTestSuite(BaseRequestSuite(), suite, '');
403+
return suite;
404+
}
405+
406+
function RequestSuiteWithSmallChunks () {
407+
let suite = {
408+
setUpAll: function() {
409+
internal.debugSetFailAt("HttpCommTask<T>::readCallback_in_small_chunks");
410+
},
411+
412+
tearDownAll: function() {
413+
internal.debugClearFailAt();
414+
},
415+
};
416+
417+
deriveTestSuite(BaseRequestSuite(), suite, '_SmallChunks');
418+
return suite;
419+
}
398420

399421
////////////////////////////////////////////////////////////////////////////////
400422
/// @brief executes the test suite
401423
////////////////////////////////////////////////////////////////////////////////
402424

403425
jsunity.run(RequestSuite);
426+
jsunity.run(RequestSuiteWithSmallChunks);
404427

405428
return jsunity.done();
406429

0 commit comments

Comments
 (0)
0