From bd0357f22a80d5245fdc4e8bd4a8b9cd42901362 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E7=BD=97=E6=B3=BD=E8=BD=A9?= <spacewanderlzx@gmail.com>
Date: Fri, 23 Feb 2024 11:09:48 +0800
Subject: [PATCH] filtermanager: skip method was broken when there are multi
 plugins (#308)

This bug is introduced during optimization.
Signed-off-by: spacewander <spacewanderlzx@gmail.com>

---------

Signed-off-by: spacewander <spacewanderlzx@gmail.com>
---
 pkg/filtermanager/filtermanager.go      | 10 ++++++++--
 pkg/filtermanager/filtermanager_test.go | 25 +++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/pkg/filtermanager/filtermanager.go b/pkg/filtermanager/filtermanager.go
index 58af4adb..fcf42ca3 100644
--- a/pkg/filtermanager/filtermanager.go
+++ b/pkg/filtermanager/filtermanager.go
@@ -350,6 +350,11 @@ func FilterManagerFactory(c interface{}, cb capi.FilterCallbackHandler) capi.Str
 	fm := conf.pool.Get().(*filterManager)
 	fm.callbacks.FilterCallbackHandler = cb
 
+	canSkipMethod := fm.canSkipMethod
+	if canSkipMethod == nil {
+		canSkipMethod = newSkipMethodsMap()
+	}
+
 	filters := make([]*filterWrapper, len(newConfig))
 	for i, fc := range newConfig {
 		factory := fc.Factory
@@ -357,7 +362,6 @@ func FilterManagerFactory(c interface{}, cb capi.FilterCallbackHandler) capi.Str
 		f := factory(config, fm.callbacks)
 		// Technically, the factory might create different f for different calls. We don't support this edge case for now.
 		if fm.canSkipMethod == nil {
-			canSkipMethod := newSkipMethodsMap()
 			for meth := range canSkipMethod {
 				ok, err := isMethodFromPassThroughFilter(f, meth)
 				if err != nil {
@@ -366,11 +370,13 @@ func FilterManagerFactory(c interface{}, cb capi.FilterCallbackHandler) capi.Str
 				}
 				canSkipMethod[meth] = canSkipMethod[meth] && ok
 			}
-			fm.canSkipMethod = canSkipMethod
 		}
 		filters[i] = newFilterWrapper(fc.Name, f)
 	}
 
+	if fm.canSkipMethod == nil {
+		fm.canSkipMethod = canSkipMethod
+	}
 	fm.filters = filters
 
 	if conf.consumerFiltersEndAt != 0 {
diff --git a/pkg/filtermanager/filtermanager_test.go b/pkg/filtermanager/filtermanager_test.go
index 1f0298bd..0bfc350c 100644
--- a/pkg/filtermanager/filtermanager_test.go
+++ b/pkg/filtermanager/filtermanager_test.go
@@ -343,6 +343,31 @@ func (f *addReqFilter) DecodeHeaders(headers api.RequestHeaderMap, endStream boo
 	return api.Continue
 }
 
+func TestSkipMethodWhenThereAreMultiFilters(t *testing.T) {
+	cb := envoy.NewCAPIFilterCallbackHandler()
+	config := initFilterManagerConfig("ns")
+	config.current = []*model.ParsedFilterConfig{
+		{
+			Name:    "add_req",
+			Factory: addReqFactory,
+			ParsedConfig: addReqConf{
+				hdrName: "x-htnn-route",
+			},
+		},
+		{
+			Name:    "on_log",
+			Factory: onLogFactory,
+		},
+	}
+
+	for i := 0; i < 2; i++ {
+		m := FilterManagerFactory(config, cb).(*filterManager)
+		assert.Equal(t, false, m.canSkipOnLog)
+		assert.Equal(t, false, m.canSkipDecodeHeaders)
+		assert.Equal(t, true, m.canSkipDecodeData)
+	}
+}
+
 func TestFiltersFromConsumer(t *testing.T) {
 	cb := envoy.NewCAPIFilterCallbackHandler()
 	config := initFilterManagerConfig("ns")