diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 7ce364e87074..5fc82cec4f3a 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -203,6 +203,10 @@ bug_fixes: - area: admin change: | Removing the ECDS config entry from the config dump when it does not exist (or has expired). +- area: geoip + change: | + Switch to use StopAllIterationAndWatermark instead of StopIteration in geoip filter to fix an issue where large + POST request body may get corrupted when geoip is enabled. removed_config_or_runtime: # *Normally occurs at the end of the* :ref:`deprecation period ` diff --git a/source/extensions/filters/http/geoip/geoip_filter.cc b/source/extensions/filters/http/geoip/geoip_filter.cc index 2d197bb158fc..462d06a45fbd 100644 --- a/source/extensions/filters/http/geoip/geoip_filter.cc +++ b/source/extensions/filters/http/geoip/geoip_filter.cc @@ -63,8 +63,9 @@ Http::FilterHeadersStatus GeoipFilter::decodeHeaders(Http::RequestHeaderMap& hea }); }); - // Stop the iteration for headers for the current filter and the filters following. - return Http::FilterHeadersStatus::StopIteration; + // Stop the iteration for headers and data (POST request) for the current filter and the filters + // following. + return Http::FilterHeadersStatus::StopAllIterationAndWatermark; } Http::FilterDataStatus GeoipFilter::decodeData(Buffer::Instance&, bool) { diff --git a/test/extensions/filters/http/geoip/geoip_filter_test.cc b/test/extensions/filters/http/geoip/geoip_filter_test.cc index 2a0aa027ffbd..60bbf4a1981b 100644 --- a/test/extensions/filters/http/geoip/geoip_filter_test.cc +++ b/test/extensions/filters/http/geoip/geoip_filter_test.cc @@ -101,7 +101,7 @@ TEST_F(GeoipFilterTest, NoXffSuccessfulLookup) { EXPECT_EQ(Http::FilterDataStatus::Continue, filter_->decodeData(data_, false)); Http::TestRequestTrailerMapImpl request_trailers; EXPECT_EQ(Http::FilterTrailersStatus::Continue, filter_->decodeTrailers(request_trailers)); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter_->decodeHeaders(request_headers, false)); captured_cb_(Geolocation::LookupResult{{"x-geo-city", "dummy-city"}}); EXPECT_CALL(filter_callbacks_, continueDecoding()); @@ -134,7 +134,7 @@ TEST_F(GeoipFilterTest, UseXffSuccessfulLookup) { DoAll(SaveArg<0>(&captured_rq_), SaveArg<1>(&captured_cb_), Invoke([this]() { captured_cb_(Geolocation::LookupResult{{"x-geo-region", "dummy-region"}}); }))); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter_->decodeHeaders(request_headers, false)); EXPECT_CALL(filter_callbacks_, continueDecoding()); dispatcher_->run(Event::Dispatcher::RunType::Block); @@ -167,7 +167,7 @@ TEST_F(GeoipFilterTest, GeoHeadersOverridenForIncomingRequest) { captured_cb_(Geolocation::LookupResult{ {"x-geo-city", "dummy-city"}, {"x-geo-region", "dummy-region"}}); }))); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter_->decodeHeaders(request_headers, false)); EXPECT_CALL(filter_callbacks_, continueDecoding()); dispatcher_->run(Event::Dispatcher::RunType::Block); @@ -214,7 +214,7 @@ TEST_F(GeoipFilterTest, AllHeadersPropagatedCorrectly) { {"x-geo-anon-tor", "true"}, {"x-geo-anon-proxy", "true"}}); }))); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter_->decodeHeaders(request_headers, false)); EXPECT_CALL(filter_callbacks_, continueDecoding()); dispatcher_->run(Event::Dispatcher::RunType::Block); @@ -252,7 +252,7 @@ TEST_F(GeoipFilterTest, GeoHeaderNotAppendedOnEmptyLookup) { captured_cb_(Geolocation::LookupResult{ {"x-geo-city", ""}, {"x-geo-region", "dummy-region"}}); }))); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter_->decodeHeaders(request_headers, false)); EXPECT_CALL(filter_callbacks_, continueDecoding()); dispatcher_->run(Event::Dispatcher::RunType::Block); @@ -279,7 +279,7 @@ TEST_F(GeoipFilterTest, NoCrashIfFilterDestroyedBeforeCallbackCalled) { .WillRepeatedly(DoAll(SaveArg<0>(&captured_rq_), SaveArg<1>(&captured_cb_), Invoke([this]() { captured_cb_(Geolocation::LookupResult{{"x-geo-city", "dummy-city"}}); }))); - EXPECT_EQ(Http::FilterHeadersStatus::StopIteration, + EXPECT_EQ(Http::FilterHeadersStatus::StopAllIterationAndWatermark, filter_->decodeHeaders(request_headers, false)); filter_.reset(); dispatcher_->run(Event::Dispatcher::RunType::Block);