Skip to content

Commit 3d98fb9

Browse files
AgraVatorAgraVator
committed
fix: add recursion depth
1 parent 96c1fb1 commit 3d98fb9

3 files changed

Lines changed: 72 additions & 35 deletions

File tree

‎xds/src/main/java/io/grpc/xds/CompositeFilter.java‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ public Filter newInstance(String name) {
113113

114114
@Override
115115
public ConfigOrError<CompositeFilterConfig> parseFilterConfig(
116-
Message rawProtoMessage, int depth) {
116+
Message rawProtoMessage, FilterConfigParseContext context) {
117117
if (!isSupported()) {
118118
return ConfigOrError.fromError("Composite Filter is experimental "
119119
+ "and disabled by default.");
@@ -122,11 +122,15 @@ public ConfigOrError<CompositeFilterConfig> parseFilterConfig(
122122
return ConfigOrError.fromError("Invalid message type: "
123123
+ rawProtoMessage.getClass().getName());
124124
}
125+
int depth = context.recursionDepth() != null ? context.recursionDepth() : 0;
126+
if (depth >= 8) {
127+
return ConfigOrError.fromError("Maximum recursion depth of 8 exceeded");
128+
}
125129
try {
126130
Any any = (Any) rawProtoMessage;
127131
if (any.is(ExtensionWithMatcher.class)) {
128132
ExtensionWithMatcher proto = any.unpack(ExtensionWithMatcher.class);
129-
return parseMatcherConfig(proto.getXdsMatcher(), depth);
133+
return parseMatcherConfig(proto.getXdsMatcher(), context);
130134
} else if (any.is(Composite.class)) {
131135
return ConfigOrError.fromConfig(new CompositeFilterConfig(null));
132136
}
@@ -138,7 +142,7 @@ public ConfigOrError<CompositeFilterConfig> parseFilterConfig(
138142

139143
@Override
140144
public ConfigOrError<CompositeFilterConfig> parseFilterConfigOverride(
141-
Message rawProtoMessage, int depth) {
145+
Message rawProtoMessage, FilterConfigParseContext context) {
142146
if (!isSupported()) {
143147
return ConfigOrError.fromError("Composite Filter is experimental and disabled"
144148
+ " by default.");
@@ -147,11 +151,15 @@ public ConfigOrError<CompositeFilterConfig> parseFilterConfigOverride(
147151
return ConfigOrError.fromError("Invalid message type: "
148152
+ rawProtoMessage.getClass().getName());
149153
}
154+
int depth = context.recursionDepth() != null ? context.recursionDepth() : 0;
155+
if (depth >= 8) {
156+
return ConfigOrError.fromError("Maximum recursion depth of 8 exceeded");
157+
}
150158
try {
151159
Any any = (Any) rawProtoMessage;
152160
if (any.is(ExtensionWithMatcherPerRoute.class)) {
153161
ExtensionWithMatcherPerRoute proto = any.unpack(ExtensionWithMatcherPerRoute.class);
154-
return parseMatcherConfig(proto.getXdsMatcher(), depth);
162+
return parseMatcherConfig(proto.getXdsMatcher(), context);
155163
}
156164
} catch (InvalidProtocolBufferException e) {
157165
return ConfigOrError.fromError("Invalid proto: " + e);
@@ -161,14 +169,14 @@ public ConfigOrError<CompositeFilterConfig> parseFilterConfigOverride(
161169
}
162170

163171
private ConfigOrError<CompositeFilterConfig> parseMatcherConfig(
164-
@Nullable Matcher matcherProto, int depth) {
172+
@Nullable Matcher matcherProto, FilterConfigParseContext context) {
165173
if (matcherProto == null) {
166174
return ConfigOrError.fromConfig(new CompositeFilterConfig(null));
167175
}
168176

169177
try {
170178
UnifiedMatcher<FilterDelegate> matcher = UnifiedMatcher.create(matcherProto,
171-
config -> createFilterDelegate(config, depth));
179+
config -> createFilterDelegate(config, context));
172180
return ConfigOrError.fromConfig(new CompositeFilterConfig(matcher));
173181
} catch (Exception e) {
174182
return ConfigOrError.fromError("Failed to create matcher: " + e.getMessage());
@@ -180,7 +188,7 @@ private boolean isSupported() {
180188
}
181189

182190
private static FilterDelegate createFilterDelegate(
183-
com.github.xds.core.v3.TypedExtensionConfig config, int depth) {
191+
com.github.xds.core.v3.TypedExtensionConfig config, FilterConfigParseContext context) {
184192
try {
185193
Any actionAny = config.getTypedConfig();
186194
if (actionAny.is(ExecuteFilterAction.class)) {
@@ -227,8 +235,12 @@ private static FilterDelegate createFilterDelegate(
227235
if (provider == null) {
228236
throw new IllegalArgumentException("Action filter not found: " + typeUrl);
229237
}
230-
ConfigOrError<? extends FilterConfig> parsed = Filter.Parser
231-
.parseFilterConfig(provider, rawConfig, depth + 1);
238+
int depth = context.recursionDepth() != null ? context.recursionDepth() : 0;
239+
Filter.FilterConfigParseContext childContext = context.toBuilder()
240+
.recursionDepth(depth + 1)
241+
.build();
242+
ConfigOrError<? extends FilterConfig> parsed = provider.parseFilterConfig(rawConfig,
243+
childContext);
232244
if (parsed.errorDetail != null) {
233245
throw new IllegalArgumentException(
234246
"Failed to parse child filter: " + parsed.errorDetail);

‎xds/src/main/java/io/grpc/xds/Filter.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,11 @@ abstract static class FilterConfigParseContext {
138138

139139
abstract ServerInfo serverInfo();
140140

141+
@Nullable
142+
abstract Integer recursionDepth();
143+
144+
abstract Builder toBuilder();
145+
141146
static Builder builder() {
142147
return new AutoValue_Filter_FilterConfigParseContext.Builder();
143148
}
@@ -148,6 +153,8 @@ abstract static class Builder {
148153

149154
abstract Builder serverInfo(ServerInfo info);
150155

156+
abstract Builder recursionDepth(Integer depth);
157+
151158
abstract FilterConfigParseContext build();
152159
}
153160
}

‎xds/src/test/java/io/grpc/xds/CompositeFilterTest.java‎

Lines changed: 44 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public void setUp() {
8585

8686
when(fakeProvider.typeUrls()).thenReturn(new String[]{FAKE_TYPE_URL});
8787
ConfigOrError<? extends FilterConfig> configRes = ConfigOrError.fromConfig(fakeConfig);
88-
when(fakeProvider.parseFilterConfig(any(Any.class), org.mockito.ArgumentMatchers.anyInt()))
88+
when(fakeProvider.parseFilterConfig(any(Any.class), any()))
8989
.thenReturn((ConfigOrError) configRes);
9090
when(fakeProvider.newInstance(any(String.class))).thenReturn(fakeFilter);
9191
when(fakeFilter.buildClientInterceptor(any(), any(), any())).thenReturn(fakeClientInterceptor);
@@ -106,6 +106,19 @@ public void tearDown() {
106106
typeUrl -> FilterRegistry.getDefaultRegistry().get(typeUrl);
107107
}
108108

109+
private Filter.FilterConfigParseContext getFilterContext() {
110+
return Filter.FilterConfigParseContext.builder()
111+
.bootstrapInfo(io.grpc.xds.client.Bootstrapper.BootstrapInfo.builder()
112+
.servers(Collections.singletonList(
113+
io.grpc.xds.client.Bootstrapper.ServerInfo.create(
114+
"test_target", Collections.emptyMap())))
115+
.node(io.grpc.xds.client.EnvoyProtoData.Node.newBuilder().build())
116+
.build())
117+
.serverInfo(io.grpc.xds.client.Bootstrapper.ServerInfo.create(
118+
"test_target", Collections.emptyMap(), false, true, false, false))
119+
.build();
120+
}
121+
109122
@Test
110123
public void parseConfig() {
111124
Matcher.OnMatch matchAction = Matcher.OnMatch.newBuilder()
@@ -167,7 +180,7 @@ public void parseConfig() {
167180
.build();
168181

169182
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
170-
.parseFilterConfig(Any.pack(proto), 0);
183+
.parseFilterConfig(Any.pack(proto), getFilterContext());
171184

172185
assertThat(result.errorDetail).isNull();
173186
assertThat(result.config).isNotNull();
@@ -236,7 +249,7 @@ public void clientInterceptorDelegates() {
236249
.build();
237250

238251
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
239-
.parseFilterConfig(Any.pack(proto), 0);
252+
.parseFilterConfig(Any.pack(proto), getFilterContext());
240253

241254
assertThat(result.errorDetail).isNull();
242255
assertThat(result.config).isNotNull();
@@ -282,7 +295,7 @@ public void clientInterceptorSkips() {
282295
.build();
283296

284297
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
285-
.parseFilterConfig(Any.pack(proto), 0);
298+
.parseFilterConfig(Any.pack(proto), getFilterContext());
286299

287300
CompositeFilter filter = (CompositeFilter) provider.newInstance("composite");
288301
ClientInterceptor interceptor = filter.buildClientInterceptor(result.config, null,
@@ -383,7 +396,7 @@ public void clientInterceptorDelegatesChain() {
383396
.build();
384397

385398
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
386-
.parseFilterConfig(Any.pack(proto), 0);
399+
.parseFilterConfig(Any.pack(proto), getFilterContext());
387400

388401
assertThat(result.errorDetail).isNull();
389402
CompositeFilter filter = (CompositeFilter) provider.newInstance("composite");
@@ -504,7 +517,7 @@ public void serverNameInputMatch() {
504517
.build();
505518

506519
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
507-
.parseFilterConfig(Any.pack(proto), 0);
520+
.parseFilterConfig(Any.pack(proto), getFilterContext());
508521

509522
assertThat(result.errorDetail).isNull();
510523

@@ -617,7 +630,7 @@ public void samplePercentAlwaysFalse() {
617630
.build();
618631

619632
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
620-
.parseFilterConfig(Any.pack(proto), 0);
633+
.parseFilterConfig(Any.pack(proto), getFilterContext());
621634

622635
assertThat(result.errorDetail).isNull();
623636

@@ -710,7 +723,7 @@ public void parseFilterConfigRejectsOverrideMessage() {
710723
.build();
711724

712725
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
713-
.parseFilterConfig(Any.pack(overrideProto), 0);
726+
.parseFilterConfig(Any.pack(overrideProto), getFilterContext());
714727

715728
assertThat(result.errorDetail).contains("Unsupported message type in parseFilterConfig");
716729
}
@@ -722,7 +735,7 @@ public void parseFilterConfigOverrideRejectsConfigMessage() {
722735
.build();
723736

724737
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
725-
.parseFilterConfigOverride(Any.pack(configProto), 0);
738+
.parseFilterConfigOverride(Any.pack(configProto), getFilterContext());
726739

727740
assertThat(result.errorDetail).contains("Unsupported message type in"
728741
+ " parseFilterConfigOverride");
@@ -733,7 +746,7 @@ public void parseFilterConfigFailsWhenDisabled() {
733746
System.clearProperty("GRPC_EXPERIMENTAL_XDS_COMPOSITE_FILTER");
734747
try {
735748
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
736-
.parseFilterConfig(Any.pack(Composite.getDefaultInstance()), 0);
749+
.parseFilterConfig(Any.pack(Composite.getDefaultInstance()), getFilterContext());
737750
assertThat(result.errorDetail).contains("Composite Filter is experimental");
738751
} finally {
739752
System.setProperty("GRPC_EXPERIMENTAL_XDS_COMPOSITE_FILTER", "true");
@@ -743,7 +756,7 @@ public void parseFilterConfigFailsWhenDisabled() {
743756
@Test
744757
public void parseFilterConfigWithEmptyConfig() {
745758
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
746-
.parseFilterConfig(Any.pack(Composite.getDefaultInstance()), 0);
759+
.parseFilterConfig(Any.pack(Composite.getDefaultInstance()), getFilterContext());
747760

748761
assertThat(result.errorDetail).isNull();
749762
assertThat(result.config.matcher).isNull();
@@ -794,7 +807,7 @@ public void clientInterceptorUsesOverrideMatcher() {
794807
.build();
795808

796809
ConfigOrError<CompositeFilter.CompositeFilterConfig> baseResult = provider
797-
.parseFilterConfig(Any.pack(baseProto), 0);
810+
.parseFilterConfig(Any.pack(baseProto), getFilterContext());
798811

799812
// Override matcher that is empty (skips everything)
800813
Matcher overrideMatcherProto = Matcher.newBuilder().build();
@@ -803,7 +816,7 @@ public void clientInterceptorUsesOverrideMatcher() {
803816
.build();
804817

805818
ConfigOrError<CompositeFilter.CompositeFilterConfig> overrideResult = provider
806-
.parseFilterConfigOverride(Any.pack(overrideProto), 0);
819+
.parseFilterConfigOverride(Any.pack(overrideProto), getFilterContext());
807820

808821
CompositeFilter filter = (CompositeFilter) provider.newInstance("composite");
809822
ClientInterceptor interceptor = filter.buildClientInterceptor(
@@ -880,7 +893,7 @@ public void serverInterceptorDelegates() {
880893
.build();
881894

882895
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
883-
.parseFilterConfig(Any.pack(proto), 0);
896+
.parseFilterConfig(Any.pack(proto), getFilterContext());
884897

885898
CompositeFilter filter = (CompositeFilter) provider.newInstance("composite");
886899
ServerInterceptor interceptor = filter.buildServerInterceptor(result.config, null);
@@ -945,7 +958,7 @@ public void clientInterceptorClosesFiltersOnClose() {
945958
.build();
946959

947960
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
948-
.parseFilterConfig(Any.pack(proto), 0);
961+
.parseFilterConfig(Any.pack(proto), getFilterContext());
949962

950963
CompositeFilter filter = (CompositeFilter) provider.newInstance("composite");
951964
ClientInterceptor interceptor = filter.buildClientInterceptor(result.config, null,
@@ -1020,18 +1033,22 @@ public void parseFilterConfigExceedsRecursionLimit() {
10201033
final Any configAny = Any.pack(configProto);
10211034

10221035
// Mock fakeProvider to call provider.parseFilterConfig recursively
1023-
when(fakeProvider.parseFilterConfig(any(), org.mockito.ArgumentMatchers.anyInt()))
1036+
when(fakeProvider.parseFilterConfig(any(), any()))
10241037
.thenAnswer(new org.mockito.stubbing.Answer<ConfigOrError>() {
10251038
@Override
10261039
public ConfigOrError answer(
10271040
org.mockito.invocation.InvocationOnMock invocation) throws Throwable {
1028-
int passedDepth = invocation.getArgument(1);
1029-
return Filter.Parser.parseFilterConfig(provider, configAny, passedDepth + 1);
1041+
Filter.FilterConfigParseContext context = invocation.getArgument(1);
1042+
int depth = context.recursionDepth() != null ? context.recursionDepth() : 0;
1043+
Filter.FilterConfigParseContext childContext = context.toBuilder()
1044+
.recursionDepth(depth + 1)
1045+
.build();
1046+
return provider.parseFilterConfig(configAny, childContext);
10301047
}
10311048
});
10321049

1033-
ConfigOrError<? extends Filter.FilterConfig> result = Filter.Parser
1034-
.parseFilterConfig(provider, configAny, 0);
1050+
ConfigOrError<? extends Filter.FilterConfig> result = provider
1051+
.parseFilterConfig(configAny, getFilterContext());
10351052

10361053
assertThat(result.errorDetail).contains("Maximum recursion depth of 8 exceeded");
10371054
}
@@ -1079,7 +1096,7 @@ public void clientInterceptorSkipsOnSkipFilter() {
10791096
.build();
10801097

10811098
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
1082-
.parseFilterConfig(Any.pack(proto), 0);
1099+
.parseFilterConfig(Any.pack(proto), getFilterContext());
10831100

10841101
assertThat(result.errorDetail).isNull();
10851102

@@ -1235,15 +1252,16 @@ public void providerMethodsCovered() {
12351252
@Test
12361253
public void parseFilterConfigInvalidMessageType() {
12371254
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
1238-
.parseFilterConfig(com.google.protobuf.Empty.getDefaultInstance(), 0);
1255+
.parseFilterConfig(com.google.protobuf.Empty.getDefaultInstance(), getFilterContext());
12391256

12401257
assertThat(result.errorDetail).contains("Invalid message type");
12411258
}
12421259

12431260
@Test
12441261
public void parseFilterConfigOverrideInvalidMessageType() {
12451262
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
1246-
.parseFilterConfigOverride(com.google.protobuf.Empty.getDefaultInstance(), 0);
1263+
.parseFilterConfigOverride(
1264+
com.google.protobuf.Empty.getDefaultInstance(), getFilterContext());
12471265

12481266
assertThat(result.errorDetail).contains("Invalid message type");
12491267
}
@@ -1254,7 +1272,7 @@ public void parseFilterConfigOverrideFailsWhenDisabled() {
12541272
try {
12551273
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
12561274
.parseFilterConfigOverride(
1257-
Any.pack(ExtensionWithMatcherPerRoute.getDefaultInstance()), 0);
1275+
Any.pack(ExtensionWithMatcherPerRoute.getDefaultInstance()), getFilterContext());
12581276
assertThat(result.errorDetail).contains("Composite Filter is experimental");
12591277
} finally {
12601278
System.setProperty("GRPC_EXPERIMENTAL_XDS_COMPOSITE_FILTER", "true");
@@ -1269,7 +1287,7 @@ public void parseFilterConfigInvalidProtoBytes() {
12691287
new byte[]{(byte) 0x80})) // Invalid proto bytes
12701288
.build();
12711289
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
1272-
.parseFilterConfig(invalidAny, 0);
1290+
.parseFilterConfig(invalidAny, getFilterContext());
12731291
assertThat(result.errorDetail).contains("Invalid proto:");
12741292
}
12751293

@@ -1281,7 +1299,7 @@ public void parseFilterConfigOverrideInvalidProtoBytes() {
12811299
new byte[]{(byte) 0x80})) // Invalid proto bytes
12821300
.build();
12831301
ConfigOrError<CompositeFilter.CompositeFilterConfig> result = provider
1284-
.parseFilterConfigOverride(invalidAny, 0);
1302+
.parseFilterConfigOverride(invalidAny, getFilterContext());
12851303
assertThat(result.errorDetail).contains("Invalid proto:");
12861304
}
12871305
}

0 commit comments

Comments
 (0)