From b182d87d9c84ffe88b3f9e0959fd314c6dd58aeb Mon Sep 17 00:00:00 2001 From: Ivan Lezhankin Date: Tue, 15 Jun 2021 17:33:46 +0300 Subject: [PATCH 1/3] Add settings for HTTP header limitations --- programs/library-bridge/Handlers.cpp | 2 +- programs/odbc-bridge/ColumnInfoHandler.cpp | 2 +- .../odbc-bridge/IdentifierQuoteHandler.cpp | 2 +- programs/odbc-bridge/MainHandler.cpp | 2 +- programs/odbc-bridge/SchemaAllowedHandler.cpp | 2 +- src/Core/Settings.h | 3 ++ src/Server/HTTP/HTMLForm.cpp | 36 ++++++++++--------- src/Server/HTTP/HTMLForm.h | 26 ++++++-------- src/Server/HTTP/HTTPServerRequest.cpp | 5 ++- src/Server/HTTP/HTTPServerRequest.h | 5 +-- src/Server/HTTP/ReadHeaders.h | 6 +--- src/Server/HTTPHandler.cpp | 2 +- src/Server/InterserverIOHTTPHandler.cpp | 2 +- src/Server/InterserverIOHTTPHandler.h | 2 +- src/Server/ReplicasStatusHandler.cpp | 13 +++---- src/Server/ReplicasStatusHandler.h | 5 +-- 16 files changed, 53 insertions(+), 62 deletions(-) diff --git a/programs/library-bridge/Handlers.cpp b/programs/library-bridge/Handlers.cpp index 6a1bfbbccb7..bc955705556 100644 --- a/programs/library-bridge/Handlers.cpp +++ b/programs/library-bridge/Handlers.cpp @@ -51,7 +51,7 @@ namespace void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response) { LOG_TRACE(log, "Request URI: {}", request.getURI()); - HTMLForm params(request); + HTMLForm params(getContext(), request); if (!params.has("method")) { diff --git a/programs/odbc-bridge/ColumnInfoHandler.cpp b/programs/odbc-bridge/ColumnInfoHandler.cpp index f4f575bb33d..c968ff12f5b 100644 --- a/programs/odbc-bridge/ColumnInfoHandler.cpp +++ b/programs/odbc-bridge/ColumnInfoHandler.cpp @@ -69,7 +69,7 @@ namespace void ODBCColumnsInfoHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response) { - HTMLForm params(request, request.getStream()); + HTMLForm params(getContext(), request, request.getStream()); LOG_TRACE(log, "Request URI: {}", request.getURI()); auto process_error = [&response, this](const std::string & message) diff --git a/programs/odbc-bridge/IdentifierQuoteHandler.cpp b/programs/odbc-bridge/IdentifierQuoteHandler.cpp index 124a5c420f8..2919519f990 100644 --- a/programs/odbc-bridge/IdentifierQuoteHandler.cpp +++ b/programs/odbc-bridge/IdentifierQuoteHandler.cpp @@ -21,7 +21,7 @@ namespace DB { void IdentifierQuoteHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response) { - HTMLForm params(request, request.getStream()); + HTMLForm params(getContext(), request, request.getStream()); LOG_TRACE(log, "Request URI: {}", request.getURI()); auto process_error = [&response, this](const std::string & message) diff --git a/programs/odbc-bridge/MainHandler.cpp b/programs/odbc-bridge/MainHandler.cpp index ffa636e8b49..d8aa617c359 100644 --- a/programs/odbc-bridge/MainHandler.cpp +++ b/programs/odbc-bridge/MainHandler.cpp @@ -50,7 +50,7 @@ void ODBCHandler::processError(HTTPServerResponse & response, const std::string void ODBCHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response) { - HTMLForm params(request); + HTMLForm params(getContext(), request); LOG_TRACE(log, "Request URI: {}", request.getURI()); if (mode == "read") diff --git a/programs/odbc-bridge/SchemaAllowedHandler.cpp b/programs/odbc-bridge/SchemaAllowedHandler.cpp index 3a20148780d..5bf996ec2b4 100644 --- a/programs/odbc-bridge/SchemaAllowedHandler.cpp +++ b/programs/odbc-bridge/SchemaAllowedHandler.cpp @@ -28,7 +28,7 @@ namespace void SchemaAllowedHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response) { - HTMLForm params(request, request.getStream()); + HTMLForm params(getContext(), request, request.getStream()); LOG_TRACE(log, "Request URI: {}", request.getURI()); auto process_error = [&response, this](const std::string & message) diff --git a/src/Core/Settings.h b/src/Core/Settings.h index 2aed174c088..fca593be020 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -234,6 +234,9 @@ class IColumn; M(Seconds, http_send_timeout, DEFAULT_HTTP_READ_BUFFER_TIMEOUT, "HTTP send timeout", 0) \ M(Seconds, http_receive_timeout, DEFAULT_HTTP_READ_BUFFER_TIMEOUT, "HTTP receive timeout", 0) \ M(UInt64, http_max_uri_size, 1048576, "Maximum URI length of HTTP request", 0) \ + M(UInt64, http_max_fields_number, 1000000, "Maximum number of fields in HTTP header", 0) \ + M(UInt64, http_max_field_name_size, 1024, "Maximum length of field name in HTTP header", 0) \ + M(UInt64, http_max_field_value_size, 8192, "Maximum length of field value in HTTP header", 0) \ M(Bool, optimize_throw_if_noop, false, "If setting is enabled and OPTIMIZE query didn't actually assign a merge then an explanatory exception is thrown", 0) \ M(Bool, use_index_for_in_with_subqueries, true, "Try using an index if there is a subquery or a table expression on the right side of the IN operator.", 0) \ M(Bool, joined_subquery_requires_alias, true, "Force joined subqueries and table functions to have aliases for correct name qualification.", 0) \ diff --git a/src/Server/HTTP/HTMLForm.cpp b/src/Server/HTTP/HTMLForm.cpp index 9e0f74dcc33..67f97153861 100644 --- a/src/Server/HTTP/HTMLForm.cpp +++ b/src/Server/HTTP/HTMLForm.cpp @@ -1,5 +1,6 @@ #include +#include #include #include #include @@ -35,38 +36,41 @@ const std::string HTMLForm::ENCODING_MULTIPART = "multipart/form-data"; const int HTMLForm::UNKNOWN_CONTENT_LENGTH = -1; -HTMLForm::HTMLForm() : field_limit(DFL_FIELD_LIMIT), value_length_limit(DFL_MAX_VALUE_LENGTH), encoding(ENCODING_URL) +HTMLForm::HTMLForm(ContextPtr context) + : max_fields_number(context->getSettingsRef().http_max_fields_number) + , max_field_name_size(context->getSettingsRef().http_max_field_name_size) + , max_field_value_size(context->getSettingsRef().http_max_field_value_size) + , encoding(ENCODING_URL) { } -HTMLForm::HTMLForm(const std::string & encoding_) - : field_limit(DFL_FIELD_LIMIT), value_length_limit(DFL_MAX_VALUE_LENGTH), encoding(encoding_) +HTMLForm::HTMLForm(ContextPtr context, const std::string & encoding_) : HTMLForm(context) { + encoding = encoding_; } -HTMLForm::HTMLForm(const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody, PartHandler & handler) - : field_limit(DFL_FIELD_LIMIT), value_length_limit(DFL_MAX_VALUE_LENGTH) +HTMLForm::HTMLForm(ContextPtr context, const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody, PartHandler & handler) + : HTMLForm(context) { load(request, requestBody, handler); } -HTMLForm::HTMLForm(const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody) - : field_limit(DFL_FIELD_LIMIT), value_length_limit(DFL_MAX_VALUE_LENGTH) +HTMLForm::HTMLForm(ContextPtr context, const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody) : HTMLForm(context) { load(request, requestBody); } -HTMLForm::HTMLForm(const Poco::Net::HTTPRequest & request) : HTMLForm(Poco::URI(request.getURI())) +HTMLForm::HTMLForm(ContextPtr context, const Poco::Net::HTTPRequest & request) : HTMLForm(context, Poco::URI(request.getURI())) { } -HTMLForm::HTMLForm(const Poco::URI & uri) : field_limit(DFL_FIELD_LIMIT), value_length_limit(DFL_MAX_VALUE_LENGTH) +HTMLForm::HTMLForm(ContextPtr context, const Poco::URI & uri) : HTMLForm(context) { - ReadBufferFromString istr(uri.getRawQuery()); // STYLE_CHECK_ALLOW_STD_STRING_STREAM + ReadBufferFromString istr(uri.getRawQuery()); // STYLE_CHECK_ALLOW_STD_STRING_STREAM readQuery(istr); } @@ -123,7 +127,7 @@ void HTMLForm::readQuery(ReadBuffer & in) while (true) { - if (field_limit > 0 && fields == field_limit) + if (max_fields_number > 0 && fields == max_fields_number) throw Poco::Net::HTMLFormException("Too many form fields"); std::string name; @@ -133,7 +137,7 @@ void HTMLForm::readQuery(ReadBuffer & in) { if (ch == '+') ch = ' '; - if (name.size() < MAX_NAME_LENGTH) + if (name.size() < max_field_name_size) name += ch; else throw Poco::Net::HTMLFormException("Field name too long"); @@ -145,7 +149,7 @@ void HTMLForm::readQuery(ReadBuffer & in) { if (ch == '+') ch = ' '; - if (value.size() < value_length_limit) + if (value.size() < max_field_value_size) value += ch; else throw Poco::Net::HTMLFormException("Field value too long"); @@ -185,11 +189,11 @@ void HTMLForm::readMultipart(ReadBuffer & in_, PartHandler & handler) /// Read each part until next boundary (or last boundary) while (!in.eof()) { - if (field_limit && fields > field_limit) + if (max_fields_number && fields > max_fields_number) throw Poco::Net::HTMLFormException("Too many form fields"); Poco::Net::MessageHeader header; - readHeaders(header, in); + readHeaders(header, in, max_fields_number, max_field_name_size, max_field_value_size); skipToNextLineOrEOF(in); NameValueCollection params; @@ -209,7 +213,7 @@ void HTMLForm::readMultipart(ReadBuffer & in_, PartHandler & handler) while (in.read(ch)) { - if (value.size() > value_length_limit) + if (value.size() > max_field_value_size) throw Poco::Net::HTMLFormException("Field value too long"); value += ch; } diff --git a/src/Server/HTTP/HTMLForm.h b/src/Server/HTTP/HTMLForm.h index ca6bb9048f1..7935f7b53f1 100644 --- a/src/Server/HTTP/HTMLForm.h +++ b/src/Server/HTTP/HTMLForm.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -19,31 +20,31 @@ public: enum Options { - OPT_USE_CONTENT_LENGTH = 0x01 // don't use Chunked Transfer-Encoding for multipart requests. + OPT_USE_CONTENT_LENGTH = 0x01, /// don't use Chunked Transfer-Encoding for multipart requests. }; /// Creates an empty HTMLForm and sets the /// encoding to "application/x-www-form-urlencoded". - HTMLForm(); + explicit HTMLForm(ContextPtr context); /// Creates an empty HTMLForm that uses the given encoding. /// Encoding must be either "application/x-www-form-urlencoded" (which is the default) or "multipart/form-data". - explicit HTMLForm(const std::string & encoding); + explicit HTMLForm(ContextPtr context, const std::string & encoding); /// Creates a HTMLForm from the given HTTP request. /// Uploaded files are passed to the given PartHandler. - HTMLForm(const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody, PartHandler & handler); + HTMLForm(ContextPtr context, const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody, PartHandler & handler); /// Creates a HTMLForm from the given HTTP request. /// Uploaded files are silently discarded. - HTMLForm(const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody); + HTMLForm(ContextPtr context, const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody); /// Creates a HTMLForm from the given HTTP request. /// The request must be a GET request and the form data must be in the query string (URL encoded). /// For POST requests, you must use one of the constructors taking an additional input stream for the request body. - explicit HTMLForm(const Poco::Net::HTTPRequest & request); + explicit HTMLForm(ContextPtr context, const Poco::Net::HTTPRequest & request); - explicit HTMLForm(const Poco::URI & uri); + explicit HTMLForm(ContextPtr context, const Poco::URI & uri); template T getParsed(const std::string & key, T default_value) @@ -76,13 +77,6 @@ private: /// This buffer provides data line by line to check for boundary line in a convenient way. class MultipartReadBuffer; - enum Limits - { - DFL_FIELD_LIMIT = 100, - MAX_NAME_LENGTH = 1024, - DFL_MAX_VALUE_LENGTH = 256 * 1024 - }; - struct Part { std::string name; @@ -91,8 +85,8 @@ private: using PartVec = std::vector; - size_t field_limit; - size_t value_length_limit; + const size_t max_fields_number, max_field_name_size, max_field_value_size; + std::string encoding; std::string boundary; PartVec parts; diff --git a/src/Server/HTTP/HTTPServerRequest.cpp b/src/Server/HTTP/HTTPServerRequest.cpp index 69dc8d4dbda..d4359ee0355 100644 --- a/src/Server/HTTP/HTTPServerRequest.cpp +++ b/src/Server/HTTP/HTTPServerRequest.cpp @@ -17,6 +17,9 @@ namespace DB { HTTPServerRequest::HTTPServerRequest(ContextPtr context, HTTPServerResponse & response, Poco::Net::HTTPServerSession & session) : max_uri_size(context->getSettingsRef().http_max_uri_size) + , max_fields_number(context->getSettingsRef().http_max_fields_number) + , max_field_name_size(context->getSettingsRef().http_max_field_name_size) + , max_field_value_size(context->getSettingsRef().http_max_field_value_size) { response.attachRequest(this); @@ -110,7 +113,7 @@ void HTTPServerRequest::readRequest(ReadBuffer & in) skipToNextLineOrEOF(in); - readHeaders(*this, in); + readHeaders(*this, in, max_fields_number, max_field_name_size, max_field_value_size); skipToNextLineOrEOF(in); diff --git a/src/Server/HTTP/HTTPServerRequest.h b/src/Server/HTTP/HTTPServerRequest.h index a560f907cf0..c6e8d2dd728 100644 --- a/src/Server/HTTP/HTTPServerRequest.h +++ b/src/Server/HTTP/HTTPServerRequest.h @@ -40,14 +40,11 @@ private: /// Limits for basic sanity checks when reading a header enum Limits { - MAX_NAME_LENGTH = 256, - MAX_VALUE_LENGTH = 8192, MAX_METHOD_LENGTH = 32, MAX_VERSION_LENGTH = 8, - MAX_FIELDS_NUMBER = 100, }; - const size_t max_uri_size; + const size_t max_uri_size, max_fields_number, max_field_name_size, max_field_value_size; std::unique_ptr stream; Poco::Net::SocketImpl * socket; diff --git a/src/Server/HTTP/ReadHeaders.h b/src/Server/HTTP/ReadHeaders.h index e94cddcf489..1b0e627f779 100644 --- a/src/Server/HTTP/ReadHeaders.h +++ b/src/Server/HTTP/ReadHeaders.h @@ -8,10 +8,6 @@ namespace DB class ReadBuffer; void readHeaders( - Poco::Net::MessageHeader & headers, - ReadBuffer & in, - size_t max_fields_number = 100, - size_t max_name_length = 256, - size_t max_value_length = 8192); + Poco::Net::MessageHeader & headers, ReadBuffer & in, size_t max_fields_number, size_t max_name_length, size_t max_value_length); } diff --git a/src/Server/HTTPHandler.cpp b/src/Server/HTTPHandler.cpp index 6eab6cdda5e..28105de7111 100644 --- a/src/Server/HTTPHandler.cpp +++ b/src/Server/HTTPHandler.cpp @@ -895,7 +895,7 @@ void HTTPHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse if (request.getVersion() == HTTPServerRequest::HTTP_1_1) response.setChunkedTransferEncoding(true); - HTMLForm params(request); + HTMLForm params(request_context, request); with_stacktrace = params.getParsed("stacktrace", false); /// FIXME: maybe this check is already unnecessary. diff --git a/src/Server/InterserverIOHTTPHandler.cpp b/src/Server/InterserverIOHTTPHandler.cpp index 64af8860b23..5329d4f77fd 100644 --- a/src/Server/InterserverIOHTTPHandler.cpp +++ b/src/Server/InterserverIOHTTPHandler.cpp @@ -50,7 +50,7 @@ std::pair InterserverIOHTTPHandler::checkAuthentication(HTTPServer void InterserverIOHTTPHandler::processQuery(HTTPServerRequest & request, HTTPServerResponse & response, Output & used_output) { - HTMLForm params(request); + HTMLForm params(server.context(), request); LOG_TRACE(log, "Request URI: {}", request.getURI()); diff --git a/src/Server/InterserverIOHTTPHandler.h b/src/Server/InterserverIOHTTPHandler.h index c0d776115e1..da5b286b9e5 100644 --- a/src/Server/InterserverIOHTTPHandler.h +++ b/src/Server/InterserverIOHTTPHandler.h @@ -1,8 +1,8 @@ #pragma once +#include #include #include -#include #include diff --git a/src/Server/ReplicasStatusHandler.cpp b/src/Server/ReplicasStatusHandler.cpp index 86295cc5170..7cb47b85cda 100644 --- a/src/Server/ReplicasStatusHandler.cpp +++ b/src/Server/ReplicasStatusHandler.cpp @@ -18,23 +18,20 @@ namespace DB { - -ReplicasStatusHandler::ReplicasStatusHandler(IServer & server) - : context(server.context()) +ReplicasStatusHandler::ReplicasStatusHandler(IServer & server) : WithContext(server.context()) { } - void ReplicasStatusHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response) { try { - HTMLForm params(request); + HTMLForm params(getContext(), request); /// Even if lag is small, output detailed information about the lag. bool verbose = params.get("verbose", "") == "1"; - const MergeTreeSettings & settings = context->getReplicatedMergeTreeSettings(); + const MergeTreeSettings & settings = getContext()->getReplicatedMergeTreeSettings(); bool ok = true; WriteBufferFromOwnString message; @@ -48,7 +45,7 @@ void ReplicasStatusHandler::handleRequest(HTTPServerRequest & request, HTTPServe if (!db.second->canContainMergeTreeTables()) continue; - for (auto iterator = db.second->getTablesIterator(context); iterator->isValid(); iterator->next()) + for (auto iterator = db.second->getTablesIterator(getContext()); iterator->isValid(); iterator->next()) { const auto & table = iterator->table(); if (!table) @@ -73,7 +70,7 @@ void ReplicasStatusHandler::handleRequest(HTTPServerRequest & request, HTTPServe } } - const auto & config = context->getConfigRef(); + const auto & config = getContext()->getConfigRef(); setResponseDefaultHeaders(response, config.getUInt("keep_alive_timeout", 10)); if (!ok) diff --git a/src/Server/ReplicasStatusHandler.h b/src/Server/ReplicasStatusHandler.h index eda0b15ed6f..1a5388aa2ab 100644 --- a/src/Server/ReplicasStatusHandler.h +++ b/src/Server/ReplicasStatusHandler.h @@ -9,11 +9,8 @@ class Context; class IServer; /// Replies "Ok.\n" if all replicas on this server don't lag too much. Otherwise output lag information. -class ReplicasStatusHandler : public HTTPRequestHandler +class ReplicasStatusHandler : public HTTPRequestHandler, WithContext { -private: - ContextPtr context; - public: explicit ReplicasStatusHandler(IServer & server_); From ba08a580f8476f8e993528e8ae8ee8eebe0372f5 Mon Sep 17 00:00:00 2001 From: Ivan Lezhankin Date: Wed, 16 Jun 2021 17:33:14 +0300 Subject: [PATCH 2/3] Add test --- programs/library-bridge/Handlers.cpp | 2 +- programs/odbc-bridge/ColumnInfoHandler.cpp | 2 +- .../odbc-bridge/IdentifierQuoteHandler.cpp | 2 +- programs/odbc-bridge/MainHandler.cpp | 2 +- programs/odbc-bridge/SchemaAllowedHandler.cpp | 2 +- src/Core/Settings.h | 6 ++--- src/Server/HTTP/HTMLForm.cpp | 22 +++++++-------- src/Server/HTTP/HTMLForm.h | 15 ++++++----- src/Server/HTTP/HTTPServerRequest.cpp | 2 +- src/Server/HTTPHandler.cpp | 2 +- src/Server/InterserverIOHTTPHandler.cpp | 2 +- src/Server/ReplicasStatusHandler.cpp | 2 +- .../0_stateless/01903_http_fields.reference | 8 ++++++ .../queries/0_stateless/01903_http_fields.sh | 27 +++++++++++++++++++ 14 files changed, 66 insertions(+), 30 deletions(-) create mode 100644 tests/queries/0_stateless/01903_http_fields.reference create mode 100755 tests/queries/0_stateless/01903_http_fields.sh diff --git a/programs/library-bridge/Handlers.cpp b/programs/library-bridge/Handlers.cpp index bc955705556..ec82d7d52f4 100644 --- a/programs/library-bridge/Handlers.cpp +++ b/programs/library-bridge/Handlers.cpp @@ -51,7 +51,7 @@ namespace void LibraryRequestHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response) { LOG_TRACE(log, "Request URI: {}", request.getURI()); - HTMLForm params(getContext(), request); + HTMLForm params(getContext()->getSettingsRef(), request); if (!params.has("method")) { diff --git a/programs/odbc-bridge/ColumnInfoHandler.cpp b/programs/odbc-bridge/ColumnInfoHandler.cpp index c968ff12f5b..4b503afb3b3 100644 --- a/programs/odbc-bridge/ColumnInfoHandler.cpp +++ b/programs/odbc-bridge/ColumnInfoHandler.cpp @@ -69,7 +69,7 @@ namespace void ODBCColumnsInfoHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response) { - HTMLForm params(getContext(), request, request.getStream()); + HTMLForm params(getContext()->getSettingsRef(), request, request.getStream()); LOG_TRACE(log, "Request URI: {}", request.getURI()); auto process_error = [&response, this](const std::string & message) diff --git a/programs/odbc-bridge/IdentifierQuoteHandler.cpp b/programs/odbc-bridge/IdentifierQuoteHandler.cpp index 2919519f990..356c2aadee1 100644 --- a/programs/odbc-bridge/IdentifierQuoteHandler.cpp +++ b/programs/odbc-bridge/IdentifierQuoteHandler.cpp @@ -21,7 +21,7 @@ namespace DB { void IdentifierQuoteHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response) { - HTMLForm params(getContext(), request, request.getStream()); + HTMLForm params(getContext()->getSettingsRef(), request, request.getStream()); LOG_TRACE(log, "Request URI: {}", request.getURI()); auto process_error = [&response, this](const std::string & message) diff --git a/programs/odbc-bridge/MainHandler.cpp b/programs/odbc-bridge/MainHandler.cpp index d8aa617c359..edfd2be0dec 100644 --- a/programs/odbc-bridge/MainHandler.cpp +++ b/programs/odbc-bridge/MainHandler.cpp @@ -50,7 +50,7 @@ void ODBCHandler::processError(HTTPServerResponse & response, const std::string void ODBCHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response) { - HTMLForm params(getContext(), request); + HTMLForm params(getContext()->getSettingsRef(), request); LOG_TRACE(log, "Request URI: {}", request.getURI()); if (mode == "read") diff --git a/programs/odbc-bridge/SchemaAllowedHandler.cpp b/programs/odbc-bridge/SchemaAllowedHandler.cpp index 5bf996ec2b4..af10d8f50cd 100644 --- a/programs/odbc-bridge/SchemaAllowedHandler.cpp +++ b/programs/odbc-bridge/SchemaAllowedHandler.cpp @@ -28,7 +28,7 @@ namespace void SchemaAllowedHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse & response) { - HTMLForm params(getContext(), request, request.getStream()); + HTMLForm params(getContext()->getSettingsRef(), request, request.getStream()); LOG_TRACE(log, "Request URI: {}", request.getURI()); auto process_error = [&response, this](const std::string & message) diff --git a/src/Core/Settings.h b/src/Core/Settings.h index fca593be020..3b91e26cd4f 100644 --- a/src/Core/Settings.h +++ b/src/Core/Settings.h @@ -234,9 +234,9 @@ class IColumn; M(Seconds, http_send_timeout, DEFAULT_HTTP_READ_BUFFER_TIMEOUT, "HTTP send timeout", 0) \ M(Seconds, http_receive_timeout, DEFAULT_HTTP_READ_BUFFER_TIMEOUT, "HTTP receive timeout", 0) \ M(UInt64, http_max_uri_size, 1048576, "Maximum URI length of HTTP request", 0) \ - M(UInt64, http_max_fields_number, 1000000, "Maximum number of fields in HTTP header", 0) \ - M(UInt64, http_max_field_name_size, 1024, "Maximum length of field name in HTTP header", 0) \ - M(UInt64, http_max_field_value_size, 8192, "Maximum length of field value in HTTP header", 0) \ + M(UInt64, http_max_fields, 1000000, "Maximum number of fields in HTTP header", 0) \ + M(UInt64, http_max_field_name_size, 1048576, "Maximum length of field name in HTTP header", 0) \ + M(UInt64, http_max_field_value_size, 1048576, "Maximum length of field value in HTTP header", 0) \ M(Bool, optimize_throw_if_noop, false, "If setting is enabled and OPTIMIZE query didn't actually assign a merge then an explanatory exception is thrown", 0) \ M(Bool, use_index_for_in_with_subqueries, true, "Try using an index if there is a subquery or a table expression on the right side of the IN operator.", 0) \ M(Bool, joined_subquery_requires_alias, true, "Force joined subqueries and table functions to have aliases for correct name qualification.", 0) \ diff --git a/src/Server/HTTP/HTMLForm.cpp b/src/Server/HTTP/HTMLForm.cpp index 67f97153861..86e08f3c8e7 100644 --- a/src/Server/HTTP/HTMLForm.cpp +++ b/src/Server/HTTP/HTMLForm.cpp @@ -1,6 +1,6 @@ #include -#include +#include #include #include #include @@ -36,39 +36,39 @@ const std::string HTMLForm::ENCODING_MULTIPART = "multipart/form-data"; const int HTMLForm::UNKNOWN_CONTENT_LENGTH = -1; -HTMLForm::HTMLForm(ContextPtr context) - : max_fields_number(context->getSettingsRef().http_max_fields_number) - , max_field_name_size(context->getSettingsRef().http_max_field_name_size) - , max_field_value_size(context->getSettingsRef().http_max_field_value_size) +HTMLForm::HTMLForm(const Settings & settings) + : max_fields_number(settings.http_max_fields) + , max_field_name_size(settings.http_max_field_name_size) + , max_field_value_size(settings.http_max_field_value_size) , encoding(ENCODING_URL) { } -HTMLForm::HTMLForm(ContextPtr context, const std::string & encoding_) : HTMLForm(context) +HTMLForm::HTMLForm(const Settings & settings, const std::string & encoding_) : HTMLForm(settings) { encoding = encoding_; } -HTMLForm::HTMLForm(ContextPtr context, const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody, PartHandler & handler) - : HTMLForm(context) +HTMLForm::HTMLForm(const Settings & settings, const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody, PartHandler & handler) + : HTMLForm(settings) { load(request, requestBody, handler); } -HTMLForm::HTMLForm(ContextPtr context, const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody) : HTMLForm(context) +HTMLForm::HTMLForm(const Settings & settings, const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody) : HTMLForm(settings) { load(request, requestBody); } -HTMLForm::HTMLForm(ContextPtr context, const Poco::Net::HTTPRequest & request) : HTMLForm(context, Poco::URI(request.getURI())) +HTMLForm::HTMLForm(const Settings & settings, const Poco::Net::HTTPRequest & request) : HTMLForm(settings, Poco::URI(request.getURI())) { } -HTMLForm::HTMLForm(ContextPtr context, const Poco::URI & uri) : HTMLForm(context) +HTMLForm::HTMLForm(const Settings & settings, const Poco::URI & uri) : HTMLForm(settings) { ReadBufferFromString istr(uri.getRawQuery()); // STYLE_CHECK_ALLOW_STD_STRING_STREAM readQuery(istr); diff --git a/src/Server/HTTP/HTMLForm.h b/src/Server/HTTP/HTMLForm.h index 7935f7b53f1..16889b41d80 100644 --- a/src/Server/HTTP/HTMLForm.h +++ b/src/Server/HTTP/HTMLForm.h @@ -1,6 +1,5 @@ #pragma once -#include #include #include @@ -13,6 +12,8 @@ namespace DB { +struct Settings; + class HTMLForm : public Poco::Net::NameValueCollection, private boost::noncopyable { public: @@ -25,26 +26,26 @@ public: /// Creates an empty HTMLForm and sets the /// encoding to "application/x-www-form-urlencoded". - explicit HTMLForm(ContextPtr context); + explicit HTMLForm(const Settings & settings); /// Creates an empty HTMLForm that uses the given encoding. /// Encoding must be either "application/x-www-form-urlencoded" (which is the default) or "multipart/form-data". - explicit HTMLForm(ContextPtr context, const std::string & encoding); + explicit HTMLForm(const Settings & settings, const std::string & encoding); /// Creates a HTMLForm from the given HTTP request. /// Uploaded files are passed to the given PartHandler. - HTMLForm(ContextPtr context, const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody, PartHandler & handler); + HTMLForm(const Settings & settings, const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody, PartHandler & handler); /// Creates a HTMLForm from the given HTTP request. /// Uploaded files are silently discarded. - HTMLForm(ContextPtr context, const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody); + HTMLForm(const Settings & settings, const Poco::Net::HTTPRequest & request, ReadBuffer & requestBody); /// Creates a HTMLForm from the given HTTP request. /// The request must be a GET request and the form data must be in the query string (URL encoded). /// For POST requests, you must use one of the constructors taking an additional input stream for the request body. - explicit HTMLForm(ContextPtr context, const Poco::Net::HTTPRequest & request); + explicit HTMLForm(const Settings & settings, const Poco::Net::HTTPRequest & request); - explicit HTMLForm(ContextPtr context, const Poco::URI & uri); + explicit HTMLForm(const Settings & settings, const Poco::URI & uri); template T getParsed(const std::string & key, T default_value) diff --git a/src/Server/HTTP/HTTPServerRequest.cpp b/src/Server/HTTP/HTTPServerRequest.cpp index d4359ee0355..788c13557f3 100644 --- a/src/Server/HTTP/HTTPServerRequest.cpp +++ b/src/Server/HTTP/HTTPServerRequest.cpp @@ -17,7 +17,7 @@ namespace DB { HTTPServerRequest::HTTPServerRequest(ContextPtr context, HTTPServerResponse & response, Poco::Net::HTTPServerSession & session) : max_uri_size(context->getSettingsRef().http_max_uri_size) - , max_fields_number(context->getSettingsRef().http_max_fields_number) + , max_fields_number(context->getSettingsRef().http_max_fields) , max_field_name_size(context->getSettingsRef().http_max_field_name_size) , max_field_value_size(context->getSettingsRef().http_max_field_value_size) { diff --git a/src/Server/HTTPHandler.cpp b/src/Server/HTTPHandler.cpp index 28105de7111..271c1700daf 100644 --- a/src/Server/HTTPHandler.cpp +++ b/src/Server/HTTPHandler.cpp @@ -895,7 +895,7 @@ void HTTPHandler::handleRequest(HTTPServerRequest & request, HTTPServerResponse if (request.getVersion() == HTTPServerRequest::HTTP_1_1) response.setChunkedTransferEncoding(true); - HTMLForm params(request_context, request); + HTMLForm params(request_context->getSettingsRef(), request); with_stacktrace = params.getParsed("stacktrace", false); /// FIXME: maybe this check is already unnecessary. diff --git a/src/Server/InterserverIOHTTPHandler.cpp b/src/Server/InterserverIOHTTPHandler.cpp index 5329d4f77fd..d76361e856e 100644 --- a/src/Server/InterserverIOHTTPHandler.cpp +++ b/src/Server/InterserverIOHTTPHandler.cpp @@ -50,7 +50,7 @@ std::pair InterserverIOHTTPHandler::checkAuthentication(HTTPServer void InterserverIOHTTPHandler::processQuery(HTTPServerRequest & request, HTTPServerResponse & response, Output & used_output) { - HTMLForm params(server.context(), request); + HTMLForm params(server.context()->getSettingsRef(), request); LOG_TRACE(log, "Request URI: {}", request.getURI()); diff --git a/src/Server/ReplicasStatusHandler.cpp b/src/Server/ReplicasStatusHandler.cpp index 7cb47b85cda..b7dc8c14858 100644 --- a/src/Server/ReplicasStatusHandler.cpp +++ b/src/Server/ReplicasStatusHandler.cpp @@ -26,7 +26,7 @@ void ReplicasStatusHandler::handleRequest(HTTPServerRequest & request, HTTPServe { try { - HTMLForm params(getContext(), request); + HTMLForm params(getContext()->getSettingsRef(), request); /// Even if lag is small, output detailed information about the lag. bool verbose = params.get("verbose", "") == "1"; diff --git a/tests/queries/0_stateless/01903_http_fields.reference b/tests/queries/0_stateless/01903_http_fields.reference new file mode 100644 index 00000000000..c18b4e9b082 --- /dev/null +++ b/tests/queries/0_stateless/01903_http_fields.reference @@ -0,0 +1,8 @@ +1 +1 +1 +1 +1 +1 +1 +1 diff --git a/tests/queries/0_stateless/01903_http_fields.sh b/tests/queries/0_stateless/01903_http_fields.sh new file mode 100755 index 00000000000..0f2b0df13d9 --- /dev/null +++ b/tests/queries/0_stateless/01903_http_fields.sh @@ -0,0 +1,27 @@ +#!/usr/bin/env bash + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CURDIR"/../shell_config.sh + +DEFAULT_MAX_NAME_SIZE=$($CLICKHOUSE_CLIENT -q "SELECT value FROM system.settings WHERE name='http_max_field_name_size'") +DEFAULT_MAX_VALUE_SIZE=$($CLICKHOUSE_CLIENT -q "SELECT value FROM system.settings WHERE name='http_max_field_value_size'") + +python3 -c "print('a'*($DEFAULT_MAX_NAME_SIZE-2) + ';')" > $CLICKHOUSE_TMP/short_name.txt +python3 -c "print('a'*($DEFAULT_MAX_NAME_SIZE+1) + ';')" > $CLICKHOUSE_TMP/long_name.txt +python3 -c "print('a'*($DEFAULT_MAX_NAME_SIZE-2) + ': ' + 'b'*($DEFAULT_MAX_VALUE_SIZE-2))" > $CLICKHOUSE_TMP/short_short.txt +python3 -c "print('a'*($DEFAULT_MAX_NAME_SIZE-2) + ': ' + 'b'*($DEFAULT_MAX_VALUE_SIZE+1))" > $CLICKHOUSE_TMP/short_long.txt +python3 -c "print('a'*($DEFAULT_MAX_NAME_SIZE+1) + ': ' + 'b'*($DEFAULT_MAX_VALUE_SIZE-2))" > $CLICKHOUSE_TMP/long_short.txt + +${CLICKHOUSE_CURL} -sS "${CLICKHOUSE_URL}" -H @$CLICKHOUSE_TMP/short_name.txt -d 'SELECT 1' +${CLICKHOUSE_CURL} -sSv "${CLICKHOUSE_URL}" -H @$CLICKHOUSE_TMP/long_name.txt -d 'SELECT 1' 2>&1 | grep -Fc '400 Bad Request' +${CLICKHOUSE_CURL} -sS "${CLICKHOUSE_URL}" -H @$CLICKHOUSE_TMP/short_short.txt -d 'SELECT 1' +${CLICKHOUSE_CURL} -sSv "${CLICKHOUSE_URL}" -H @$CLICKHOUSE_TMP/short_long.txt -d 'SELECT 1' 2>&1 | grep -Fc '400 Bad Request' +${CLICKHOUSE_CURL} -sSv "${CLICKHOUSE_URL}" -H @$CLICKHOUSE_TMP/long_short.txt -d 'SELECT 1' 2>&1 | grep -Fc '400 Bad Request' + +# Session and query settings shouldn't affect the HTTP field's name or value sizes. +${CLICKHOUSE_CURL} -sSv "${CLICKHOUSE_URL}&http_max_field_name_size=$(($DEFAULT_MAX_NAME_SIZE+10))" -H @$CLICKHOUSE_TMP/long_name.txt -d 'SELECT 1' 2>&1 | grep -Fc '400 Bad Request' +${CLICKHOUSE_CURL} -sSv "${CLICKHOUSE_URL}&http_max_field_value_size=$(($DEFAULT_MAX_VALUE_SIZE+10))" -H @$CLICKHOUSE_TMP/short_long.txt -d 'SELECT 1' 2>&1 | grep -Fc '400 Bad Request' +${CLICKHOUSE_CURL} -sSv "${CLICKHOUSE_URL}&http_max_field_name_size=$(($DEFAULT_MAX_NAME_SIZE+10))" -H @$CLICKHOUSE_TMP/long_short.txt -d 'SELECT 1' 2>&1 | grep -Fc '400 Bad Request' + +# TODO: test that session context doesn't affect these settings either. From 16931a661c0b7f69f49e1d7fc63ab89aff2ccaa3 Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Thu, 17 Jun 2021 01:50:06 +0300 Subject: [PATCH 3/3] Update HTTPServerRequest.h --- src/Server/HTTP/HTTPServerRequest.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Server/HTTP/HTTPServerRequest.h b/src/Server/HTTP/HTTPServerRequest.h index c6e8d2dd728..9e33e78a424 100644 --- a/src/Server/HTTP/HTTPServerRequest.h +++ b/src/Server/HTTP/HTTPServerRequest.h @@ -44,7 +44,10 @@ private: MAX_VERSION_LENGTH = 8, }; - const size_t max_uri_size, max_fields_number, max_field_name_size, max_field_value_size; + const size_t max_uri_size; + const size_t max_fields_number; + const size_t max_field_name_size; + const size_t max_field_value_size; std::unique_ptr stream; Poco::Net::SocketImpl * socket;