Skip to content

Commit e866d93

Browse files
committed
Fixup 12494: Address copilot comments
1 parent 5b85805 commit e866d93

File tree

2 files changed

+24
-14
lines changed

2 files changed

+24
-14
lines changed

xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutationFilter.java

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,10 @@ public HeaderMutationFilter(Optional<HeaderMutationRulesConfig> mutationRules) {
4747
public HeaderMutations filter(HeaderMutations mutations)
4848
throws HeaderMutationDisallowedException {
4949
ImmutableList<HeaderValueOption> allowedHeaders =
50-
filterCollection(mutations.headers(), this::shouldIgnore, this::isHeaderMutationAllowed);
50+
filterCollection(mutations.headers(), this::isDisallowed, this::isHeaderMutationAllowed);
5151
ImmutableList<String> allowedHeadersToRemove =
52-
filterCollection(mutations.headersToRemove(), this::shouldIgnore,
53-
this::isHeaderMutationAllowed);
52+
filterCollection(mutations.headersToRemove(), this::isDisallowed,
53+
this::isHeaderMutationAllowed);
5454
return HeaderMutations.create(allowedHeaders, allowedHeadersToRemove);
5555
}
5656

@@ -68,19 +68,18 @@ private <T> ImmutableList<T> filterCollection(Collection<T> items,
6868
if (isAllowedPredicate.test(item)) {
6969
allowed.add(item);
7070
} else if (disallowIsError()) {
71-
throw new HeaderMutationDisallowedException(
72-
"Header mutation disallowed for header: " + item);
71+
throw new HeaderMutationDisallowedException("Header mutation disallowed");
7372
}
7473
}
7574
return allowed.build();
7675
}
7776

78-
private boolean shouldIgnore(String key) {
79-
return HeaderValueValidationUtils.shouldIgnore(key);
77+
private boolean isDisallowed(String key) {
78+
return HeaderValueValidationUtils.isDisallowed(key);
8079
}
8180

82-
private boolean shouldIgnore(HeaderValueOption option) {
83-
return HeaderValueValidationUtils.shouldIgnore(option.header());
81+
private boolean isDisallowed(HeaderValueOption option) {
82+
return HeaderValueValidationUtils.isDisallowed(option.header());
8483
}
8584

8685
private boolean isHeaderMutationAllowed(HeaderValueOption option) {

xds/src/main/java/io/grpc/xds/internal/headermutations/HeaderMutator.java

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ public void applyMutations(final HeaderMutations mutations, Metadata headers) {
5050
// in case of conflicts. Copying the order from Envoy here, which does removals at the end.
5151
applyHeaderUpdates(mutations.headers(), headers);
5252
for (String headerToRemove : mutations.headersToRemove()) {
53-
headers.discardAll(Metadata.Key.of(headerToRemove, Metadata.ASCII_STRING_MARSHALLER));
53+
Metadata.Key<?> key = headerToRemove.endsWith(Metadata.BINARY_HEADER_SUFFIX)
54+
? Metadata.Key.of(headerToRemove, Metadata.BINARY_BYTE_MARSHALLER)
55+
: Metadata.Key.of(headerToRemove, Metadata.ASCII_STRING_MARSHALLER);
56+
headers.discardAll(key);
5457
}
5558
}
5659

@@ -67,11 +70,19 @@ private void updateHeader(final HeaderValueOption option, Metadata mutableHeader
6770
boolean keepEmptyValue = option.keepEmptyValue();
6871

6972
if (header.key().endsWith(Metadata.BINARY_HEADER_SUFFIX)) {
70-
updateHeader(action, Metadata.Key.of(header.key(), Metadata.BINARY_BYTE_MARSHALLER),
71-
header.rawValue().get().toByteArray(), mutableHeaders, keepEmptyValue);
73+
if (header.rawValue().isPresent()) {
74+
updateHeader(action, Metadata.Key.of(header.key(), Metadata.BINARY_BYTE_MARSHALLER),
75+
header.rawValue().get().toByteArray(), mutableHeaders, keepEmptyValue);
76+
} else {
77+
logger.fine("Missing binary rawValue for header: " + header.key());
78+
}
7279
} else {
73-
updateHeader(action, Metadata.Key.of(header.key(), Metadata.ASCII_STRING_MARSHALLER),
74-
header.value().get(), mutableHeaders, keepEmptyValue);
80+
if (header.value().isPresent()) {
81+
updateHeader(action, Metadata.Key.of(header.key(), Metadata.ASCII_STRING_MARSHALLER),
82+
header.value().get(), mutableHeaders, keepEmptyValue);
83+
} else {
84+
logger.fine("Missing value for header: " + header.key());
85+
}
7586
}
7687
}
7788

0 commit comments

Comments
 (0)