lua: reset downstream_ssl_connection in StreamInfoWrapper when object…

… is marked dead by Lua GC (envoyproxy#14092) Fixes envoyproxy#14091 The problem and fix is similiar to envoyproxy#4312 Risk Level: Low Testing: regression test, manual testing Docs Changes: N/A Release Notes: N/A Signed-off-by: Marcin Falkowski <marcin.falkowski@allegro.pl>
andreyprezotto · Nov 24, 2020 · ce172b7 · ce172b7
1 parent 59945aa
commit ce172b7
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 2 deletions.
diff --git a/source/extensions/filters/http/lua/wrappers.h b/source/extensions/filters/http/lua/wrappers.h
@@ -222,7 +222,10 @@ class StreamInfoWrapper : public Filters::Common::Lua::BaseLuaObject<StreamInfoW
   DECLARE_LUA_FUNCTION(StreamInfoWrapper, luaDownstreamDirectRemoteAddress);
 
   // Envoy::Lua::BaseLuaObject
-  void onMarkDead() override { dynamic_metadata_wrapper_.reset(); }
+  void onMarkDead() override {
+    dynamic_metadata_wrapper_.reset();
+    downstream_ssl_connection_.reset();
+  }
 
   StreamInfo::StreamInfo& stream_info_;
   Filters::Common::Lua::LuaDeathRef<DynamicMetadataMapWrapper> dynamic_metadata_wrapper_;

diff --git a/test/extensions/filters/http/lua/lua_filter_test.cc b/test/extensions/filters/http/lua/lua_filter_test.cc
@@ -1884,7 +1884,7 @@ TEST_F(LuaHttpFilterTest, InspectStreamInfoDowstreamSslConnection) {
 
   Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}};
 
-  auto connection_info = std::make_shared<Ssl::MockConnectionInfo>();
+  const auto connection_info = std::make_shared<Ssl::MockConnectionInfo>();
   EXPECT_CALL(decoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(stream_info_));
   EXPECT_CALL(stream_info_, downstreamSslConnection()).WillRepeatedly(Return(connection_info));
 
@@ -1992,6 +1992,35 @@ TEST_F(LuaHttpFilterTest, InspectStreamInfoDowstreamSslConnectionOnPlainConnecti
   EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, true));
 }
 
+// Should survive from multiple streamInfo():downstreamSslConnection() calls.
+// This is a regression test for #14091.
+TEST_F(LuaHttpFilterTest, SurviveMultipleDownstreamSslConnectionCalls) {
+  const std::string SCRIPT{R"EOF(
+    function envoy_on_request(request_handle)
+      if request_handle:streamInfo():downstreamSslConnection() ~= nil then
+         request_handle:logTrace("downstreamSslConnection is present")
+      end
+    end
+  )EOF"};
+
+  setup(SCRIPT);
+
+  const auto connection_info = std::make_shared<Ssl::MockConnectionInfo>();
+  EXPECT_CALL(decoder_callbacks_, streamInfo()).WillRepeatedly(ReturnRef(stream_info_));
+  EXPECT_CALL(stream_info_, downstreamSslConnection()).WillRepeatedly(Return(connection_info));
+
+  for (uint64_t i = 0; i < 200; i++) {
+    EXPECT_CALL(*filter_,
+                scriptLog(spdlog::level::trace, StrEq("downstreamSslConnection is present")));
+
+    Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}};
+    EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, true));
+
+    filter_->onDestroy();
+    setupFilter();
+  }
+}
+
 TEST_F(LuaHttpFilterTest, ImportPublicKey) {
   const std::string SCRIPT{R"EOF(
     function string.fromhex(str)