prepare the fixing of resthandlers

arangod/GeneralServer/VppCommTask.cpp

@@ -176,13 +176,7 @@ void VppCommTask::addResponse(VppResponse* response) {
 
   std::vector<VPackSlice> slices;
   slices.push_back(response_message._header);
-
+  slices.push_back(response_message._payload);
-  VPackBuilder builder;
-  if (response_message._generateBody) {
-    builder = basics::VelocyPackHelper::sanitizeExternalsChecked(
-        response_message._payload);
-    slices.push_back(builder.slice());
-  }
 
   LOG_TOPIC(DEBUG, Logger::COMMUNICATION) << "VppCommTask: "
                                           << "created response:";

lib/Basics/VelocyPackHelper.cpp

@@ -22,25 +22,25 @@
 ////////////////////////////////////////////////////////////////////////////////
 
 #include "VelocyPackHelper.h"
-#include "Basics/conversions.h"
 #include "Basics/Exceptions.h"
-#include "Logger/Logger.h"
 #include "Basics/StaticStrings.h"
 #include "Basics/StringBuffer.h"
 #include "Basics/StringUtils.h"
 #include "Basics/Utf8Helper.h"
 #include "Basics/VPackStringBufferAdapter.h"
+#include "Basics/conversions.h"
 #include "Basics/files.h"
 #include "Basics/hashes.h"
 #include "Basics/tri-strings.h"
+#include "Logger/Logger.h"
 
 #include <velocypack/AttributeTranslator.h>
-#include <velocypack/velocypack-common.h>
 #include <velocypack/Collection.h>
 #include <velocypack/Dumper.h>
 #include <velocypack/Options.h>
 #include <velocypack/Slice.h>
 #include <velocypack/velocypack-aliases.h>
+#include <velocypack/velocypack-common.h>
 
 extern "C" {
 unsigned long long XXH64(const void* input, size_t length,
@@ -57,18 +57,21 @@ static std::unique_ptr<VPackCustomTypeHandler> CustomTypeHandler;
 // a default custom type handler that prevents throwing exceptions when
 // custom types are encountered during Slice.toJson() and family
 struct DefaultCustomTypeHandler final : public VPackCustomTypeHandler {
-  void dump(VPackSlice const&, VPackDumper* dumper, VPackSlice const&) override {
+  void dump(VPackSlice const&, VPackDumper* dumper,
+            VPackSlice const&) override {
     LOG(WARN) << "DefaultCustomTypeHandler called";
     dumper->appendString("hello from CustomTypeHandler");
   }
-  std::string toString(VPackSlice const&, VPackOptions const*, VPackSlice const&) override {
+  std::string toString(VPackSlice const&, VPackOptions const*,
+                       VPackSlice const&) override {
     LOG(WARN) << "DefaultCustomTypeHandler called";
     return "hello from CustomTypeHandler";
   }
 };
 
 // attribute exclude handler for skipping over system attributes
-struct SystemAttributeExcludeHandler final : public VPackAttributeExcludeHandler {
+struct SystemAttributeExcludeHandler final
+    : public VPackAttributeExcludeHandler {
   bool shouldExclude(VPackSlice const& key, int nesting) override final {
     VPackValueLength keyLength;
     char const* p = key.getString(keyLength);
@@ -116,8 +119,7 @@ void VelocyPackHelper::initialize() {
   VPackOptions::Defaults.attributeTranslator = Translator.get();
   VPackOptions::Defaults.unsupportedTypeBehavior =
       VPackOptions::ConvertUnsupportedType;
-  
+
-  
   CustomTypeHandler.reset(new DefaultCustomTypeHandler);
 
   VPackOptions::Defaults.customTypeHandler = CustomTypeHandler.get();
@@ -126,16 +128,21 @@ void VelocyPackHelper::initialize() {
                                                  // HTTP xfer
 
   // run quick selfs test with the attribute translator
-  TRI_ASSERT(VPackSlice(Translator->translate(StaticStrings::KeyString))
+  TRI_ASSERT(
-                 .getUInt() == KeyAttribute - AttributeBase);
+      VPackSlice(Translator->translate(StaticStrings::KeyString)).getUInt() ==
-  TRI_ASSERT(VPackSlice(Translator->translate(StaticStrings::RevString))
+      KeyAttribute - AttributeBase);
-                 .getUInt() == RevAttribute - AttributeBase);
+  TRI_ASSERT(
-  TRI_ASSERT(VPackSlice(Translator->translate(StaticStrings::IdString))
+      VPackSlice(Translator->translate(StaticStrings::RevString)).getUInt() ==
-                 .getUInt() == IdAttribute - AttributeBase);
+      RevAttribute - AttributeBase);
-  TRI_ASSERT(VPackSlice(Translator->translate(StaticStrings::FromString))
+  TRI_ASSERT(
-                 .getUInt() == FromAttribute - AttributeBase);
+      VPackSlice(Translator->translate(StaticStrings::IdString)).getUInt() ==
-  TRI_ASSERT(VPackSlice(Translator->translate(StaticStrings::ToString))
+      IdAttribute - AttributeBase);
-                 .getUInt() == ToAttribute - AttributeBase);
+  TRI_ASSERT(
+      VPackSlice(Translator->translate(StaticStrings::FromString)).getUInt() ==
+      FromAttribute - AttributeBase);
+  TRI_ASSERT(
+      VPackSlice(Translator->translate(StaticStrings::ToString)).getUInt() ==
+      ToAttribute - AttributeBase);
 
   TRI_ASSERT(VPackSlice(Translator->translate(KeyAttribute - AttributeBase))
                  .copyString() == StaticStrings::KeyString);
@@ -471,7 +478,8 @@ static bool PrintVelocyPack(int fd, VPackSlice const& slice,
   }
 
   arangodb::basics::StringBuffer buffer(TRI_UNKNOWN_MEM_ZONE);
-  arangodb::basics::VPackStringBufferAdapter bufferAdapter(buffer.stringBuffer());
+  arangodb::basics::VPackStringBufferAdapter bufferAdapter(
+      buffer.stringBuffer());
   try {
     VPackDumper dumper(&bufferAdapter);
     dumper.dump(slice);
@@ -882,21 +890,20 @@ bool VelocyPackHelper::hasExternals(VPackSlice input) {
   return false;
 }
 
-VPackBuilder VelocyPackHelper::sanitizeExternalsChecked(VPackSlice input,
+VPackBuffer<uint8_t> VelocyPackHelper::sanitizeExternalsChecked(
-                                                        bool checkExternals) {
+    VPackSlice input, VPackOptions const* options, bool checkExternals) {
-  VPackBuilder builder;
+  VPackBuffer<uint8_t> buffer;
+  VPackBuilder builder(buffer, options);
   bool resolveExt = true;
   if (checkExternals) {
     resolveExt = hasExternals(input);
   }
-
   if (resolveExt) {  // resolve
     SanitizeExternals(input, builder);
   } else {
     builder.add(input);
   }
-
+  return buffer;  // elided
-  return builder;  // elided
 }
 
 arangodb::LoggerStream& operator<<(arangodb::LoggerStream& logger,

lib/Basics/VelocyPackHelper.h

@@ -29,9 +29,9 @@
 #include "Logger/Logger.h"
 
 #include <velocypack/Builder.h>
+#include <velocypack/Options.h>
 #include <velocypack/Parser.h>
 #include <velocypack/Slice.h>
-#include <velocypack/Options.h>
 #include <velocypack/velocypack-aliases.h>
 
 namespace arangodb {
@@ -350,8 +350,11 @@ class VelocyPackHelper {
                                 arangodb::velocypack::Builder&);
 
   static bool hasExternals(arangodb::velocypack::Slice const);
-  static VPackBuilder sanitizeExternalsChecked(
+
-      arangodb::velocypack::Slice const, bool checkExternals = true);
+  static VPackBuffer<uint8_t> sanitizeExternalsChecked(
+      arangodb::velocypack::Slice const,
+      VPackOptions const* options = &VPackOptions::Options::Defaults,
+      bool checkExternals = true);
 
   static uint8_t const KeyAttribute = 0x31;
   static uint8_t const RevAttribute = 0x32;

lib/Rest/GeneralResponse.cpp

@@ -25,10 +25,52 @@
 #include "GeneralResponse.h"
 
 #include "Basics/StringUtils.h"
+#include "Basics/VelocyPackHelper.h"
 
 using namespace arangodb;
 using namespace arangodb::basics;
 
+void GeneralResponse::addPayload(VPackSlice const& slice,
+                                 arangodb::velocypack::Options const* options,
+                                 bool resolve_externals) {
+  if (!options) {
+    options = &arangodb::velocypack::Options::Defaults;
+  }
+
+  if (resolve_externals) {
+    auto tmpBuffer =
+        basics::VelocyPackHelper::sanitizeExternalsChecked(slice, options);
+    _vpackPayloads.push_back(std::move(tmpBuffer));
+  } else {
+    // just copy
+    _vpackPayloads.emplace_back(slice.byteSize());
+    std::memcpy(&_vpackPayloads.back(), slice.start(), slice.byteSize());
+  }
+  addPayloadPostHook(options);
+};
+
+void GeneralResponse::addPayload(VPackBuffer<uint8_t>&& buffer,
+                                 arangodb::velocypack::Options const* options,
+                                 bool resolve_externals) {
+  // TODO
+  // skip sanatizing here for http if conent type is json because it will
+  // be dumped anyway -- check with jsteemann
+  addPayloadPreHook(resolve_externals);
+
+  if (!options) {
+    options = &arangodb::velocypack::Options::Defaults;
+  }
+
+  if (resolve_externals) {
+    auto tmpBuffer = basics::VelocyPackHelper::sanitizeExternalsChecked(
+        VPackSlice(buffer.data()), options);
+    _vpackPayloads.push_back(std::move(tmpBuffer));
+  } else {
+    _vpackPayloads.push_back(std::move(buffer));
+  }
+  addPayloadPostHook(options);
+};
+
 std::string GeneralResponse::responseString(ResponseCode code) {
   switch (code) {
     //  Informational 1xx
@@ -159,8 +201,7 @@ std::string GeneralResponse::responseString(ResponseCode code) {
   return StringUtils::itoa((int)code) + " Unknown";
 }
 
-rest::ResponseCode GeneralResponse::responseCode(
+rest::ResponseCode GeneralResponse::responseCode(std::string const& str) {
-    std::string const& str) {
   int number = ::atoi(str.c_str());
 
   switch (number) {

lib/Rest/GeneralResponse.h

@@ -127,15 +127,16 @@ class GeneralResponse {
                           arangodb::velocypack::Options const& =
                               arangodb::velocypack::Options::Defaults) = 0;
 
-  virtual void addPayload(VPackSlice const& slice) {
+  virtual void addPayloadPreHook(bool& resolve_externals){};
-    // this is slower as it has an extra copy!!
+  virtual void addPayloadPostHook(
-    _vpackPayloads.emplace_back(slice.byteSize());
+      arangodb::velocypack::Options const* options){};
-    std::memcpy(&_vpackPayloads.back(), slice.start(), slice.byteSize());
-  };
 
-  virtual void addPayload(VPackBuffer<uint8_t>&& buffer) {
+  void addPayload(VPackSlice const& slice,
-    _vpackPayloads.push_back(std::move(buffer));
+                  arangodb::velocypack::Options const* options = nullptr,
-  };
+                  bool resolve_externals = true);
+  void addPayload(VPackBuffer<uint8_t>&& buffer,
+                  arangodb::velocypack::Options const* options = nullptr,
+                  bool resolve_externals = true);
 
   void setOptions(VPackOptions options) { _options = std::move(options); };
 

lib/Rest/HttpResponse.cpp

@@ -29,13 +29,13 @@
 #include <velocypack/Options.h>
 #include <velocypack/velocypack-aliases.h>
 
-#include "Meta/conversion.h"
 #include "Basics/Exceptions.h"
 #include "Basics/StringBuffer.h"
 #include "Basics/StringUtils.h"
 #include "Basics/VPackStringBufferAdapter.h"
 #include "Basics/VelocyPackDumper.h"
 #include "Basics/tri-strings.h"
+#include "Meta/conversion.h"
 #include "Rest/GeneralRequest.h"
 
 using namespace arangodb;
@@ -47,7 +47,8 @@ HttpResponse::HttpResponse(ResponseCode code)
     : GeneralResponse(code),
       _isHeadResponse(false),
       _body(TRI_UNKNOWN_MEM_ZONE, false),
-      _bodySize(0) {
+      _bodySize(0),
+      _generateBody(false) {
   _contentType = ContentType::TEXT;
   _connectionType = rest::CONNECTION_KEEP_ALIVE;
   if (_body.c_str() == nullptr) {
@@ -299,19 +300,13 @@ void HttpResponse::writeHeader(StringBuffer* output) {
   // end of header, body to follow
 }
 
-void HttpResponse::setPayload(arangodb::velocypack::Slice const& slice,
+void HttpResponse::addPayloadPostHook(VPackOptions const* options) {
-                              bool generateBody, VPackOptions const& options) {
+  VPackSlice slice(_vpackPayloads.front().data());
-  if (_contentType != rest::ContentType::CUSTOM) {
-    // do not overwrite the content type set by the user!!!
-    //_contentType = contentType; //FIXME
-    //    _contentType =
-    //    meta::enumToEnum<rest::ContentType>(request->contentTypeResponse());
-  }
 
   switch (_contentType) {
     case rest::ContentType::VPACK: {
       size_t length = static_cast<size_t>(slice.byteSize());
-      if (generateBody) {
+      if (_generateBody) {
         _body.appendText(slice.startAs<const char>(), length);
       } else {
         headResponse(length);
@@ -320,9 +315,8 @@ void HttpResponse::setPayload(arangodb::velocypack::Slice const& slice,
     }
     default: {
       setContentType(rest::ContentType::JSON);
-
+      if (_generateBody) {
-      if (generateBody) {
+        arangodb::basics::VelocyPackDumper dumper(&_body, options);
-        arangodb::basics::VelocyPackDumper dumper(&_body, &options);
         dumper.dumpValue(slice);
       } else {
         // TODO can we optimize this?
@@ -333,11 +327,17 @@ void HttpResponse::setPayload(arangodb::velocypack::Slice const& slice,
         VPackStringBufferAdapter buffer(tmp.stringBuffer());
 
         // usual dumping -  but not to the response body
-        VPackDumper dumper(&buffer, &options);
+        VPackDumper dumper(&buffer, options);
         dumper.dump(slice);
 
         headResponse(tmp.length());
       }
     }
   }
+}
+
+void HttpResponse::setPayload(arangodb::velocypack::Slice const& slice,
+                              bool generateBody, VPackOptions const& options) {
+  _generateBody = generateBody;
+  addPayload(slice, &options, true);
 };

lib/Rest/HttpResponse.h

@@ -74,6 +74,7 @@ class HttpResponse : public GeneralResponse {
  public:
   void reset(ResponseCode code) override final;
 
+  void addPayloadPostHook(VPackOptions const* options);
   void setPayload(arangodb::velocypack::Slice const&, bool generateBody,
                   arangodb::velocypack::Options const&) override final;
 
@@ -90,6 +91,7 @@ class HttpResponse : public GeneralResponse {
   std::vector<std::string> _cookies;
   basics::StringBuffer _body;
   size_t _bodySize;
+  bool _generateBody;
 };
 }
 

lib/Rest/VppResponse.cpp

@@ -44,7 +44,7 @@ using namespace arangodb::basics;
 bool VppResponse::HIDE_PRODUCT_HEADER = false;
 
 VppResponse::VppResponse(ResponseCode code, uint64_t id)
-    : GeneralResponse(code), _header(nullptr), _payload(), _messageId(id) {
+    : GeneralResponse(code), _header(nullptr), _messageId(id) {
   _contentType = ContentType::VPACK;
   _connectionType = rest::CONNECTION_KEEP_ALIVE;
 }
@@ -61,8 +61,7 @@ void VppResponse::setPayload(arangodb::velocypack::Slice const& slice,
                              bool generateBody, VPackOptions const& options) {
   if (generateBody) {
     _generateBody = true;
-    _payload.append(slice.startAs<char>(),
+    addPayload(slice, &options);
-                    std::distance(slice.begin(), slice.end()));
   }
 };
 
@@ -78,7 +77,7 @@ VPackMessageNoOwnBuffer VppResponse::prepareForNetwork() {
   builder.close();
   _header = builder.steal();
   return VPackMessageNoOwnBuffer(VPackSlice(_header->data()),
-                                 VPackSlice(_payload.data()), _messageId,
+                                 VPackSlice(_vpackPayloads.front().data()),
-                                 _generateBody);
+                                 _messageId, _generateBody);
 }
 // void VppResponse::writeHeader(basics::StringBuffer*) {}

lib/Rest/VppResponse.h

@@ -64,7 +64,6 @@ class VppResponse : public GeneralResponse {
   //_headers        - from Base
   std::shared_ptr<VPackBuffer<uint8_t>>
       _header;  // generated form _headers when prepared for network
-  VPackBuffer<uint8_t> _payload;
   uint64_t _messageId;
   bool _generateBody;  // this must be true if payload should be send
 };