Skip to content

Commit b9caa47

Browse files
committed
Make keepEmptyvalue consistent with envoy impl
https://github.com/grpc/proposal/pull/481/changes#r3022089452
1 parent e046a49 commit b9caa47

2 files changed

Lines changed: 15 additions & 35 deletions

File tree

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

Lines changed: 12 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -71,23 +71,29 @@ private void updateHeader(final HeaderValueOption option, Metadata mutableHeader
7171

7272
if (header.key().endsWith(Metadata.BINARY_HEADER_SUFFIX)) {
7373
if (header.rawValue().isPresent()) {
74-
updateHeader(action, Metadata.Key.of(header.key(), Metadata.BINARY_BYTE_MARSHALLER),
75-
header.rawValue().get().toByteArray(), mutableHeaders, keepEmptyValue);
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);
78+
}
7679
} else {
7780
logger.fine("Missing binary rawValue for header: " + header.key());
7881
}
7982
} else {
8083
if (header.value().isPresent()) {
81-
updateHeader(action, Metadata.Key.of(header.key(), Metadata.ASCII_STRING_MARSHALLER),
82-
header.value().get(), mutableHeaders, keepEmptyValue);
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);
88+
}
8389
} else {
8490
logger.fine("Missing value for header: " + header.key());
8591
}
8692
}
8793
}
8894

8995
private <T> void updateHeader(final HeaderAppendAction action, final Metadata.Key<T> key,
90-
final T value, Metadata mutableHeaders, boolean keepEmptyValue) {
96+
final T value, Metadata mutableHeaders) {
9197
switch (action) {
9298
case APPEND_IF_EXISTS_OR_ADD:
9399
mutableHeaders.put(key, value);
@@ -112,33 +118,6 @@ private <T> void updateHeader(final HeaderAppendAction action, final Metadata.Ke
112118
// Should be unreachable unless there's a proto schema mismatch.
113119
logger.fine("Unknown HeaderAppendAction: " + action);
114120
}
115-
116-
if (!keepEmptyValue) {
117-
checkAndRemoveEmpty(key, mutableHeaders);
118-
}
119-
}
120-
121-
private <T> void checkAndRemoveEmpty(Metadata.Key<T> key, Metadata headers) {
122-
Iterable<T> values = headers.getAll(key);
123-
if (values == null) {
124-
return;
125-
}
126-
boolean allEmpty = true;
127-
for (T val : values) {
128-
if (val instanceof String) {
129-
if (!((String) val).isEmpty()) {
130-
allEmpty = false;
131-
break;
132-
}
133-
} else if (val instanceof byte[]) {
134-
if (((byte[]) val).length > 0) {
135-
allEmpty = false;
136-
break;
137-
}
138-
}
139-
}
140-
if (allEmpty) {
141-
headers.discardAll(key);
142-
}
143121
}
144122
}
123+

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,8 @@ public void applyMutations_keepEmptyValue() {
230230
headerMutator.applyMutations(mutations, headers);
231231

232232
assertThat(headers.containsKey(NEW_ADD_KEY)).isFalse();
233-
assertThat(headers.getAll(APPEND_KEY)).containsExactly("existing-value", "");
234-
assertThat(headers.containsKey(OVERWRITE_KEY)).isFalse();
233+
assertThat(headers.getAll(APPEND_KEY)).containsExactly("existing-value");
234+
assertThat(headers.get(OVERWRITE_KEY)).isEqualTo("existing-value");
235235

236236
Metadata.Key<String> keepEmptyKey =
237237
Metadata.Key.of("keep-empty-key", Metadata.ASCII_STRING_MARSHALLER);
@@ -242,6 +242,7 @@ public void applyMutations_keepEmptyValue() {
242242
assertThat(headers.get(keepEmptyKey)).isEqualTo("");
243243
assertThat(headers.containsKey(keepEmptyOverwriteKey)).isTrue();
244244
assertThat(headers.get(keepEmptyOverwriteKey)).isEqualTo("");
245+
245246
}
246247

247248
@Test

0 commit comments

Comments
 (0)