thrift: split transport/protocol code into interface and implementation

source/extensions/filters/network/thrift_proxy/BUILD

@@ -62,34 +62,64 @@ envoy_cc_library(
     ],
 )
 
+envoy_cc_library(
+    name = "protocol_interface",
+    hdrs = [
+        "protocol.h",
+    ],
+    external_deps = ["abseil_optional"],
+    deps = [
+        "//include/envoy/buffer:buffer_interface",
+        "//source/common/singleton:const_singleton",
+    ],
+)
+
 envoy_cc_library(
     name = "protocol_lib",
     srcs = [
-        "binary_protocol.cc",
-        "compact_protocol.cc",
-        "protocol.cc",
+        "binary_protocol_impl.cc",
+        "compact_protocol_impl.cc",
+        "protocol_impl.cc",
     ],
     hdrs = [
-        "binary_protocol.h",
-        "compact_protocol.h",
-        "protocol.h",
+        "binary_protocol_impl.h",
+        "compact_protocol_impl.h",
+        "protocol_impl.h",
     ],
     external_deps = ["abseil_optional"],
     deps = [
         ":buffer_helper_lib",
+        ":protocol_interface",
         "//source/common/singleton:const_singleton",
     ],
 )
 
 envoy_cc_library(
-    name = "transport_lib",
-    srcs = ["transport.cc"],
+    name = "transport_interface",
     hdrs = ["transport.h"],
+    deps = [
+        ":protocol_lib",
+        "//include/envoy/buffer:buffer_interface",
+        "//source/common/singleton:const_singleton",
+    ],
+)
+
+envoy_cc_library(
+    name = "transport_lib",
+    srcs = [
+        "framed_transport_impl.cc",
+        "transport_impl.cc",
+    ],
+    hdrs = [
+        "framed_transport_impl.h",
+        "transport_impl.h",
+        "unframed_transport_impl.h",
+    ],
     deps = [
         ":buffer_helper_lib",
         ":protocol_lib",
+        ":transport_interface",
         "//source/common/common:assert_lib",
-        "//source/common/common:utility_lib",
         "//source/common/singleton:const_singleton",
     ],
 )

source/extensions/filters/network/thrift_proxy/binary_protocol.cc → source/extensions/filters/network/thrift_proxy/binary_protocol_impl.cc

@@ -1,4 +1,4 @@
-#include "extensions/filters/network/thrift_proxy/binary_protocol.h"
+#include "extensions/filters/network/thrift_proxy/binary_protocol_impl.h"
 
 #include "envoy/common/exception.h"
 

source/extensions/filters/network/thrift_proxy/binary_protocol.h → source/extensions/filters/network/thrift_proxy/binary_protocol_impl.h

@@ -5,7 +5,7 @@
 #include "envoy/buffer/buffer.h"
 #include "envoy/common/pure.h"
 
-#include "extensions/filters/network/thrift_proxy/protocol.h"
+#include "extensions/filters/network/thrift_proxy/protocol_impl.h"
 
 namespace Envoy {
 namespace Extensions {

source/extensions/filters/network/thrift_proxy/compact_protocol.cc → source/extensions/filters/network/thrift_proxy/compact_protocol_impl.cc

@@ -1,4 +1,4 @@
-#include "extensions/filters/network/thrift_proxy/compact_protocol.h"
+#include "extensions/filters/network/thrift_proxy/compact_protocol_impl.h"
 
 #include "envoy/common/exception.h"
 

source/extensions/filters/network/thrift_proxy/compact_protocol.h → source/extensions/filters/network/thrift_proxy/compact_protocol_impl.h

@@ -6,7 +6,7 @@
 #include "envoy/buffer/buffer.h"
 #include "envoy/common/pure.h"
 
-#include "extensions/filters/network/thrift_proxy/protocol.h"
+#include "extensions/filters/network/thrift_proxy/protocol_impl.h"
 
 #include "absl/types/optional.h"
 

source/extensions/filters/network/thrift_proxy/filter.cc

@@ -5,8 +5,8 @@
 #include "common/common/assert.h"
 
 #include "extensions/filters/network/thrift_proxy/buffer_helper.h"
-#include "extensions/filters/network/thrift_proxy/protocol.h"
-#include "extensions/filters/network/thrift_proxy/transport.h"
+#include "extensions/filters/network/thrift_proxy/protocol_impl.h"
+#include "extensions/filters/network/thrift_proxy/transport_impl.h"
 
 namespace Envoy {
 namespace Extensions {

source/extensions/filters/network/thrift_proxy/framed_transport_impl.cc

@@ -0,0 +1,38 @@
+#include "extensions/filters/network/thrift_proxy/framed_transport_impl.h"
+
+#include "envoy/common/exception.h"
+
+#include "extensions/filters/network/thrift_proxy/buffer_helper.h"
+#include "extensions/filters/network/thrift_proxy/transport_impl.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace NetworkFilters {
+namespace ThriftProxy {
+
+bool FramedTransportImpl::decodeFrameStart(Buffer::Instance& buffer) {
+  if (buffer.length() < 4) {
+    return false;
+  }
+
+  int32_t size = BufferHelper::peekI32(buffer);
+
+  if (size <= 0 || size > MaxFrameSize) {
+    throw EnvoyException(fmt::format("invalid thrift framed transport frame size {}", size));
+  }
+
+  onFrameStart(absl::optional<uint32_t>(static_cast<uint32_t>(size)));
+
+  buffer.drain(4);
+  return true;
+}
+
+bool FramedTransportImpl::decodeFrameEnd(Buffer::Instance&) {
+  onFrameComplete();
+  return true;
+}
+
+} // namespace ThriftProxy
+} // namespace NetworkFilters
+} // namespace Extensions
+} // namespace Envoy

source/extensions/filters/network/thrift_proxy/framed_transport_impl.h

@@ -0,0 +1,35 @@
+#pragma once
+
+#include <string>
+
+#include "envoy/buffer/buffer.h"
+
+#include "extensions/filters/network/thrift_proxy/transport_impl.h"
+
+#include "absl/types/optional.h"
+
+namespace Envoy {
+namespace Extensions {
+namespace NetworkFilters {
+namespace ThriftProxy {
+
+/**
+ * FramedTransportImpl implements the Thrift Framed transport.
+ * See https://github.com/apache/thrift/blob/master/doc/specs/thrift-rpc.md
+ */
+class FramedTransportImpl : public TransportImplBase {
+public:
+  FramedTransportImpl(TransportCallbacks& callbacks) : TransportImplBase(callbacks) {}
+
+  // Transport
+  const std::string& name() const override { return TransportNames::get().FRAMED; }
+  bool decodeFrameStart(Buffer::Instance& buffer) override;
+  bool decodeFrameEnd(Buffer::Instance& buffer) override;
+
+  static const int32_t MaxFrameSize = 0xFA0000;
+};
+
+} // namespace ThriftProxy
+} // namespace NetworkFilters
+} // namespace Extensions
+} // namespace Envoy

source/extensions/filters/network/thrift_proxy/protocol.h

@@ -1,18 +1,15 @@
 #pragma once
 
 #include <memory>
-#include <stack>
 #include <string>
-#include <utility>
-#include <vector>
 
 #include "envoy/buffer/buffer.h"
 #include "envoy/common/pure.h"
 
-#include "common/common/fmt.h"
-#include "common/common/macros.h"
 #include "common/singleton/const_singleton.h"
 
+#include "absl/strings/string_view.h"
+
 namespace Envoy {
 namespace Extensions {
 namespace NetworkFilters {
@@ -343,102 +340,6 @@ class Protocol {
 
 typedef std::unique_ptr<Protocol> ProtocolPtr;
 
-/*
- * ProtocolImplBase provides a base class for Protocol implementations.
- */
-class ProtocolImplBase : public virtual Protocol {
-public:
-  ProtocolImplBase(ProtocolCallbacks& callbacks) : callbacks_(callbacks) {}
-
-protected:
-  void onMessageStart(const absl::string_view name, MessageType msg_type, int32_t seq_id) const {
-    callbacks_.messageStart(name, msg_type, seq_id);
-  }
-  void onStructBegin(const absl::string_view name) const { callbacks_.structBegin(name); }
-  void onStructField(const absl::string_view name, FieldType field_type, int16_t field_id) const {
-    callbacks_.structField(name, field_type, field_id);
-  }
-  void onStructEnd() const { callbacks_.structEnd(); }
-  void onMessageComplete() const { callbacks_.messageComplete(); }
-
-  ProtocolCallbacks& callbacks_;
-};
-
-/**
- * AutoProtocolImpl attempts to distinguish between the Thrift binary (strict mode only) and
- * compact protocols and then delegates subsequent decoding operations to the appropriate Protocol
- * implementation.
- */
-class AutoProtocolImpl : public ProtocolImplBase {
-public:
-  AutoProtocolImpl(ProtocolCallbacks& callbacks)
-      : ProtocolImplBase(callbacks), name_(ProtocolNames::get().AUTO) {}
-
-  // Protocol
-  const std::string& name() const override { return name_; }
-  bool readMessageBegin(Buffer::Instance& buffer, std::string& name, MessageType& msg_type,
-                        int32_t& seq_id) override;
-  bool readMessageEnd(Buffer::Instance& buffer) override;
-  bool readStructBegin(Buffer::Instance& buffer, std::string& name) override {
-    return protocol_->readStructBegin(buffer, name);
-  }
-  bool readStructEnd(Buffer::Instance& buffer) override { return protocol_->readStructEnd(buffer); }
-  bool readFieldBegin(Buffer::Instance& buffer, std::string& name, FieldType& field_type,
-                      int16_t& field_id) override {
-    return protocol_->readFieldBegin(buffer, name, field_type, field_id);
-  }
-  bool readFieldEnd(Buffer::Instance& buffer) override { return protocol_->readFieldEnd(buffer); }
-  bool readMapBegin(Buffer::Instance& buffer, FieldType& key_type, FieldType& value_type,
-                    uint32_t& size) override {
-    return protocol_->readMapBegin(buffer, key_type, value_type, size);
-  }
-  bool readMapEnd(Buffer::Instance& buffer) override { return protocol_->readMapEnd(buffer); }
-  bool readListBegin(Buffer::Instance& buffer, FieldType& elem_type, uint32_t& size) override {
-    return protocol_->readListBegin(buffer, elem_type, size);
-  }
-  bool readListEnd(Buffer::Instance& buffer) override { return protocol_->readListEnd(buffer); }
-  bool readSetBegin(Buffer::Instance& buffer, FieldType& elem_type, uint32_t& size) override {
-    return protocol_->readSetBegin(buffer, elem_type, size);
-  }
-  bool readSetEnd(Buffer::Instance& buffer) override { return protocol_->readSetEnd(buffer); }
-  bool readBool(Buffer::Instance& buffer, bool& value) override {
-    return protocol_->readBool(buffer, value);
-  }
-  bool readByte(Buffer::Instance& buffer, uint8_t& value) override {
-    return protocol_->readByte(buffer, value);
-  }
-  bool readInt16(Buffer::Instance& buffer, int16_t& value) override {
-    return protocol_->readInt16(buffer, value);
-  }
-  bool readInt32(Buffer::Instance& buffer, int32_t& value) override {
-    return protocol_->readInt32(buffer, value);
-  }
-  bool readInt64(Buffer::Instance& buffer, int64_t& value) override {
-    return protocol_->readInt64(buffer, value);
-  }
-  bool readDouble(Buffer::Instance& buffer, double& value) override {
-    return protocol_->readDouble(buffer, value);
-  }
-  bool readString(Buffer::Instance& buffer, std::string& value) override {
-    return protocol_->readString(buffer, value);
-  }
-  bool readBinary(Buffer::Instance& buffer, std::string& value) override {
-    return protocol_->readBinary(buffer, value);
-  }
-
-  /*
-   * Explicitly set the protocol. Public to simplify testing.
-   */
-  void setProtocol(ProtocolPtr&& proto) {
-    protocol_ = std::move(proto);
-    name_ = fmt::format("{}({})", protocol_->name(), ProtocolNames::get().AUTO);
-  }
-
-private:
-  ProtocolPtr protocol_{};
-  std::string name_;
-};
-
 } // namespace ThriftProxy
 } // namespace NetworkFilters
 } // namespace Extensions

source/extensions/filters/network/thrift_proxy/protocol.cc → source/extensions/filters/network/thrift_proxy/protocol_impl.cc

@@ -1,4 +1,4 @@
-#include "extensions/filters/network/thrift_proxy/protocol.h"
+#include "extensions/filters/network/thrift_proxy/protocol_impl.h"
 
 #include <algorithm>
 
@@ -8,9 +8,9 @@
 #include "common/common/byte_order.h"
 #include "common/common/macros.h"
 
-#include "extensions/filters/network/thrift_proxy/binary_protocol.h"
+#include "extensions/filters/network/thrift_proxy/binary_protocol_impl.h"
 #include "extensions/filters/network/thrift_proxy/buffer_helper.h"
-#include "extensions/filters/network/thrift_proxy/compact_protocol.h"
+#include "extensions/filters/network/thrift_proxy/compact_protocol_impl.h"
 
 namespace Envoy {
 namespace Extensions {