lua: reset downstream_ssl_connection in StreamInfoWrapper when object…

… is marked dead by Lua GC (#14092) (#14449) Co-authored-by: Marcin Falkowski <[email protected]>
envoyproxy · Dec 22, 2020 · db0ae3d · db0ae3d
1 parent 9c117fb
commit db0ae3d
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 2 deletions.
diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
@@ -13,6 +13,8 @@ Bug Fixes
 ---------
 *Changes expected to improve the state of the world and are unlikely to have negative effects*
 
+* lua: fixed crash when Lua script contains streamInfo():downstreamSslConnection().
+
 Removed Config or Runtime
 -------------------------
 *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

diff --git a/source/extensions/filters/http/lua/wrappers.h b/source/extensions/filters/http/lua/wrappers.h
@@ -207,7 +207,10 @@ class StreamInfoWrapper : public Filters::Common::Lua::BaseLuaObject<StreamInfoW
   DECLARE_LUA_FUNCTION(StreamInfoWrapper, luaDownstreamSslConnection);
 
   // 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
@@ -1881,7 +1881,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));
 
@@ -1989,6 +1989,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)