From 29571a2e3a439c2367cf00523203454fca62d188 Mon Sep 17 00:00:00 2001 From: Jan Steemann Date: Tue, 16 Oct 2012 11:33:49 +0200 Subject: [PATCH] issue #231: handle more client error cases --- lib/HttpServer/HttpCommTask.h | 84 ++++++++++++++++++++++++----------- lib/Rest/HttpRequest.cpp | 14 ++++-- lib/Rest/HttpRequest.h | 4 +- 3 files changed, 69 insertions(+), 33 deletions(-) diff --git a/lib/HttpServer/HttpCommTask.h b/lib/HttpServer/HttpCommTask.h index 1b37b4fe9e..4c48d2aed5 100644 --- a/lib/HttpServer/HttpCommTask.h +++ b/lib/HttpServer/HttpCommTask.h @@ -197,44 +197,31 @@ namespace triagens { case HttpRequest::HTTP_REQUEST_GET: case HttpRequest::HTTP_REQUEST_DELETE: case HttpRequest::HTTP_REQUEST_HEAD: - this->_bodyLength = this->_request->contentLength(); - - if (this->_bodyLength > 0) { - LOGGER_WARNING << "received http GET/DELETE/HEAD request with body length, this should not happen"; - this->_readRequestBody = true; - } - else { - handleRequest = true; - } - break; - case HttpRequest::HTTP_REQUEST_POST: case HttpRequest::HTTP_REQUEST_PUT: - case HttpRequest::HTTP_REQUEST_PATCH: - this->_bodyLength = this->_request->contentLength(); + case HttpRequest::HTTP_REQUEST_PATCH: { + const bool expectContentLength = (this->_requestType == HttpRequest::HTTP_REQUEST_POST || + this->_requestType == HttpRequest::HTTP_REQUEST_PUT || + this->_requestType == HttpRequest::HTTP_REQUEST_DELETE); - if (this->_bodyLength > 0) { - this->_readRequestBody = true; - - if (this->_bodyLength > this->_maximalBodySize) { - LOGGER_WARNING << "maximal body size is " << this->_maximalBodySize << ", request body size is " << this->_bodyLength; - // request entity too large - HttpResponse response(HttpResponse::ENTITY_TOO_LARGE); - this->handleResponse(&response); - return true; - } + if (! checkContentLength(expectContentLength)) { + return true; } - else { + + if (this->_bodyLength == 0) { handleRequest = true; } break; + } - default: + default: { LOGGER_WARNING << "got corrupted HTTP request '" << string(this->_readBuffer->c_str(), (this->_readPosition < 6 ? this->_readPosition : 6)) << "'"; - // bad request - HttpResponse response(HttpResponse::BAD); + // bad request, method not allowed + HttpResponse response(HttpResponse::METHOD_NOT_ALLOWED); this->handleResponse(&response); + return true; + } } // check for a 100-continue @@ -430,6 +417,7 @@ namespace triagens { response->headResponse(response->bodySize()); } + // reserve some outbuffer size const size_t len = response->bodySize() + 128; triagens::basics::StringBuffer* buffer = new triagens::basics::StringBuffer(TRI_UNKNOWN_MEM_ZONE, len); // write header @@ -455,6 +443,48 @@ namespace triagens { this->fillWriteBuffer(); } +//////////////////////////////////////////////////////////////////////////////// +/// check the content-length header of a request and fail it is broken +//////////////////////////////////////////////////////////////////////////////// + + bool checkContentLength (const bool expectContentLength) { + const int64_t bodyLength = this->_request->contentLength(); + + if (bodyLength < 0) { + // bad request, body length is < 0. this is a client error + HttpResponse response(HttpResponse::LENGTH_REQUIRED); + this->handleResponse(&response); + + return false; + } + + if (! expectContentLength && bodyLength > 0) { + // content-length header was sent but the request method does not support that + // we'll warn but read the body anyway + LOGGER_WARNING << "received HTTP GET/DELETE/HEAD request with content-length, this should not happen"; + } + + if ((size_t) bodyLength > this->_maximalBodySize) { + // request entity too large + LOGGER_WARNING << "maximal body size is " << this->_maximalBodySize << ", request body size is " << bodyLength; + HttpResponse response(HttpResponse::ENTITY_TOO_LARGE); + this->handleResponse(&response); + + return false; + } + + // set instance variable to content-length value + this->_bodyLength = (size_t) bodyLength; + if (this->_bodyLength > 0) { + // we'll read the body + this->_readRequestBody = true; + } + + // everything's fine + return true; + } + + //////////////////////////////////////////////////////////////////////////////// /// @} //////////////////////////////////////////////////////////////////////////////// diff --git a/lib/Rest/HttpRequest.cpp b/lib/Rest/HttpRequest.cpp index 72af159c83..010a81b8df 100644 --- a/lib/Rest/HttpRequest.cpp +++ b/lib/Rest/HttpRequest.cpp @@ -235,7 +235,7 @@ void HttpRequest::write (TRI_string_buffer_t* buffer) const { } TRI_AppendString2StringBuffer(buffer, "content-length: ", 16); - TRI_AppendUInt64StringBuffer(buffer, _contentLength); + TRI_AppendInt64StringBuffer(buffer, _contentLength); TRI_AppendString2StringBuffer(buffer, "\r\n\r\n", 4); if (_body != 0 && 0 < _bodySize) { @@ -247,7 +247,7 @@ void HttpRequest::write (TRI_string_buffer_t* buffer) const { /// {@inheritDoc} //////////////////////////////////////////////////////////////////////////////// -size_t HttpRequest::contentLength () const { +int64_t HttpRequest::contentLength () const { return _contentLength; } @@ -385,10 +385,15 @@ size_t HttpRequest::bodySize () const { int HttpRequest::setBody (char const* newBody, size_t length) { _body = TRI_DuplicateString2Z(TRI_UNKNOWN_MEM_ZONE, newBody, length); - _contentLength = _bodySize = length; + if (_body == 0) { + return TRI_ERROR_OUT_OF_MEMORY; + } _freeables.push_back(_body); + _contentLength = (int64_t) length; + _bodySize = length; + return TRI_ERROR_NO_ERROR; } @@ -398,7 +403,8 @@ int HttpRequest::setBody (char const* newBody, size_t length) { void HttpRequest::setHeader (char const* key, size_t keyLength, char const* value) { if (keyLength == 14 && memcmp(key, "content-length", keyLength) == 0) { // 14 = strlen("content-length") - _contentLength = TRI_UInt64String(value); + + _contentLength = TRI_Int64String(value); } else { _headers.insert(key, keyLength, value); diff --git a/lib/Rest/HttpRequest.h b/lib/Rest/HttpRequest.h index 92a8c9369d..71f94df759 100644 --- a/lib/Rest/HttpRequest.h +++ b/lib/Rest/HttpRequest.h @@ -289,7 +289,7 @@ namespace triagens { /// @brief returns the content length //////////////////////////////////////////////////////////////////////////////// - size_t contentLength () const; + int64_t contentLength () const; //////////////////////////////////////////////////////////////////////////////// /// @brief returns a header field @@ -519,7 +519,7 @@ namespace triagens { /// @brief content length //////////////////////////////////////////////////////////////////////////////// - size_t _contentLength; + int64_t _contentLength; //////////////////////////////////////////////////////////////////////////////// /// @brief body