Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: multi value header handling #1888

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import io.micronaut.core.annotation.NonNull;
import io.micronaut.core.annotation.Nullable;
import io.micronaut.core.convert.ConversionService;
import io.micronaut.function.aws.proxy.MutableMapListOfStringAndMapStringConvertibleMultiValue;
import io.micronaut.http.HttpMethod;

import jakarta.inject.Singleton;
Expand All @@ -32,7 +31,6 @@
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.List;
import java.util.Map;

/**
Expand All @@ -47,7 +45,7 @@ public void handle(@NonNull ConversionService conversionService,
@NonNull HttpServletRequest request,
@NonNull APIGatewayV2HTTPResponse awsProxyResponse,
@NonNull HttpServletResponse response) throws IOException {
populateHeaders(conversionService, awsProxyResponse, response);
populateHeaders(awsProxyResponse, response);
response.setStatus(awsProxyResponse.getStatusCode());
HttpMethod httpMethod = HttpMethod.parse(request.getMethod());
if (httpMethod != HttpMethod.HEAD && httpMethod != HttpMethod.OPTIONS) {
Expand All @@ -65,15 +63,10 @@ public void handle(@NonNull ConversionService conversionService,
}
}

private void populateHeaders(@NonNull ConversionService conversionService,
@NonNull APIGatewayV2HTTPResponse apiGatewayV2HTTPResponse,
private void populateHeaders(@NonNull APIGatewayV2HTTPResponse apiGatewayV2HTTPResponse,
@NonNull HttpServletResponse response) {
Map<String, String> singleHeaders = apiGatewayV2HTTPResponse.getHeaders();
Map<String, List<String>> multiValueHeaders = apiGatewayV2HTTPResponse.getMultiValueHeaders();
MutableMapListOfStringAndMapStringConvertibleMultiValue entries = new MutableMapListOfStringAndMapStringConvertibleMultiValue(conversionService, multiValueHeaders, singleHeaders);

for (String name: entries.names()) {
response.addHeader(name, String.join(",", entries.getAll(name)));
for (Map.Entry<String, String> entry : apiGatewayV2HTTPResponse.getHeaders().entrySet()) {
response.addHeader(entry.getKey(), entry.getValue());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,13 @@
package io.micronaut.function.aws.proxy;

import io.micronaut.core.annotation.Internal;
import io.micronaut.core.annotation.NonNull;
import io.micronaut.core.annotation.Nullable;
import io.micronaut.core.util.CollectionUtils;
import io.micronaut.http.HttpHeaders;
import io.micronaut.http.MutableHttpHeaders;

import java.util.Arrays;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
Expand All @@ -31,6 +35,8 @@
@Internal
public final class MapCollapseUtils {

private static final List<String> HEADERS_ALLOWING_COMMAS = Arrays.asList(HttpHeaders.DATE, HttpHeaders.USER_AGENT);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be configurable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how Netty handles this @yawkat do you know?

Copy link
Contributor Author

@sdelamo sdelamo Sep 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be configurable?

but it should be configurable at not in AWS right?, should this configuration be in servlet-core?


private MapCollapseUtils() {
}

Expand Down Expand Up @@ -81,9 +87,20 @@ public static Map<String, List<String>> collapse(Map<String, List<String>> multi
}
if (CollectionUtils.isNotEmpty(single)) {
for (var entry: single.entrySet()) {
List<String> strings = values.computeIfAbsent(entry.getKey(), s -> new ArrayList<>());
if (!strings.contains(entry.getValue())) {
strings.add(entry.getValue());
String headerName = entry.getKey();
List<String> strings = values.computeIfAbsent(headerName, s -> new ArrayList<>());
String value = entry.getValue();
if (HEADERS_ALLOWING_COMMAS.contains(headerName)) {
if (!strings.contains(value)) {
strings.add(value);
}
} else {
for (String v : splitCommaSeparatedValue(value)) {
v = v.trim();
if (!strings.contains(v)) {
strings.add(v);
}
}
}
}
}
Expand All @@ -103,4 +120,12 @@ public static Map<String, String> collapse(Map<String, List<String>> input) {
}
return result;
}

@NonNull
private static List<String> splitCommaSeparatedValue(@Nullable String value) {
if (value == null) {
return Collections.emptyList();
}
return Arrays.asList(value.split(","));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package io.micronaut.function.aws.proxy

import com.amazonaws.services.lambda.runtime.events.APIGatewayProxyRequestEvent
import com.amazonaws.services.lambda.runtime.events.APIGatewayV2HTTPEvent
import io.micronaut.core.convert.ConversionService
import io.micronaut.http.CaseInsensitiveMutableHttpHeaders
import io.micronaut.http.HttpHeaders
import io.micronaut.http.MutableHttpHeaders
import io.micronaut.test.extensions.spock.annotation.MicronautTest
import jakarta.inject.Inject
import spock.lang.Specification

@MicronautTest
class MapCollapseUtilsSpec extends Specification {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The examples Foo and Key in this test are taken directly from the AWS Lambda Console. I did request and logged them to see how they are represented in the Lambda Event Objects.


@Inject
ConversionService conversionService

/**
*
* GET /prod/ HTTP/1.1
* Key: value1,value2,value3
* Foo: Bar
*/
void "payload v2 example"() {
given:
APIGatewayV2HTTPEvent event = new APIGatewayV2HTTPEvent()
Map<String, String> headers = new HashMap<>();
headers.put(HttpHeaders.DATE, "Wed, 21 Oct 2015 07:28:00 GMT")
headers.put("foo", "Bar")
headers.put("key", "value1,value2,value3")
headers.put(HttpHeaders.USER_AGENT, "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko)")
event.setHeaders(headers)

when:
MutableHttpHeaders httpHeaders = new CaseInsensitiveMutableHttpHeaders(MapCollapseUtils.collapse(Collections.emptyMap(), event.getHeaders()), conversionService);

then:
noExceptionThrown()

and:
"Bar" == httpHeaders.get("Foo")
["value1", "value2", "value3"] == httpHeaders.getAll("Key")
"Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko)" == httpHeaders.get(HttpHeaders.USER_AGENT)
"Wed, 21 Oct 2015 07:28:00 GMT" == httpHeaders.get(HttpHeaders.DATE)
}

/**
*
* GET /prod/ HTTP/1.1
* Key: value1,value2,value3
* Foo: Bar
*/
void "payload v1 example"() {
given:
APIGatewayProxyRequestEvent request = new APIGatewayProxyRequestEvent()
Map<String, String> headers = new HashMap<>()
headers.put("Foo", "Bar")
headers.put("Key", "value1,value2,value3")
request.setHeaders(headers)
Map<String, List<String>> multiValueHeaders = new HashMap<>()
multiValueHeaders.put("Key", Arrays.asList("value1", "value2", "value3"))
multiValueHeaders.put("Foo", Collections.singletonList("Bar"))
request.setMultiValueHeaders(multiValueHeaders)

when:
MutableHttpHeaders httpHeaders = new CaseInsensitiveMutableHttpHeaders(MapCollapseUtils.collapse(request.getMultiValueHeaders(), request.getHeaders()), conversionService);

then:
noExceptionThrown()

and:
"Bar" == httpHeaders.get("Foo")
["value1", "value2", "value3"] == httpHeaders.getAll("Key")

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
*/
@Internal
public final class APIGatewayV2HTTPEventFactory {
private final static String COMMA = ",";

private APIGatewayV2HTTPEventFactory() {
}
Expand All @@ -44,7 +45,7 @@ public static APIGatewayV2HTTPEvent create(HttpRequest<?> request, JsonMapper js
public Map<String, String> getHeaders() {
Map<String, String> result = new HashMap<>();
for (String headerName : request.getHeaders().names()) {
result.put(headerName, request.getHeaders().get(headerName));
result.put(headerName, String.join(COMMA, request.getHeaders().getAll(headerName)));
}
return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
"io.micronaut.http.server.tck.lambda.tests"
})
@ExcludeClassNamePatterns({
"io.micronaut.http.server.tck.tests.HeadersTest", // https://github.com/micronaut-projects/micronaut-aws/issues/1861
"io.micronaut.http.server.tck.tests.FilterProxyTest" // Immmutable request
})
@SuiteDisplayName("HTTP Server TCK for Function AWS API Gateway Proxy v2 Event model")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
"io.micronaut.http.server.tck.lambda.tests"
})
@ExcludeClassNamePatterns({
"io.micronaut.http.server.tck.tests.HeadersTest", // https://github.com/micronaut-projects/micronaut-aws/issues/1861
"io.micronaut.http.server.tck.tests.LocalErrorReadingBodyTest", // Binding body different type (e.g. a String in error handler)
"io.micronaut.http.server.tck.tests.FilterProxyTest" // Immmutable request

Expand Down