Skip to content

Commit ff00d5b

Browse files
committed
xds: ignore keep_empty_value and discard header keys on empty mutations
In GrpcService/ext_proc header mutations, we do not support the `keep_empty_value` field (grpc/proposal@f6d42d6). If any header mutation results in a header containing an empty value (either string or binary), the entire header key is discarded/removed from the metadata. This chang modifies HeaderMutator to apply mutations and discard keys containing empty values.
1 parent 663c505 commit ff00d5b

2 files changed

Lines changed: 34 additions & 19 deletions

File tree

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

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,24 +67,23 @@ private void applyHeaderUpdates(final Iterable<HeaderValueOption> headerOptions,
6767
private void updateHeader(final HeaderValueOption option, Metadata mutableHeaders) {
6868
HeaderValue header = option.header();
6969
HeaderAppendAction action = option.appendAction();
70-
boolean keepEmptyValue = option.keepEmptyValue();
7170

7271
if (header.key().endsWith(Metadata.BINARY_HEADER_SUFFIX)) {
7372
if (header.rawValue().isPresent()) {
74-
byte[] value = header.rawValue().get().toByteArray();
75-
if (value.length > 0 || keepEmptyValue) {
76-
updateHeader(action, Metadata.Key.of(header.key(), Metadata.BINARY_BYTE_MARSHALLER),
77-
value, mutableHeaders);
73+
Metadata.Key<byte[]> key = Metadata.Key.of(header.key(), Metadata.BINARY_BYTE_MARSHALLER);
74+
updateHeader(action, key, header.rawValue().get().toByteArray(), mutableHeaders);
75+
if (containsEmpty(key, mutableHeaders)) {
76+
mutableHeaders.discardAll(key);
7877
}
7978
} else {
8079
logger.fine("Missing binary rawValue for header: " + header.key());
8180
}
8281
} else {
8382
if (header.value().isPresent()) {
84-
String value = header.value().get();
85-
if (!value.isEmpty() || keepEmptyValue) {
86-
updateHeader(action, Metadata.Key.of(header.key(), Metadata.ASCII_STRING_MARSHALLER),
87-
value, mutableHeaders);
83+
Metadata.Key<String> key = Metadata.Key.of(header.key(), Metadata.ASCII_STRING_MARSHALLER);
84+
updateHeader(action, key, header.value().get(), mutableHeaders);
85+
if (containsEmpty(key, mutableHeaders)) {
86+
mutableHeaders.discardAll(key);
8887
}
8988
} else {
9089
logger.fine("Missing value for header: " + header.key());
@@ -119,5 +118,24 @@ private <T> void updateHeader(final HeaderAppendAction action, final Metadata.Ke
119118
logger.fine("Unknown HeaderAppendAction: " + action);
120119
}
121120
}
121+
122+
private <T> boolean containsEmpty(Metadata.Key<T> key, Metadata headers) {
123+
Iterable<T> values = headers.getAll(key);
124+
if (values == null) {
125+
return false;
126+
}
127+
for (T val : values) {
128+
if (val instanceof String) {
129+
if (((String) val).isEmpty()) {
130+
return true;
131+
}
132+
} else if (val instanceof byte[]) {
133+
if (((byte[]) val).length == 0) {
134+
return true;
135+
}
136+
}
137+
}
138+
return false;
139+
}
122140
}
123141

xds/src/test/java/io/grpc/xds/internal/headermutations/HeaderMutatorTest.java

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -247,30 +247,27 @@ public void applyMutations_keepEmptyValue() {
247247
headerMutator.applyMutations(mutations, headers);
248248

249249
assertThat(headers.containsKey(NEW_ADD_KEY)).isFalse();
250-
assertThat(headers.getAll(APPEND_KEY)).containsExactly("existing-value");
251-
assertThat(headers.get(OVERWRITE_KEY)).isEqualTo("existing-value");
250+
assertThat(headers.containsKey(APPEND_KEY)).isFalse();
251+
assertThat(headers.containsKey(OVERWRITE_KEY)).isFalse();
252252
assertThat(headers.containsKey(ADD_KEY)).isFalse();
253-
assertThat(headers.get(OVERWRITE_IF_EXISTS_KEY)).isEqualTo("existing-value");
253+
assertThat(headers.containsKey(OVERWRITE_IF_EXISTS_KEY)).isFalse();
254254

255255
Metadata.Key<String> keepEmptyKey =
256256
Metadata.Key.of("keep-empty-key", Metadata.ASCII_STRING_MARSHALLER);
257257
Metadata.Key<String> keepEmptyOverwriteKey =
258258
Metadata.Key.of("keep-empty-overwrite-key", Metadata.ASCII_STRING_MARSHALLER);
259259

260-
assertThat(headers.containsKey(keepEmptyKey)).isTrue();
261-
assertThat(headers.get(keepEmptyKey)).isEqualTo("");
262-
assertThat(headers.containsKey(keepEmptyOverwriteKey)).isTrue();
263-
assertThat(headers.get(keepEmptyOverwriteKey)).isEqualTo("");
260+
assertThat(headers.containsKey(keepEmptyKey)).isFalse();
261+
assertThat(headers.containsKey(keepEmptyOverwriteKey)).isFalse();
264262

265263
Metadata.Key<byte[]> keepEmptyBinKey =
266264
Metadata.Key.of("keep-empty-bin-key-bin", Metadata.BINARY_BYTE_MARSHALLER);
267265
Metadata.Key<byte[]> ignoreEmptyBinKey =
268266
Metadata.Key.of("ignore-empty-bin-key-bin", Metadata.BINARY_BYTE_MARSHALLER);
269267

270-
assertThat(headers.containsKey(keepEmptyBinKey)).isTrue();
271-
assertThat(headers.get(keepEmptyBinKey)).isEqualTo(new byte[0]);
268+
assertThat(headers.containsKey(keepEmptyBinKey)).isFalse();
272269
assertThat(headers.containsKey(ignoreEmptyBinKey)).isFalse();
273-
assertThat(headers.get(overwriteEmptyBinKey)).isEqualTo(originalBinValue);
270+
assertThat(headers.containsKey(overwriteEmptyBinKey)).isFalse();
274271
}
275272

276273
@Test

0 commit comments

Comments
 (0)