From eb1aa6e0242f7e49078c14c4fef3be2f92ecd8a1 Mon Sep 17 00:00:00 2001 From: Lars Maier Date: Fri, 28 Jun 2019 10:02:48 +0200 Subject: [PATCH] Response compression (#9300) * First draft of response compression. * Cleanup. * Removed compression from /_api/version. * Added ruby test for response compression. --- arangod/Agency/RestAgencyHandler.cpp | 3 +- arangod/GeneralServer/RestHandler.cpp | 18 ++++++++ arangod/GeneralServer/RestHandler.h | 1 + arangod/RestHandler/RestVersionHandler.cpp | 4 +- lib/Basics/StaticStrings.cpp | 3 ++ lib/Basics/StaticStrings.h | 3 ++ lib/Rest/CommonDefines.h | 5 ++ lib/Rest/GeneralRequest.h | 13 ++++-- lib/Rest/GeneralResponse.cpp | 1 + lib/Rest/GeneralResponse.h | 8 ++++ lib/Rest/HttpRequest.cpp | 7 +++ lib/Rest/HttpResponse.h | 4 +- lib/Rest/VstResponse.h | 3 ++ tests/IResearch/RestHandlerMock.h | 2 + tests/rb/HttpInterface/api-http-spec.rb | 53 ++++++++++++++++------ 15 files changed, 106 insertions(+), 22 deletions(-) diff --git a/arangod/Agency/RestAgencyHandler.cpp b/arangod/Agency/RestAgencyHandler.cpp index 87e2573f3b..7d330b65a5 100644 --- a/arangod/Agency/RestAgencyHandler.cpp +++ b/arangod/Agency/RestAgencyHandler.cpp @@ -568,7 +568,7 @@ RestStatus RestAgencyHandler::handleConfig() { RestStatus RestAgencyHandler::handleState() { VPackBuilder body; - { + { VPackObjectBuilder o(&body); _agent->readDB(body); } @@ -583,6 +583,7 @@ RestStatus RestAgencyHandler::reportMethodNotAllowed() { } RestStatus RestAgencyHandler::execute() { + response()->setAllowCompression(true); try { auto const& suffixes = _request->suffixes(); if (suffixes.empty()) { // Empty request diff --git a/arangod/GeneralServer/RestHandler.cpp b/arangod/GeneralServer/RestHandler.cpp index cef3982f29..0c914ce178 100644 --- a/arangod/GeneralServer/RestHandler.cpp +++ b/arangod/GeneralServer/RestHandler.cpp @@ -295,6 +295,8 @@ void RestHandler::runHandlerStateMachine() { case HandlerState::FINALIZE: RequestStatistics::SET_REQUEST_END(_statistics); + // compress response if required + compressResponse(); // Callback may stealStatistics! _callback(this); // Schedule callback BEFORE! finalize @@ -485,6 +487,22 @@ void RestHandler::generateError(rest::ResponseCode code, int errorNumber, } } +void RestHandler::compressResponse() { + if (_response->isCompressionAllowed()) { + + switch (_request->acceptEncoding()) { + case rest::EncodingType::DEFLATE: + _response->deflate(); + _response->setHeader(StaticStrings::ContentEncoding, StaticStrings::EncodingDeflate); + break; + + default: + break; + } + + } +} + //////////////////////////////////////////////////////////////////////////////// /// @brief generates an error //////////////////////////////////////////////////////////////////////////////// diff --git a/arangod/GeneralServer/RestHandler.h b/arangod/GeneralServer/RestHandler.h index dcc4058fe0..f222fe3159 100644 --- a/arangod/GeneralServer/RestHandler.h +++ b/arangod/GeneralServer/RestHandler.h @@ -147,6 +147,7 @@ class RestHandler : public std::enable_shared_from_this { /// otherwise execute() will be called void executeEngine(bool isContinue); void shutdownEngine(); + void compressResponse(); protected: enum class HandlerState { diff --git a/arangod/RestHandler/RestVersionHandler.cpp b/arangod/RestHandler/RestVersionHandler.cpp index f2b8dcc253..b292fa5acd 100644 --- a/arangod/RestHandler/RestVersionHandler.cpp +++ b/arangod/RestHandler/RestVersionHandler.cpp @@ -50,7 +50,7 @@ RestStatus RestVersionHandler::execute() { application_features::ApplicationServer::getFeature( "ServerSecurity"); TRI_ASSERT(security != nullptr); - + bool const allowInfo = security->canAccessHardenedApi(); result.add(VPackValue(VPackValueType::Object)); @@ -88,6 +88,8 @@ RestStatus RestVersionHandler::execute() { } // found } // allowInfo result.close(); + response()->setAllowCompression(true); + generateResult(rest::ResponseCode::OK, result.slice()); return RestStatus::DONE; } diff --git a/lib/Basics/StaticStrings.cpp b/lib/Basics/StaticStrings.cpp index 45c101c8b5..61fdc14e43 100644 --- a/lib/Basics/StaticStrings.cpp +++ b/lib/Basics/StaticStrings.cpp @@ -189,6 +189,9 @@ std::string const StaticStrings::MimeTypeText("text/plain; charset=utf-8"); std::string const StaticStrings::MimeTypeVPack("application/x-velocypack"); std::string const StaticStrings::MultiPartContentType("multipart/form-data"); +// accept-encodings +std::string const StaticStrings::EncodingDeflate("deflate"); + // collection attributes std::string const StaticStrings::DistributeShardsLike("distributeShardsLike"); std::string const StaticStrings::IsSmart("isSmart"); diff --git a/lib/Basics/StaticStrings.h b/lib/Basics/StaticStrings.h index 35894b9e9a..cde399d67f 100644 --- a/lib/Basics/StaticStrings.h +++ b/lib/Basics/StaticStrings.h @@ -174,6 +174,9 @@ class StaticStrings { static std::string const MimeTypeVPack; static std::string const MultiPartContentType; + // encodings + static std::string const EncodingDeflate; + // collection attributes static std::string const NumberOfShards; static std::string const IsSmart; diff --git a/lib/Rest/CommonDefines.h b/lib/Rest/CommonDefines.h index b7f46d61da..b419d32364 100644 --- a/lib/Rest/CommonDefines.h +++ b/lib/Rest/CommonDefines.h @@ -91,6 +91,11 @@ enum class ContentType { UNSET }; +enum class EncodingType { + DEFLATE, + UNSET +}; + enum class ProtocolVersion { HTTP_1_0, HTTP_1_1, VST_1_0, VST_1_1, UNKNOWN }; enum class ConnectionType { C_NONE, C_KEEP_ALIVE, C_CLOSE }; diff --git a/lib/Rest/GeneralRequest.h b/lib/Rest/GeneralRequest.h index 0c6087b556..ade2aab1c3 100644 --- a/lib/Rest/GeneralRequest.h +++ b/lib/Rest/GeneralRequest.h @@ -48,6 +48,7 @@ class StringBuffer; } using rest::ContentType; +using rest::EncodingType; using rest::ProtocolVersion; using rest::RequestType; @@ -86,7 +87,8 @@ class GeneralRequest { _authenticated(false), _type(RequestType::ILLEGAL), _contentType(ContentType::UNSET), - _contentTypeResponse(ContentType::UNSET) {} + _contentTypeResponse(ContentType::UNSET), + _acceptEncoding(EncodingType::UNSET) {} virtual ~GeneralRequest(); @@ -153,9 +155,9 @@ class GeneralRequest { TEST_VIRTUAL std::vector const& suffixes() const { return _suffixes; } - + void addSuffix(std::string part); - + #ifdef ARANGODB_USE_GOOGLE_TESTS void clearSuffixes() { _suffixes.clear(); @@ -178,7 +180,7 @@ class GeneralRequest { std::unordered_map const& headers() const { return _headers; } - + #ifdef ARANGODB_USE_GOOGLE_TESTS void addHeader(std::string key, std::string value) { _headers.emplace(std::move(key), std::move(value)); @@ -217,6 +219,8 @@ class GeneralRequest { ContentType contentType() const { return _contentType; } /// @brief should generally reflect the Accept header ContentType contentTypeResponse() const { return _contentTypeResponse; } + /// @brief should generally reflect the Accept-Encoding header + EncodingType acceptEncoding() const { return _acceptEncoding; } rest::AuthenticationMethod authenticationMethod() const { return _authenticationMethod; @@ -251,6 +255,7 @@ class GeneralRequest { std::vector _suffixes; ContentType _contentType; // UNSET, VPACK, JSON ContentType _contentTypeResponse; + EncodingType _acceptEncoding; std::unordered_map _headers; std::unordered_map _values; diff --git a/lib/Rest/GeneralResponse.cpp b/lib/Rest/GeneralResponse.cpp index 53f5554ab8..03ad4e55da 100644 --- a/lib/Rest/GeneralResponse.cpp +++ b/lib/Rest/GeneralResponse.cpp @@ -437,4 +437,5 @@ GeneralResponse::GeneralResponse(ResponseCode responseCode) _contentType(ContentType::UNSET), _connectionType(ConnectionType::C_NONE), _generateBody(false), + _allowCompression(false), _contentTypeRequested(ContentType::UNSET) {} diff --git a/lib/Rest/GeneralResponse.h b/lib/Rest/GeneralResponse.h index 98b5813479..2b16fed07d 100644 --- a/lib/Rest/GeneralResponse.h +++ b/lib/Rest/GeneralResponse.h @@ -41,6 +41,7 @@ class Slice; using rest::ConnectionType; using rest::ContentType; +using rest::EncodingType; using rest::ResponseCode; class GeneralRequest; @@ -77,6 +78,10 @@ class GeneralResponse { _contentType = ContentType::CUSTOM; } + + void setAllowCompression(bool allowed) { _allowCompression = allowed; } + virtual bool isCompressionAllowed() { return _allowCompression; } + void setConnectionType(ConnectionType type) { _connectionType = type; } void setContentTypeRequested(ContentType type) { _contentTypeRequested = type; @@ -158,6 +163,8 @@ class GeneralResponse { /// used for head virtual bool setGenerateBody(bool) { return _generateBody; }; + virtual int deflate(size_t size = 16384) = 0; + protected: ResponseCode _responseCode; // http response code std::unordered_map _headers; // headers/metadata map @@ -165,6 +172,7 @@ class GeneralResponse { ContentType _contentType; ConnectionType _connectionType; bool _generateBody; + bool _allowCompression; ContentType _contentTypeRequested; }; } // namespace arangodb diff --git a/lib/Rest/HttpRequest.cpp b/lib/Rest/HttpRequest.cpp index ac11aff274..f1fb5e64a5 100644 --- a/lib/Rest/HttpRequest.cpp +++ b/lib/Rest/HttpRequest.cpp @@ -552,6 +552,13 @@ void HttpRequest::setHeader(char const* key, size_t keyLength, memcmp(key, StaticStrings::Accept.c_str(), keyLength) == 0 && memcmp(value, StaticStrings::MimeTypeVPack.c_str(), valueLength) == 0) { _contentTypeResponse = ContentType::VPACK; + } else if (keyLength == StaticStrings::AcceptEncoding.size() && + valueLength == StaticStrings::EncodingDeflate.size() && + memcmp(key, StaticStrings::AcceptEncoding.c_str(), keyLength) == 0 && + memcmp(value, StaticStrings::EncodingDeflate.c_str(), valueLength) == 0) { + // This can be much more elaborated as the can specify weights on encodings + // However, for now just toggle on deflate if deflate is requested + _acceptEncoding = EncodingType::DEFLATE; } else if (keyLength == StaticStrings::ContentTypeHeader.size() && valueLength == StaticStrings::MimeTypeVPack.size() && memcmp(key, StaticStrings::ContentTypeHeader.c_str(), keyLength) == 0 && diff --git a/lib/Rest/HttpResponse.h b/lib/Rest/HttpResponse.h index 0ef16c3074..fa6538868d 100644 --- a/lib/Rest/HttpResponse.h +++ b/lib/Rest/HttpResponse.h @@ -96,7 +96,9 @@ class HttpResponse : public GeneralResponse { private: // the body must already be set. deflate is then run on the existing body - int deflate(size_t = 16384); + int deflate(size_t size = 16384) override { + return _body->deflate(size); + } std::unique_ptr stealBody() { std::unique_ptr bb(_body); diff --git a/lib/Rest/VstResponse.h b/lib/Rest/VstResponse.h index 0bb83e6dcb..64d74352cb 100644 --- a/lib/Rest/VstResponse.h +++ b/lib/Rest/VstResponse.h @@ -61,6 +61,9 @@ class VstResponse : public GeneralResponse { void addPayload(VPackBuffer&&, arangodb::velocypack::Options const* = nullptr, bool resolveExternals = true) override; + bool isCompressionAllowed() override { return false; } + int deflate(size_t size = 16384) override { return 0; }; + private: //_responseCode - from Base //_headers - from Base diff --git a/tests/IResearch/RestHandlerMock.h b/tests/IResearch/RestHandlerMock.h index 3a761124b1..ef180e5f9f 100644 --- a/tests/IResearch/RestHandlerMock.h +++ b/tests/IResearch/RestHandlerMock.h @@ -55,6 +55,8 @@ struct GeneralResponseMock: public arangodb::GeneralResponse { virtual void addPayload(arangodb::velocypack::Slice const& slice, arangodb::velocypack::Options const* options = nullptr, bool resolveExternals = true) override; virtual void reset(arangodb::ResponseCode code) override; virtual arangodb::Endpoint::TransportType transportType() override; + int deflate(size_t size = 16384) override { return 0; } + bool isCompressionAllowed() override { return false; } }; #endif diff --git a/tests/rb/HttpInterface/api-http-spec.rb b/tests/rb/HttpInterface/api-http-spec.rb index c5f60798e2..9906f598ee 100644 --- a/tests/rb/HttpInterface/api-http-spec.rb +++ b/tests/rb/HttpInterface/api-http-spec.rb @@ -13,12 +13,12 @@ describe ArangoDB do context "binary data" do before do # make sure system collections exist - ArangoDB.post("/_admin/execute", :body => "var db = require('internal').db; try { db._create('_modules', { isSystem: true, distributeShardsLike: '_graphs' }); } catch (err) {} try { db._create('_routing', { isSystem: true, distributeShardsLike: '_graphs' }); } catch (err) {}") - + ArangoDB.post("/_admin/execute", :body => "var db = require('internal').db; try { db._create('_modules', { isSystem: true, distributeShardsLike: '_graphs' }); } catch (err) {} try { db._create('_routing', { isSystem: true, distributeShardsLike: '_graphs' }); } catch (err) {}") + # clean up first ArangoDB.delete("/_api/document/_modules/UnitTestRoutingTest") ArangoDB.delete("/_api/document/_routing/UnitTestRoutingTest") - + # register module in _modules body = "{ \"_key\" : \"UnitTestRoutingTest\", \"path\" : \"/db:/FoxxTest\", \"content\" : \"exports.do = function(req, res, options, next) { res.body = require('internal').rawRequestBody(req); res.responseCode = 201; res.contentType = 'application/x-foobar'; };\" }" doc = ArangoDB.log_post("#{prefix}-post-binary-data", "/_api/document?collection=_modules", :body => body) @@ -28,16 +28,16 @@ describe ArangoDB do body = "{ \"_key\" : \"UnitTestRoutingTest\", \"url\" : { \"match\" : \"/foxxtest\", \"methods\" : [ \"post\", \"put\" ] }, \"action\": { \"controller\" : \"db://FoxxTest\" } }" doc = ArangoDB.log_post("#{prefix}-post-binary-data", "/_api/document?collection=_routing", :body => body) doc.code.should eq(202) - + ArangoDB.log_post("#{prefix}-post-binary-data", "/_admin/routing/reload", :body => "") end after do ArangoDB.delete("/_api/document/_modules/UnitTestRoutingTest") ArangoDB.delete("/_api/document/_routing/UnitTestRoutingTest") - + # drop collections - ArangoDB.post("/_admin/execute", :body => "var db = require('internal').db; try { db._drop('_modules', true); } catch (err) {} try { db._drop('_routing', true); } catch (err) {}") + ArangoDB.post("/_admin/execute", :body => "var db = require('internal').db; try { db._drop('_modules', true); } catch (err) {} try { db._drop('_routing', true); } catch (err) {}") end it "checks handling of a request with binary data" do @@ -69,7 +69,7 @@ describe ArangoDB do it "calls an action and times out" do cmd = "/_admin/execute" body = "require('internal').wait(4);" - begin + begin ArangoDB.log_post("#{prefix}-http-timeout", cmd, :body => body) rescue Timeout::Error # if we get any different error, the rescue block won't catch it and @@ -91,9 +91,9 @@ describe ArangoDB do it "calls an action and times out" do cmd = "/_admin/execute" body = "require('internal').wait(4);" - begin + begin ArangoDB.log_post("#{prefix}-http-timeout", cmd, :body => body) - rescue Timeout::Error + rescue Timeout::Error # if we get any different error, the rescue block won't catch it and # the test will fail end @@ -114,7 +114,7 @@ describe ArangoDB do doc.code.should eq(200) doc.response.body.should be_nil end - + it "checks whether HEAD returns a body on 3xx" do cmd = "/_api/collection" doc = ArangoDB.log_head("#{prefix}-head-unsupported-method1", cmd) @@ -130,7 +130,7 @@ describe ArangoDB do doc.code.should eq(405) doc.response.body.should be_nil end - + it "checks whether HEAD returns a body on 4xx" do cmd = "/_api/non-existing-method" doc = ArangoDB.log_head("#{prefix}-head-non-existing-method", cmd) @@ -145,7 +145,7 @@ describe ArangoDB do # create collection with one document @cid = ArangoDB.create_collection(cn) - + cmd = "/_api/document?collection=#{cn}" body = "{ \"Hello\" : \"World\" }" doc = ArangoDB.log_post("#{prefix}", cmd, :body => body) @@ -210,7 +210,7 @@ describe ArangoDB do end ################################################################################ -## checking HTTP OPTIONS +## checking HTTP OPTIONS ################################################################################ context "options requests" do @@ -271,6 +271,29 @@ describe ArangoDB do end end +################################################################################ +## checking GZIP requests +################################################################################ + + context "deflate requests" do + it "checks handling of a request, with deflate support" do + require 'uri' + require 'net/http' + require 'zlib' + + cmd = "/_api/version" + deflatedVersion = ArangoDB.log_get("version-deflate-get", cmd, :headers => { "Accept-Encoding" => "deflate" }, :format => :plain) + version = ArangoDB.log_get("version-get", cmd, :headers => { "Accept-Encoding" => "" }, :format => :plain) + + # check content encoding + deflatedVersion.headers['Content-Encoding'].should eq('deflate') + + # compare both responses + inflatedVersionStr = Zlib::inflate deflatedVersion.body + version.body.should eq(inflatedVersionStr) + end + end + ################################################################################ ## checking CORS requests ################################################################################ @@ -301,7 +324,7 @@ describe ArangoDB do doc.headers['access-control-allow-credentials'].should eq("false") doc.headers['access-control-max-age'].should be_nil end - + it "checks handling of a CORS GET request" do cmd = "/_api/version" doc = ArangoDB.log_get("#{prefix}-cors", cmd, { :headers => { "Origin" => "http://127.0.0.1" } } ) @@ -313,7 +336,7 @@ describe ArangoDB do doc.headers['access-control-allow-credentials'].should eq("false") doc.headers['access-control-max-age'].should be_nil end - + it "checks handling of a CORS GET request from origin that is trusted" do cmd = "/_api/version" doc = ArangoDB.log_get("#{prefix}-cors", cmd, { :headers => { "Origin" => "http://was-erlauben-strunz.it" } } )