From 35c1bb7e68025d5c518f3963ea4a5979a33a6852 Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 6 Sep 2021 11:18:03 +0800 Subject: [PATCH 01/19] fix luaSetBytes as char Signed-off-by: chaojiang Signed-off-by: Patrick --- source/extensions/filters/common/lua/wrappers.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/source/extensions/filters/common/lua/wrappers.cc b/source/extensions/filters/common/lua/wrappers.cc index cb148e18bcfe2..6a70f4f2a5e25 100644 --- a/source/extensions/filters/common/lua/wrappers.cc +++ b/source/extensions/filters/common/lua/wrappers.cc @@ -67,7 +67,9 @@ int BufferWrapper::luaGetBytes(lua_State* state) { int BufferWrapper::luaSetBytes(lua_State* state) { data_.drain(data_.length()); - absl::string_view bytes = luaL_checkstring(state, 2); + size_t input_size; + const char* input = luaL_checklstring(state, 2, &input_size); + absl::string_view bytes = absl::string_view(input, input_size); data_.add(bytes); lua_pushnumber(state, data_.length()); return 1; From ca56457835f41abeb390f020d6775fb5bf92bf86 Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 6 Sep 2021 12:21:31 +0800 Subject: [PATCH 02/19] fix lua script log be truncated Signed-off-by: Patrick --- .../extensions/filters/http/lua/lua_filter.cc | 26 ++++++++++++++----- .../extensions/filters/http/lua/lua_filter.h | 2 +- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/source/extensions/filters/http/lua/lua_filter.cc b/source/extensions/filters/http/lua/lua_filter.cc index 5c78826978676..2ba8925a37917 100644 --- a/source/extensions/filters/http/lua/lua_filter.cc +++ b/source/extensions/filters/http/lua/lua_filter.cc @@ -552,37 +552,49 @@ int StreamHandleWrapper::luaConnection(lua_State* state) { } int StreamHandleWrapper::luaLogTrace(lua_State* state) { - const char* message = luaL_checkstring(state, 2); + size_t size; + const char* bytes = luaL_checklstring(state, 2, &size); + absl::string_view message = absl::string_view(bytes, size); filter_.scriptLog(spdlog::level::trace, message); return 0; } int StreamHandleWrapper::luaLogDebug(lua_State* state) { - const char* message = luaL_checkstring(state, 2); + size_t size; + const char* bytes = luaL_checklstring(state, 2, &size); + absl::string_view message = absl::string_view(bytes, size); filter_.scriptLog(spdlog::level::debug, message); return 0; } int StreamHandleWrapper::luaLogInfo(lua_State* state) { - const char* message = luaL_checkstring(state, 2); + size_t size; + const char* bytes = luaL_checklstring(state, 2, &size); + absl::string_view message = absl::string_view(bytes, size); filter_.scriptLog(spdlog::level::info, message); return 0; } int StreamHandleWrapper::luaLogWarn(lua_State* state) { - const char* message = luaL_checkstring(state, 2); + size_t size; + const char* bytes = luaL_checklstring(state, 2, &size); + absl::string_view message = absl::string_view(bytes, size); filter_.scriptLog(spdlog::level::warn, message); return 0; } int StreamHandleWrapper::luaLogErr(lua_State* state) { - const char* message = luaL_checkstring(state, 2); + size_t size; + const char* bytes = luaL_checklstring(state, 2, &size); + absl::string_view message = absl::string_view(bytes, size); filter_.scriptLog(spdlog::level::err, message); return 0; } int StreamHandleWrapper::luaLogCritical(lua_State* state) { - const char* message = luaL_checkstring(state, 2); + size_t size; + const char* bytes = luaL_checklstring(state, 2, &size); + absl::string_view message = absl::string_view(bytes, size); filter_.scriptLog(spdlog::level::critical, message); return 0; } @@ -783,7 +795,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; From 3eccad4faf2aa010841215ad843cc4ed8f2430e7 Mon Sep 17 00:00:00 2001 From: Patrick Date: Tue, 7 Sep 2021 12:44:51 +0800 Subject: [PATCH 03/19] add a checkLuaStringWithLength function Signed-off-by: Patrick --- source/extensions/filters/common/lua/lua.h | 9 ++++++ .../extensions/filters/common/lua/wrappers.cc | 5 +--- .../extensions/filters/common/lua/wrappers.h | 1 + .../extensions/filters/http/lua/lua_filter.cc | 29 +++++-------------- .../filters/http/lua/lua_filter_test.cc | 27 ++++++++++++++++- 5 files changed, 45 insertions(+), 26 deletions(-) diff --git a/source/extensions/filters/common/lua/lua.h b/source/extensions/filters/common/lua/lua.h index 54bdcabe33bc5..8291e9e85a5f0 100644 --- a/source/extensions/filters/common/lua/lua.h +++ b/source/extensions/filters/common/lua/lua.h @@ -69,6 +69,15 @@ namespace Lua { lua_pushnumber(state, val); \ lua_settable(state, -3); +/** + * Check string from lua with length + */ +inline absl::string_view checkLuaStringWithLength(lua_State* state, int arg) { + size_t input_size; + const char* input = luaL_checklstring(state, arg, &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 6a70f4f2a5e25..cc3e3ce1b49f9 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,9 +66,7 @@ int BufferWrapper::luaGetBytes(lua_State* state) { int BufferWrapper::luaSetBytes(lua_State* state) { data_.drain(data_.length()); - size_t input_size; - const char* input = luaL_checklstring(state, 2, &input_size); - absl::string_view bytes = absl::string_view(input, input_size); + absl::string_view bytes = checkLuaStringWithLength(state, 2); data_.add(bytes); lua_pushnumber(state, data_.length()); return 1; diff --git a/source/extensions/filters/common/lua/wrappers.h b/source/extensions/filters/common/lua/wrappers.h index 344467d39ff2c..46bbc2ff22a52 100644 --- a/source/extensions/filters/common/lua/wrappers.h +++ b/source/extensions/filters/common/lua/wrappers.h @@ -11,6 +11,7 @@ namespace Filters { namespace Common { namespace Lua { + /** * A wrapper for a buffer. */ diff --git a/source/extensions/filters/http/lua/lua_filter.cc b/source/extensions/filters/http/lua/lua_filter.cc index 2ba8925a37917..f8b4bb818578b 100644 --- a/source/extensions/filters/http/lua/lua_filter.cc +++ b/source/extensions/filters/http/lua/lua_filter.cc @@ -552,49 +552,37 @@ int StreamHandleWrapper::luaConnection(lua_State* state) { } int StreamHandleWrapper::luaLogTrace(lua_State* state) { - size_t size; - const char* bytes = luaL_checklstring(state, 2, &size); - absl::string_view message = absl::string_view(bytes, size); + absl::string_view message = Filters::Common::Lua::checkLuaStringWithLength(state, 2); filter_.scriptLog(spdlog::level::trace, message); return 0; } int StreamHandleWrapper::luaLogDebug(lua_State* state) { - size_t size; - const char* bytes = luaL_checklstring(state, 2, &size); - absl::string_view message = absl::string_view(bytes, size); + absl::string_view message = Filters::Common::Lua::checkLuaStringWithLength(state, 2); filter_.scriptLog(spdlog::level::debug, message); return 0; } int StreamHandleWrapper::luaLogInfo(lua_State* state) { - size_t size; - const char* bytes = luaL_checklstring(state, 2, &size); - absl::string_view message = absl::string_view(bytes, size); + absl::string_view message = Filters::Common::Lua::checkLuaStringWithLength(state, 2); filter_.scriptLog(spdlog::level::info, message); return 0; } int StreamHandleWrapper::luaLogWarn(lua_State* state) { - size_t size; - const char* bytes = luaL_checklstring(state, 2, &size); - absl::string_view message = absl::string_view(bytes, size); + absl::string_view message = Filters::Common::Lua::checkLuaStringWithLength(state, 2); filter_.scriptLog(spdlog::level::warn, message); return 0; } int StreamHandleWrapper::luaLogErr(lua_State* state) { - size_t size; - const char* bytes = luaL_checklstring(state, 2, &size); - absl::string_view message = absl::string_view(bytes, size); + absl::string_view message = Filters::Common::Lua::checkLuaStringWithLength(state, 2); filter_.scriptLog(spdlog::level::err, message); return 0; } int StreamHandleWrapper::luaLogCritical(lua_State* state) { - size_t size; - const char* bytes = luaL_checklstring(state, 2, &size); - absl::string_view message = absl::string_view(bytes, size); + absl::string_view message = Filters::Common::Lua::checkLuaStringWithLength(state, 2); filter_.scriptLog(spdlog::level::critical, message); return 0; } @@ -661,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::checkLuaStringWithLength(state, 2); + auto output = absl::Base64Escape(input); lua_pushlstring(state, output.data(), output.length()); return 1; diff --git a/test/extensions/filters/http/lua/lua_filter_test.cc b/test/extensions/filters/http/lua/lua_filter_test.cc index 24c575c1daea6..12d04722c350e 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 { @@ -2392,6 +2392,31 @@ TEST_F(LuaHttpFilterTest, LuaFilterSetResponseBufferChunked) { EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_body, true)); } +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()); +} + } // namespace } // namespace Lua } // namespace HttpFilters From 559ba90c9f9c9ead90375611ec95162b8645eeac Mon Sep 17 00:00:00 2001 From: Patrick Date: Tue, 7 Sep 2021 19:13:28 +0800 Subject: [PATCH 04/19] format code and modify comment Signed-off-by: Patrick --- source/extensions/filters/common/lua/lua.h | 2 +- source/extensions/filters/common/lua/wrappers.h | 1 - test/extensions/filters/http/lua/lua_filter_test.cc | 3 ++- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/source/extensions/filters/common/lua/lua.h b/source/extensions/filters/common/lua/lua.h index 8291e9e85a5f0..eadde22f8d356 100644 --- a/source/extensions/filters/common/lua/lua.h +++ b/source/extensions/filters/common/lua/lua.h @@ -70,7 +70,7 @@ namespace Lua { lua_settable(state, -3); /** - * Check string from lua with length + * Check string from lua with length. */ inline absl::string_view checkLuaStringWithLength(lua_State* state, int arg) { size_t input_size; diff --git a/source/extensions/filters/common/lua/wrappers.h b/source/extensions/filters/common/lua/wrappers.h index 46bbc2ff22a52..344467d39ff2c 100644 --- a/source/extensions/filters/common/lua/wrappers.h +++ b/source/extensions/filters/common/lua/wrappers.h @@ -11,7 +11,6 @@ namespace Filters { namespace Common { namespace Lua { - /** * A wrapper for a buffer. */ diff --git a/test/extensions/filters/http/lua/lua_filter_test.cc b/test/extensions/filters/http/lua/lua_filter_test.cc index 12d04722c350e..2ab696f32717b 100644 --- a/test/extensions/filters/http/lua/lua_filter_test.cc +++ b/test/extensions/filters/http/lua/lua_filter_test.cc @@ -2409,7 +2409,8 @@ TEST_F(LuaHttpFilterTest, LuaBodyBufferSetBytesWithHex) { 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)); + EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + filter_->encodeHeaders(response_headers, false)); Buffer::OwnedImpl response_body(""); EXPECT_CALL(*filter_, scriptLog(spdlog::level::trace, StrEq("G1111"))); From 468c0c568c6de600630680f1fa3608a4aa7bbf4e Mon Sep 17 00:00:00 2001 From: Patrick Date: Tue, 7 Sep 2021 19:31:29 +0800 Subject: [PATCH 05/19] add test setBytes with empty string Signed-off-by: Patrick --- .../filters/http/lua/lua_filter_test.cc | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/extensions/filters/http/lua/lua_filter_test.cc b/test/extensions/filters/http/lua/lua_filter_test.cc index 2ab696f32717b..915d11da13c79 100644 --- a/test/extensions/filters/http/lua/lua_filter_test.cc +++ b/test/extensions/filters/http/lua/lua_filter_test.cc @@ -2418,6 +2418,31 @@ TEST_F(LuaHttpFilterTest, LuaBodyBufferSetBytesWithHex) { EXPECT_EQ(5, encoder_callbacks_.buffer_->length()); } +TEST_F(LuaHttpFilterTest, LuaBodyBufferSetBytesWithEmpty) { + const std::string SCRIPT{R"EOF( + function envoy_on_response(response_handle) + local bodyBuffer = response_handle:body() + bodyBuffer:setBytes("") + 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("1111"); + EXPECT_CALL(*filter_, scriptLog(spdlog::level::trace, StrEq(""))); + EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_body, true)); + EXPECT_EQ(0, encoder_callbacks_.buffer_->length()); +} } // namespace } // namespace Lua } // namespace HttpFilters From 50dcaca1a539bdb73073dc4e989becdf189708c0 Mon Sep 17 00:00:00 2001 From: Patrick Date: Wed, 8 Sep 2021 11:31:53 +0800 Subject: [PATCH 06/19] fix httpCall test failed Signed-off-by: Patrick --- test/extensions/filters/http/lua/lua_filter_test.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/extensions/filters/http/lua/lua_filter_test.cc b/test/extensions/filters/http/lua/lua_filter_test.cc index 915d11da13c79..118c62e41628a 100644 --- a/test/extensions/filters/http/lua/lua_filter_test.cc +++ b/test/extensions/filters/http/lua/lua_filter_test.cc @@ -796,7 +796,7 @@ TEST_F(LuaHttpFilterTest, HttpCall) { request_handle:logTrace(key .. " " .. value) end request_handle:logTrace(string.len(body)) - request_handle:logTrace(body) + request_handle:logTrace(string.sub(body, 1, 4) .. string.byte(body, 5) .. string.sub(body, 6, 8)) request_handle:logTrace(string.byte(body, 5)) request_handle:logTrace(string.sub(body, 6, 8)) end @@ -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("resp0nse"))); EXPECT_CALL(*filter_, scriptLog(spdlog::level::trace, StrEq("0"))); EXPECT_CALL(*filter_, scriptLog(spdlog::level::trace, StrEq("nse"))); EXPECT_CALL(decoder_callbacks_, continueDecoding()); From 141afacdfc2d9f92f1a279bda30821ebc1301044 Mon Sep 17 00:00:00 2001 From: Patrick Date: Wed, 8 Sep 2021 11:36:59 +0800 Subject: [PATCH 07/19] add unit test for zero byte Signed-off-by: Patrick --- test/extensions/filters/http/lua/lua_filter_test.cc | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/extensions/filters/http/lua/lua_filter_test.cc b/test/extensions/filters/http/lua/lua_filter_test.cc index 118c62e41628a..cd6fccefe0d96 100644 --- a/test/extensions/filters/http/lua/lua_filter_test.cc +++ b/test/extensions/filters/http/lua/lua_filter_test.cc @@ -2419,13 +2419,11 @@ TEST_F(LuaHttpFilterTest, LuaBodyBufferSetBytesWithHex) { EXPECT_EQ(5, encoder_callbacks_.buffer_->length()); } -TEST_F(LuaHttpFilterTest, LuaBodyBufferSetBytesWithEmpty) { +TEST_F(LuaHttpFilterTest, LuaBodyBufferSetBytesWithZero) { const std::string SCRIPT{R"EOF( function envoy_on_response(response_handle) local bodyBuffer = response_handle:body() - bodyBuffer:setBytes("") - local body_str = bodyBuffer:getBytes(0, bodyBuffer:length()) - response_handle:logTrace(body_str) + bodyBuffer:setBytes("\0") end )EOF"}; @@ -2440,9 +2438,8 @@ TEST_F(LuaHttpFilterTest, LuaBodyBufferSetBytesWithEmpty) { filter_->encodeHeaders(response_headers, false)); Buffer::OwnedImpl response_body("1111"); - EXPECT_CALL(*filter_, scriptLog(spdlog::level::trace, StrEq(""))); EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_body, true)); - EXPECT_EQ(0, encoder_callbacks_.buffer_->length()); + EXPECT_EQ(1, encoder_callbacks_.buffer_->length()); } } // namespace } // namespace Lua From 7785a54b69b5f927c5275eed357a387726a95fd5 Mon Sep 17 00:00:00 2001 From: Patrick Date: Wed, 8 Sep 2021 20:42:18 +0800 Subject: [PATCH 08/19] modify comment and function name Signed-off-by: Patrick --- source/extensions/filters/common/lua/lua.h | 10 +++++++--- source/extensions/filters/common/lua/wrappers.cc | 2 +- source/extensions/filters/http/lua/lua_filter.cc | 14 +++++++------- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/source/extensions/filters/common/lua/lua.h b/source/extensions/filters/common/lua/lua.h index eadde22f8d356..c84022bd7a182 100644 --- a/source/extensions/filters/common/lua/lua.h +++ b/source/extensions/filters/common/lua/lua.h @@ -70,9 +70,13 @@ namespace Lua { lua_settable(state, -3); /** - * Check string from lua with length. - */ -inline absl::string_view checkLuaStringWithLength(lua_State* state, int arg) { + * 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 arg) { size_t input_size; const char* input = luaL_checklstring(state, arg, &input_size); return absl::string_view(input, input_size); diff --git a/source/extensions/filters/common/lua/wrappers.cc b/source/extensions/filters/common/lua/wrappers.cc index cc3e3ce1b49f9..bfb2a1c473259 100644 --- a/source/extensions/filters/common/lua/wrappers.cc +++ b/source/extensions/filters/common/lua/wrappers.cc @@ -66,7 +66,7 @@ int BufferWrapper::luaGetBytes(lua_State* state) { int BufferWrapper::luaSetBytes(lua_State* state) { data_.drain(data_.length()); - absl::string_view bytes = checkLuaStringWithLength(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 f8b4bb818578b..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) { - absl::string_view message = Filters::Common::Lua::checkLuaStringWithLength(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) { - absl::string_view message = Filters::Common::Lua::checkLuaStringWithLength(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) { - absl::string_view message = Filters::Common::Lua::checkLuaStringWithLength(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) { - absl::string_view message = Filters::Common::Lua::checkLuaStringWithLength(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) { - absl::string_view message = Filters::Common::Lua::checkLuaStringWithLength(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) { - absl::string_view message = Filters::Common::Lua::checkLuaStringWithLength(state, 2); + absl::string_view message = Filters::Common::Lua::getStringViewFromLuaString(state, 2); filter_.scriptLog(spdlog::level::critical, message); return 0; } @@ -649,7 +649,7 @@ int StreamHandleWrapper::luaImportPublicKey(lua_State* state) { } int StreamHandleWrapper::luaBase64Escape(lua_State* state) { - absl::string_view input = Filters::Common::Lua::checkLuaStringWithLength(state, 2); + absl::string_view input = Filters::Common::Lua::getStringViewFromLuaString(state, 2); auto output = absl::Base64Escape(input); lua_pushlstring(state, output.data(), output.length()); From 1362378b0b61306145ba3c69f99b806d93b3e0c4 Mon Sep 17 00:00:00 2001 From: Patrick Date: Thu, 9 Sep 2021 10:45:43 +0800 Subject: [PATCH 09/19] init input_size Signed-off-by: Patrick --- source/extensions/filters/common/lua/lua.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/filters/common/lua/lua.h b/source/extensions/filters/common/lua/lua.h index c84022bd7a182..225565f279cc6 100644 --- a/source/extensions/filters/common/lua/lua.h +++ b/source/extensions/filters/common/lua/lua.h @@ -77,7 +77,7 @@ namespace Lua { * @return absl::string_view of Lua string with proper string length. **/ inline absl::string_view getStringViewFromLuaString(lua_State* state, int arg) { - size_t input_size; + size_t input_size = 0; const char* input = luaL_checklstring(state, arg, &input_size); return absl::string_view(input, input_size); } From a492a3106cd3a8ba7decb7d116a560521c7f7499 Mon Sep 17 00:00:00 2001 From: Patrick Date: Fri, 10 Sep 2021 11:32:06 +0800 Subject: [PATCH 10/19] add comment in test Signed-off-by: Patrick --- test/extensions/filters/http/lua/lua_filter_test.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/extensions/filters/http/lua/lua_filter_test.cc b/test/extensions/filters/http/lua/lua_filter_test.cc index cd6fccefe0d96..94c7b8ed73394 100644 --- a/test/extensions/filters/http/lua/lua_filter_test.cc +++ b/test/extensions/filters/http/lua/lua_filter_test.cc @@ -2393,6 +2393,7 @@ 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) @@ -2419,6 +2420,7 @@ TEST_F(LuaHttpFilterTest, LuaBodyBufferSetBytesWithHex) { 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) From eb34c1c4222040dd7472ad64dee553284975500b Mon Sep 17 00:00:00 2001 From: Patrick Date: Fri, 10 Sep 2021 13:42:44 +0800 Subject: [PATCH 11/19] fix Signed-off-by: Patrick --- test/extensions/filters/http/lua/lua_filter_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/extensions/filters/http/lua/lua_filter_test.cc b/test/extensions/filters/http/lua/lua_filter_test.cc index 94c7b8ed73394..9f65a1cf97aee 100644 --- a/test/extensions/filters/http/lua/lua_filter_test.cc +++ b/test/extensions/filters/http/lua/lua_filter_test.cc @@ -796,7 +796,7 @@ TEST_F(LuaHttpFilterTest, HttpCall) { request_handle:logTrace(key .. " " .. value) end request_handle:logTrace(string.len(body)) - request_handle:logTrace(string.sub(body, 1, 4) .. string.byte(body, 5) .. string.sub(body, 6, 8)) + request_handle:logTrace(body) request_handle:logTrace(string.byte(body, 5)) request_handle:logTrace(string.sub(body, 6, 8)) end @@ -842,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("resp0nse"))); + 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()); From c6e70f61b9147ef6b656bb3398db84a3cb4f8c6d Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 13 Sep 2021 16:55:16 +0800 Subject: [PATCH 12/19] modify comment and add a test for unexpected type Signed-off-by: Patrick --- source/extensions/filters/common/lua/lua.h | 9 ++++++-- .../filters/http/lua/lua_filter_test.cc | 21 +++++++++++++++++-- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/source/extensions/filters/common/lua/lua.h b/source/extensions/filters/common/lua/lua.h index 225565f279cc6..905550ae1b98c 100644 --- a/source/extensions/filters/common/lua/lua.h +++ b/source/extensions/filters/common/lua/lua.h @@ -76,9 +76,14 @@ namespace Lua { * @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 arg) { +inline absl::string_view getStringViewFromLuaString(lua_State* state, int index) { size_t input_size = 0; - const char* input = luaL_checklstring(state, arg, &input_size); + // 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 + const char* input = luaL_checklstring(state, index, &input_size); return absl::string_view(input, input_size); } diff --git a/test/extensions/filters/http/lua/lua_filter_test.cc b/test/extensions/filters/http/lua/lua_filter_test.cc index 9f65a1cf97aee..5a6928df68caa 100644 --- a/test/extensions/filters/http/lua/lua_filter_test.cc +++ b/test/extensions/filters/http/lua/lua_filter_test.cc @@ -2393,7 +2393,7 @@ TEST_F(LuaHttpFilterTest, LuaFilterSetResponseBufferChunked) { EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->encodeData(response_body, true)); } -// bodyBuffer should not truncated when bodyBuffer set hex character +// 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) @@ -2420,7 +2420,7 @@ TEST_F(LuaHttpFilterTest, LuaBodyBufferSetBytesWithHex) { EXPECT_EQ(5, encoder_callbacks_.buffer_->length()); } -// bodyBuffer should not truncated when bodyBuffer set zero +// BodyBuffer should not truncated when bodyBuffer set zero TEST_F(LuaHttpFilterTest, LuaBodyBufferSetBytesWithZero) { const std::string SCRIPT{R"EOF( function envoy_on_response(response_handle) @@ -2443,6 +2443,23 @@ TEST_F(LuaHttpFilterTest, LuaBodyBufferSetBytesWithZero) { 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 From 657db60da7fbe4bc43ae36f58108e9a246015a14 Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 13 Sep 2021 19:22:44 +0800 Subject: [PATCH 13/19] format Signed-off-by: Patrick --- test/extensions/filters/http/lua/lua_filter_test.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/extensions/filters/http/lua/lua_filter_test.cc b/test/extensions/filters/http/lua/lua_filter_test.cc index 5a6928df68caa..1acd0a6b32b0d 100644 --- a/test/extensions/filters/http/lua/lua_filter_test.cc +++ b/test/extensions/filters/http/lua/lua_filter_test.cc @@ -2456,7 +2456,11 @@ TEST_F(LuaHttpFilterTest, LogTableInsteadOfString) { 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_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)); } From 90cdf26e49bdb78fa07c5a109c4ad1c85935ba5b Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 13 Sep 2021 20:20:27 +0800 Subject: [PATCH 14/19] fix spell Signed-off-by: Patrick --- source/extensions/filters/common/lua/lua.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/extensions/filters/common/lua/lua.h b/source/extensions/filters/common/lua/lua.h index 905550ae1b98c..60168fb7deb84 100644 --- a/source/extensions/filters/common/lua/lua.h +++ b/source/extensions/filters/common/lua/lua.h @@ -81,7 +81,7 @@ inline absl::string_view getStringViewFromLuaString(lua_State* state, int index) // 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 + // 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 const char* input = luaL_checklstring(state, index, &input_size); return absl::string_view(input, input_size); From badd5094f34d8d840057fbf7bb33c42642a43eb4 Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 13 Sep 2021 20:31:07 +0800 Subject: [PATCH 15/19] add doc Signed-off-by: Patrick --- docs/root/version_history/current.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index fe14c7af5c5e5..d135d2d846710 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -43,6 +43,7 @@ Bug Fixes * aws request signer: fix the AWS Request Signer extension to correctly normalize the path and query string to be signed according to AWS' guidelines, so that the hash on the server side matches. See `AWS SigV4 documentaion `_. * cluster: delete pools when they're idle to fix unbounded memory use when using PROXY protocol upstream with tcp_proxy. This behavior can be temporarily reverted by setting the ``envoy.reloadable_features.conn_pool_delete_when_idle`` runtime guard to false. * 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. +* lua: Fix printing Lua string containing hex characters. Removed Config or Runtime ------------------------- From 571966fe5a70cd982026f5731b29162f96fd692e Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 13 Sep 2021 20:34:39 +0800 Subject: [PATCH 16/19] modify comment Signed-off-by: Patrick --- source/extensions/filters/common/lua/lua.h | 1 + 1 file changed, 1 insertion(+) diff --git a/source/extensions/filters/common/lua/lua.h b/source/extensions/filters/common/lua/lua.h index 60168fb7deb84..5271ce089f5c4 100644 --- a/source/extensions/filters/common/lua/lua.h +++ b/source/extensions/filters/common/lua/lua.h @@ -83,6 +83,7 @@ inline absl::string_view getStringViewFromLuaString(lua_State* state, int index) // 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); } From 924c80581fbe2d5f6f264999e73352e3067448d7 Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 13 Sep 2021 20:49:39 +0800 Subject: [PATCH 17/19] modify doc Signed-off-by: Patrick --- docs/root/version_history/current.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index d135d2d846710..0488ccbd7d192 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -43,7 +43,7 @@ Bug Fixes * aws request signer: fix the AWS Request Signer extension to correctly normalize the path and query string to be signed according to AWS' guidelines, so that the hash on the server side matches. See `AWS SigV4 documentaion `_. * cluster: delete pools when they're idle to fix unbounded memory use when using PROXY protocol upstream with tcp_proxy. This behavior can be temporarily reverted by setting the ``envoy.reloadable_features.conn_pool_delete_when_idle`` runtime guard to false. * 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. -* lua: Fix printing Lua string containing hex characters. +* 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. Removed Config or Runtime ------------------------- From aa6ff26de9fc1a26b0b95026b860e3ee875c406f Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 13 Sep 2021 21:39:38 +0800 Subject: [PATCH 18/19] modify doc Signed-off-by: Patrick --- docs/root/version_history/current.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 0488ccbd7d192..df078d8b211e0 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -42,8 +42,8 @@ Bug Fixes * access log: fix `%UPSTREAM_CLUSTER%` when used in http upstream access logs. Previously, it was always logging as an unset value. * aws request signer: fix the AWS Request Signer extension to correctly normalize the path and query string to be signed according to AWS' guidelines, so that the hash on the server side matches. See `AWS SigV4 documentaion `_. * cluster: delete pools when they're idle to fix unbounded memory use when using PROXY protocol upstream with tcp_proxy. This behavior can be temporarily reverted by setting the ``envoy.reloadable_features.conn_pool_delete_when_idle`` runtime guard to false. -* 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. * 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 ------------------------- From 3af7a01d0f4297df945691cbb920cc47147bfae2 Mon Sep 17 00:00:00 2001 From: Patrick Date: Mon, 13 Sep 2021 22:46:38 +0800 Subject: [PATCH 19/19] modify doc Signed-off-by: Patrick --- docs/root/version_history/current.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 16ce61e245ee5..546cd8e3c43f0 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -75,7 +75,6 @@ Bug Fixes * access log: fix ``%UPSTREAM_CLUSTER%`` when used in http upstream access logs. Previously, it was always logging as an unset value. * aws request signer: fix the AWS Request Signer extension to correctly normalize the path and query string to be signed according to AWS' guidelines, so that the hash on the server side matches. See `AWS SigV4 documentaion `_. * cluster: delete pools when they're idle to fix unbounded memory use when using PROXY protocol upstream with tcp_proxy. This behavior can be temporarily reverted by setting the ``envoy.reloadable_features.conn_pool_delete_when_idle`` runtime guard to false. -* 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. * cluster: finish cluster warming even if hosts are removed before health check initialization. This only affected clusters with :ref:`ignore_health_on_host_removal `. * compressor: fix a bug where if trailers were added and a subsequent filter paused the filter chain, the request could be stalled. This behavior can be reverted by setting ``envoy.reloadable_features.fix_added_trailers`` to false. * dynamic forward proxy: fixing a validation bug where san and sni checks were not applied setting :ref:`http_protocol_options ` via :ref:`typed_extension_protocol_options `. @@ -84,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