Skip to content

Commit 44da871

Browse files
authored
fix: retrieving http headers on request object should be case insenstive (#178)
* Add test case covering case sensitive header operations * Make HttpRequest.getFirstHeader case insensitive
1 parent 3ac1abd commit 44da871

File tree

3 files changed

+30
-14
lines changed

3 files changed

+30
-14
lines changed

invoker/core/src/main/java/com/google/cloud/functions/invoker/http/HttpRequestImpl.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import java.util.List;
3131
import java.util.Map;
3232
import java.util.Optional;
33+
import java.util.TreeMap;
3334
import java.util.regex.Matcher;
3435
import java.util.regex.Pattern;
3536
import javax.servlet.ServletException;
@@ -116,8 +117,12 @@ public BufferedReader getReader() throws IOException {
116117
@Override
117118
public Map<String, List<String>> getHeaders() {
118119
return Collections.list(request.getHeaderNames()).stream()
119-
.map(name -> new SimpleEntry<>(name, Collections.list(request.getHeaders(name))))
120-
.collect(toMap(Map.Entry::getKey, Map.Entry::getValue));
120+
.collect(
121+
toMap(
122+
name -> name,
123+
name -> Collections.list(request.getHeaders(name)),
124+
(a, b) -> b,
125+
() -> new TreeMap<>(String.CASE_INSENSITIVE_ORDER)));
121126
}
122127

123128
private static class HttpPartImpl implements HttpPart {

invoker/core/src/main/java/com/google/cloud/functions/invoker/http/HttpResponseImpl.java

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@
2020
import java.io.BufferedWriter;
2121
import java.io.IOException;
2222
import java.io.OutputStream;
23-
import java.util.AbstractMap.SimpleEntry;
2423
import java.util.ArrayList;
2524
import java.util.Collection;
2625
import java.util.List;
2726
import java.util.Map;
2827
import java.util.Optional;
28+
import java.util.TreeMap;
2929
import javax.servlet.http.HttpServletResponse;
3030

3131
public class HttpResponseImpl implements HttpResponse {
@@ -64,8 +64,12 @@ public void appendHeader(String key, String value) {
6464
@Override
6565
public Map<String, List<String>> getHeaders() {
6666
return response.getHeaderNames().stream()
67-
.map(header -> new SimpleEntry<>(header, list(response.getHeaders(header))))
68-
.collect(toMap(Map.Entry::getKey, Map.Entry::getValue));
67+
.collect(
68+
toMap(
69+
name -> name,
70+
name -> new ArrayList<>(response.getHeaders(name)),
71+
(a, b) -> b,
72+
() -> new TreeMap<>(String.CASE_INSENSITIVE_ORDER)));
6973
}
7074

7175
private static <T> List<T> list(Collection<T> collection) {
@@ -82,12 +86,17 @@ public OutputStream getOutputStream() throws IOException {
8286
@Override
8387
public synchronized BufferedWriter getWriter() throws IOException {
8488
if (writer == null) {
85-
// Unfortunately this means that we get two intermediate objects between the object we return
86-
// and the underlying Writer that response.getWriter() wraps. We could try accessing the
87-
// PrintWriter.out field via reflection, but that sort of access to non-public fields of
88-
// platform classes is now frowned on and may draw warnings or even fail in subsequent
89+
// Unfortunately this means that we get two intermediate objects between the
90+
// object we return
91+
// and the underlying Writer that response.getWriter() wraps. We could try
92+
// accessing the
93+
// PrintWriter.out field via reflection, but that sort of access to non-public
94+
// fields of
95+
// platform classes is now frowned on and may draw warnings or even fail in
96+
// subsequent
8997
// versions.
90-
// We could instead wrap the OutputStream, but that would require us to deduce the appropriate
98+
// We could instead wrap the OutputStream, but that would require us to deduce
99+
// the appropriate
91100
// Charset, using logic like this:
92101
// https://github.com/eclipse/jetty.project/blob/923ec38adf/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java#L731
93102
// We may end up doing that if performance is an issue.

invoker/core/src/test/java/com/google/cloud/functions/invoker/http/HttpTest.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,8 @@ private void httpRequestMethods(
181181
assertThat(request.getHeaders()).containsAtLeastEntriesIn(expectedHeaders);
182182
},
183183
request -> assertThat(request.getFirstHeader("foo")).hasValue("bar"),
184+
request -> assertThat(request.getFirstHeader("CaSe-SeNsItIvE")).hasValue("VaLuE"),
185+
request -> assertThat(request.getFirstHeader("case-sensitive")).hasValue("VaLuE"),
184186
request -> {
185187
try {
186188
request.getParts();
@@ -197,6 +199,7 @@ private void httpRequestMethods(
197199
.header(HttpHeader.CONTENT_TYPE, "text/plain; charset=utf-8")
198200
.header("foo", "bar")
199201
.header("foo", "baz")
202+
.header("CaSe-SeNsItIvE", "VaLuE")
200203
.content(new StringContentProvider(TEST_BODY));
201204
ContentResponse response = request.send();
202205
assertThat(response.getStatus()).isEqualTo(HttpStatus.OK_200);
@@ -352,11 +355,10 @@ private void httpResponseSetAndGet(
352355
// So we just check that we can add our own headers.
353356
response.appendHeader("foo", "bar");
354357
response.appendHeader("wibbly", "wobbly");
355-
response.appendHeader("foo", "baz");
356-
Map<String, List<String>> updatedHeaders = new TreeMap<>(response.getHeaders());
357-
updatedHeaders.keySet().removeAll(initialHeaders.keySet());
358+
response.appendHeader("FoO", "baz");
359+
var updatedHeaders = response.getHeaders();
358360
assertThat(updatedHeaders)
359-
.containsExactly("foo", Arrays.asList("bar", "baz"), "wibbly", Arrays.asList("wobbly"));
361+
.containsAtLeast("foo", Arrays.asList("bar", "baz"), "wibbly", Arrays.asList("wobbly"));
360362
},
361363
};
362364
for (HttpResponseTest test : tests) {

0 commit comments

Comments
 (0)