diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index bdc1e5bb7e920..546cd8e3c43f0 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -83,6 +83,7 @@ Bug Fixes * hcm: remove deprecation for :ref:`xff_num_trusted_hops ` and forbid mixing ip detection extensions with old related knobs. * http: limit use of deferred resets in the http2 codec to server-side connections. Use of deferred reset for client connections can result in incorrect behavior and performance problems. * listener: fixed an issue on Windows where connections are not handled by all worker threads. +* lua: fix ``BodyBuffer`` setting a Lua string and printing Lua string containing hex characters. Previously, ``BodyBuffer`` setting a Lua string or printing strings with hex characters will be truncated. * xray: fix the AWS X-Ray tracer bug where span's error, fault and throttle information was not reported properly as per the `AWS X-Ray documentation `_. Before this fix, server error was reported under 'annotations' section of the segment data. Removed Config or Runtime diff --git a/source/extensions/filters/common/lua/lua.h b/source/extensions/filters/common/lua/lua.h index 54bdcabe33bc5..5271ce089f5c4 100644 --- a/source/extensions/filters/common/lua/lua.h +++ b/source/extensions/filters/common/lua/lua.h @@ -69,6 +69,25 @@ namespace Lua { lua_pushnumber(state, val); \ lua_settable(state, -3); +/** + * Get absl::string_view from Lua string. This checks if the argument at index is a string + * and build an absl::string_view from it. + * @param state the current Lua state. + * @param index the index of argument. + * @return absl::string_view of Lua string with proper string length. + **/ +inline absl::string_view getStringViewFromLuaString(lua_State* state, int index) { + size_t input_size = 0; + // When the argument at index in Lua state is not a string, for example, giving a table to + // logTrace (which uses this function under the hood), Lua script exits with an error like the + // following: "[string \"...\"]:3: bad argument #1 to 'logTrace' (string expected, got table)". + // However,`luaL_checklstring` accepts a number as its argument and implicitly converts it to a + // string, since Lua provides automatic conversion between string and number values at run time + // (https://www.lua.org/manual/5.1/manual.html#2.2.1). + const char* input = luaL_checklstring(state, index, &input_size); + return absl::string_view(input, input_size); +} + /** * Calculate the maximum space needed to be aligned. */ diff --git a/source/extensions/filters/common/lua/wrappers.cc b/source/extensions/filters/common/lua/wrappers.cc index cb148e18bcfe2..bfb2a1c473259 100644 --- a/source/extensions/filters/common/lua/wrappers.cc +++ b/source/extensions/filters/common/lua/wrappers.cc @@ -42,7 +42,6 @@ int64_t timestampInSeconds(const absl::optional& system_time) { .count() : 0; } - } // namespace int BufferWrapper::luaLength(lua_State* state) { @@ -67,7 +66,7 @@ int BufferWrapper::luaGetBytes(lua_State* state) { int BufferWrapper::luaSetBytes(lua_State* state) { data_.drain(data_.length()); - absl::string_view bytes = luaL_checkstring(state, 2); + absl::string_view bytes = getStringViewFromLuaString(state, 2); data_.add(bytes); lua_pushnumber(state, data_.length()); return 1; diff --git a/source/extensions/filters/http/lua/lua_filter.cc b/source/extensions/filters/http/lua/lua_filter.cc index 5c78826978676..095f9e7bc7738 100644 --- a/source/extensions/filters/http/lua/lua_filter.cc +++ b/source/extensions/filters/http/lua/lua_filter.cc @@ -552,37 +552,37 @@ int StreamHandleWrapper::luaConnection(lua_State* state) { } int StreamHandleWrapper::luaLogTrace(lua_State* state) { - const char* message = luaL_checkstring(state, 2); + absl::string_view message = Filters::Common::Lua::getStringViewFromLuaString(state, 2); filter_.scriptLog(spdlog::level::trace, message); return 0; } int StreamHandleWrapper::luaLogDebug(lua_State* state) { - const char* message = luaL_checkstring(state, 2); + absl::string_view message = Filters::Common::Lua::getStringViewFromLuaString(state, 2); filter_.scriptLog(spdlog::level::debug, message); return 0; } int StreamHandleWrapper::luaLogInfo(lua_State* state) { - const char* message = luaL_checkstring(state, 2); + absl::string_view message = Filters::Common::Lua::getStringViewFromLuaString(state, 2); filter_.scriptLog(spdlog::level::info, message); return 0; } int StreamHandleWrapper::luaLogWarn(lua_State* state) { - const char* message = luaL_checkstring(state, 2); + absl::string_view message = Filters::Common::Lua::getStringViewFromLuaString(state, 2); filter_.scriptLog(spdlog::level::warn, message); return 0; } int StreamHandleWrapper::luaLogErr(lua_State* state) { - const char* message = luaL_checkstring(state, 2); + absl::string_view message = Filters::Common::Lua::getStringViewFromLuaString(state, 2); filter_.scriptLog(spdlog::level::err, message); return 0; } int StreamHandleWrapper::luaLogCritical(lua_State* state) { - const char* message = luaL_checkstring(state, 2); + absl::string_view message = Filters::Common::Lua::getStringViewFromLuaString(state, 2); filter_.scriptLog(spdlog::level::critical, message); return 0; } @@ -649,9 +649,8 @@ int StreamHandleWrapper::luaImportPublicKey(lua_State* state) { } int StreamHandleWrapper::luaBase64Escape(lua_State* state) { - size_t input_size; - const char* input = luaL_checklstring(state, 2, &input_size); - auto output = absl::Base64Escape(absl::string_view(input, input_size)); + absl::string_view input = Filters::Common::Lua::getStringViewFromLuaString(state, 2); + auto output = absl::Base64Escape(input); lua_pushlstring(state, output.data(), output.length()); return 1; @@ -783,7 +782,7 @@ void Filter::scriptError(const Filters::Common::Lua::LuaException& e) { response_stream_wrapper_.reset(); } -void Filter::scriptLog(spdlog::level::level_enum level, const char* message) { +void Filter::scriptLog(spdlog::level::level_enum level, absl::string_view message) { switch (level) { case spdlog::level::trace: ENVOY_LOG(trace, "script log: {}", message); diff --git a/source/extensions/filters/http/lua/lua_filter.h b/source/extensions/filters/http/lua/lua_filter.h index e779d9fdf4245..17c9338627965 100644 --- a/source/extensions/filters/http/lua/lua_filter.h +++ b/source/extensions/filters/http/lua/lua_filter.h @@ -441,7 +441,7 @@ class Filter : public Http::StreamFilter, Logger::Loggable { Upstream::ClusterManager& clusterManager() { return config_->cluster_manager_; } void scriptError(const Filters::Common::Lua::LuaException& e); - virtual void scriptLog(spdlog::level::level_enum level, const char* message); + virtual void scriptLog(spdlog::level::level_enum level, absl::string_view message); // Http::StreamFilterBase void onDestroy() override; diff --git a/test/extensions/filters/http/lua/lua_filter_test.cc b/test/extensions/filters/http/lua/lua_filter_test.cc index a7ae58a4f2003..8c0bd5bdc8687 100644 --- a/test/extensions/filters/http/lua/lua_filter_test.cc +++ b/test/extensions/filters/http/lua/lua_filter_test.cc @@ -41,7 +41,7 @@ class TestFilter : public Filter { public: using Filter::Filter; - MOCK_METHOD(void, scriptLog, (spdlog::level::level_enum level, const char* message)); + MOCK_METHOD(void, scriptLog, (spdlog::level::level_enum level, absl::string_view message)); }; class LuaHttpFilterTest : public testing::Test { @@ -818,6 +818,7 @@ TEST_F(LuaHttpFilterTest, HttpCall) { {":method", "POST"}, {":path", "/"}, {":authority", "foo"}, + {"set-cookie", "flavor=chocolate; Path=/"}, {"set-cookie", "variant=chewy; Path=/"}, {"content-length", "11"}}; @@ -841,7 +842,7 @@ TEST_F(LuaHttpFilterTest, HttpCall) { response_message->body().add(response, 8); EXPECT_CALL(*filter_, scriptLog(spdlog::level::trace, StrEq(":status 200"))); EXPECT_CALL(*filter_, scriptLog(spdlog::level::trace, StrEq("8"))); - EXPECT_CALL(*filter_, scriptLog(spdlog::level::trace, StrEq("resp"))); + EXPECT_CALL(*filter_, scriptLog(spdlog::level::trace, StrEq(std::string("resp\0nse", 8)))); EXPECT_CALL(*filter_, scriptLog(spdlog::level::trace, StrEq("0"))); EXPECT_CALL(*filter_, scriptLog(spdlog::level::trace, StrEq("nse"))); EXPECT_CALL(decoder_callbacks_, continueDecoding()); @@ -2392,6 +2393,77 @@ TEST_F(LuaHttpFilterTest, LuaFilterSetResponseBufferChunked) { EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_body, true)); } +// BodyBuffer should not truncated when bodyBuffer set hex character +TEST_F(LuaHttpFilterTest, LuaBodyBufferSetBytesWithHex) { + const std::string SCRIPT{R"EOF( + function envoy_on_response(response_handle) + local bodyBuffer = response_handle:body() + bodyBuffer:setBytes("\x471111") + local body_str = bodyBuffer:getBytes(0, bodyBuffer:length()) + response_handle:logTrace(body_str) + end + )EOF"}; + + InSequence s; + setup(SCRIPT); + + Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, true)); + + Http::TestResponseHeaderMapImpl response_headers{{":status", "200"}}; + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->encodeHeaders(response_headers, false)); + + Buffer::OwnedImpl response_body(""); + EXPECT_CALL(*filter_, scriptLog(spdlog::level::trace, StrEq("G1111"))); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_body, true)); + EXPECT_EQ(5, encoder_callbacks_.buffer_->length()); +} + +// BodyBuffer should not truncated when bodyBuffer set zero +TEST_F(LuaHttpFilterTest, LuaBodyBufferSetBytesWithZero) { + const std::string SCRIPT{R"EOF( + function envoy_on_response(response_handle) + local bodyBuffer = response_handle:body() + bodyBuffer:setBytes("\0") + end + )EOF"}; + + InSequence s; + setup(SCRIPT); + + Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, true)); + + Http::TestResponseHeaderMapImpl response_headers{{":status", "200"}}; + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->encodeHeaders(response_headers, false)); + + Buffer::OwnedImpl response_body("1111"); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_body, true)); + EXPECT_EQ(1, encoder_callbacks_.buffer_->length()); +} + +// Script logging a table instead of the expected string. +TEST_F(LuaHttpFilterTest, LogTableInsteadOfString) { + const std::string LOG_TABLE{R"EOF( + function envoy_on_request(request_handle) + request_handle:logTrace({}) + end + )EOF"}; + + InSequence s; + setup(LOG_TABLE); + + Http::TestRequestHeaderMapImpl request_headers{{":path", "/"}}; + EXPECT_CALL( + *filter_, + scriptLog( + spdlog::level::err, + StrEq("[string \"...\"]:3: bad argument #1 to 'logTrace' (string expected, got table)"))); + EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, true)); +} + } // namespace } // namespace Lua } // namespace HttpFilters