filter: add ability to resetIdleTimer (envoyproxy#17274)

Signed-off-by: Jose Nino <[email protected]>

envoy/http/filter.h

@@ -296,6 +296,11 @@ class StreamFilterCallbacks {
    * added to.
    */
   virtual void restoreContextOnContinue(ScopeTrackedObjectStack& tracked_object_stack) PURE;
+
+  /**
+   * Called when filter activity indicates that the stream idle timeout should be reset.
+   */
+  virtual void resetIdleTimer() PURE;
 };
 
 /**

source/common/http/async_client_impl.h

@@ -84,11 +84,6 @@ class AsyncStreamImpl : public AsyncClient::Stream,
   AsyncStreamImpl(AsyncClientImpl& parent, AsyncClient::StreamCallbacks& callbacks,
                   const AsyncClient::StreamOptions& options);
 
-  // Http::StreamDecoderFilterCallbacks
-  void requestRouteConfigUpdate(Http::RouteConfigUpdatedCallbackSharedPtr) override {
-    NOT_IMPLEMENTED_GCOVR_EXCL_LINE;
-  }
-
   // Http::AsyncClient::Stream
   void sendHeaders(RequestHeaderMap& headers, bool end_stream) override;
   void sendData(Buffer::Instance& data, bool end_stream) override;
@@ -436,6 +431,10 @@ class AsyncStreamImpl : public AsyncClient::Stream,
   }
   void addUpstreamSocketOptions(const Network::Socket::OptionsSharedPtr&) override {}
   Network::Socket::OptionsSharedPtr getUpstreamSocketOptions() const override { return {}; }
+  void requestRouteConfigUpdate(Http::RouteConfigUpdatedCallbackSharedPtr) override {
+    NOT_IMPLEMENTED_GCOVR_EXCL_LINE;
+  }
+  void resetIdleTimer() override { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; }
 
   // ScopeTrackedObject
   void dumpState(std::ostream& os, int indent_level) const override {

source/common/http/filter_manager.cc

@@ -270,6 +270,10 @@ void ActiveStreamFilterBase::clearRouteCache() {
   parent_.filter_manager_callbacks_.clearRouteCache();
 }
 
+void ActiveStreamFilterBase::resetIdleTimer() {
+  parent_.filter_manager_callbacks_.resetIdleTimer();
+}
+
 void FilterMatchState::evaluateMatchTreeWithNewData(MatchDataUpdateFunc update_func) {
   if (match_tree_evaluated_ || !matching_data_) {
     return;

source/common/http/filter_manager.h

@@ -145,6 +145,7 @@ struct ActiveStreamFilterBase : public virtual StreamFilterCallbacks,
   Tracing::Config& tracingConfig() override;
   const ScopeTrackedObject& scope() override;
   void restoreContextOnContinue(ScopeTrackedObjectStack& tracked_object_stack) override;
+  void resetIdleTimer() override;
 
   // Functions to set or get iteration state.
   bool canIterate() { return iteration_state_ == IterationState::Continue; }

test/common/http/filter_manager_test.cc

@@ -550,6 +550,23 @@ TEST_F(FilterManagerTest, MultipleOnLocalReply) {
   filter_manager_->destroyFilters();
 }
 
+TEST_F(FilterManagerTest, ResetIdleTimer) {
+  initialize();
+
+  std::shared_ptr<MockStreamDecoderFilter> decoder_filter(new NiceMock<MockStreamDecoderFilter>());
+
+  EXPECT_CALL(filter_factory_, createFilterChain(_))
+      .WillRepeatedly(Invoke([&](FilterChainFactoryCallbacks& callbacks) -> void {
+        callbacks.addStreamDecoderFilter(decoder_filter);
+      }));
+  filter_manager_->createFilterChain();
+
+  EXPECT_CALL(filter_manager_callbacks_, resetIdleTimer());
+  decoder_filter->callbacks_->resetIdleTimer();
+
+  filter_manager_->destroyFilters();
+}
+
 } // namespace
 } // namespace Http
 } // namespace Envoy

test/integration/BUILD

@@ -626,6 +626,7 @@ envoy_cc_test(
     deps = [
         ":http_protocol_integration_lib",
         "//test/integration/filters:backpressure_filter_config_lib",
+        "//test/integration/filters:reset_idle_timer_filter_lib",
         "//test/test_common:test_time_lib",
         "@envoy_api//envoy/config/bootstrap/v3:pkg_cc_proto",
         "@envoy_api//envoy/extensions/filters/network/http_connection_manager/v3:pkg_cc_proto",

test/integration/filters/BUILD

@@ -262,6 +262,21 @@ envoy_cc_test_library(
     ],
 )
 
+envoy_cc_test_library(
+    name = "reset_idle_timer_filter_lib",
+    srcs = [
+        "reset_idle_timer_filter.cc",
+    ],
+    deps = [
+        ":common_lib",
+        "//envoy/http:filter_interface",
+        "//envoy/registry",
+        "//envoy/server:filter_config_interface",
+        "//source/extensions/filters/http/common:pass_through_filter_lib",
+        "//test/extensions/filters/http/common:empty_http_filter_config_lib",
+    ],
+)
+
 envoy_cc_test_library(
     name = "set_response_code_filter_lib",
     srcs = [

test/integration/filters/reset_idle_timer_filter.cc

@@ -0,0 +1,39 @@
+#include <string>
+
+#include "envoy/http/filter.h"
+#include "envoy/registry/registry.h"
+#include "envoy/server/filter_config.h"
+
+#include "source/extensions/filters/http/common/pass_through_filter.h"
+
+#include "test/extensions/filters/http/common/empty_http_filter_config.h"
+
+namespace Envoy {
+
+// A test filter that resets the stream idle timer via callbacks.
+class ResetIdleTimerFilter : public Http::PassThroughFilter {
+public:
+  Http::FilterDataStatus decodeData(Buffer::Instance&, bool) override {
+    decoder_callbacks_->resetIdleTimer();
+    return Http::FilterDataStatus::Continue;
+  }
+};
+
+class ResetIdleTimerFilterConfig : public Extensions::HttpFilters::Common::EmptyHttpFilterConfig {
+public:
+  ResetIdleTimerFilterConfig() : EmptyHttpFilterConfig("reset-idle-timer-filter") {}
+
+  Http::FilterFactoryCb createFilter(const std::string&,
+                                     Server::Configuration::FactoryContext&) override {
+    return [](Http::FilterChainFactoryCallbacks& callbacks) -> void {
+      callbacks.addStreamFilter(std::make_shared<::Envoy::ResetIdleTimerFilter>());
+    };
+  }
+};
+
+// perform static registration
+static Registry::RegisterFactory<ResetIdleTimerFilterConfig,
+                                 Server::Configuration::NamedHttpFilterConfigFactory>
+    register_;
+
+} // namespace Envoy

test/integration/idle_timeout_integration_test.cc

@@ -427,6 +427,35 @@ TEST_P(IdleTimeoutIntegrationTest, RequestTimeoutIsNotDisarmedByEncode100Continu
   EXPECT_EQ("request timeout", response->body());
 }
 
+// Per-stream idle timeout reset from within a filter.
+TEST_P(IdleTimeoutIntegrationTest, PerStreamIdleTimeoutResetFromFilter) {
+  config_helper_.addFilter(R"EOF(
+  name: reset-idle-timer-filter
+  )EOF");
+  enable_per_stream_idle_timeout_ = true;
+
+  auto response = setupPerStreamIdleTimeoutTest();
+
+  sleep();
+  codec_client_->sendData(*request_encoder_, 1, false);
+
+  // Two sleeps should trigger the timer, as each advances time by timeout / 2. However, the data
+  // frame above would have caused the filter to reset the timer. Thus, the stream should not be
+  // reset yet.
+  sleep();
+
+  EXPECT_FALSE(response->complete());
+
+  sleep();
+
+  waitForTimeout(*response, "downstream_rq_idle_timeout");
+
+  // Now the timer should have triggered.
+  EXPECT_TRUE(response->complete());
+  EXPECT_EQ("408", response->headers().getStatusValue());
+  EXPECT_EQ("stream timeout", response->body());
+}
+
 // TODO(auni53) create a test filter that hangs and does not send data upstream, which would
 // trigger a configured request_timer
 

test/mocks/http/mocks.h

@@ -202,6 +202,7 @@ class MockStreamDecoderFilterCallbacks : public StreamDecoderFilterCallbacks,
   MOCK_METHOD(const Network::Connection*, connection, ());
   MOCK_METHOD(Event::Dispatcher&, dispatcher, ());
   MOCK_METHOD(void, resetStream, ());
+  MOCK_METHOD(void, resetIdleTimer, ());
   MOCK_METHOD(Upstream::ClusterInfoConstSharedPtr, clusterInfo, ());
   MOCK_METHOD(Router::RouteConstSharedPtr, route, ());
   MOCK_METHOD(Router::RouteConstSharedPtr, route, (const Router::RouteCallback&));
@@ -292,6 +293,7 @@ class MockStreamEncoderFilterCallbacks : public StreamEncoderFilterCallbacks,
   MOCK_METHOD(const Network::Connection*, connection, ());
   MOCK_METHOD(Event::Dispatcher&, dispatcher, ());
   MOCK_METHOD(void, resetStream, ());
+  MOCK_METHOD(void, resetIdleTimer, ());
   MOCK_METHOD(Upstream::ClusterInfoConstSharedPtr, clusterInfo, ());
   MOCK_METHOD(void, requestRouteConfigUpdate, (std::function<void()>));
   MOCK_METHOD(bool, canRequestRouteConfigUpdate, ());