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

Request Parameters not picked up for v1 payload format with multivalue headers enabled #1256

Closed
erikpragt-connectid opened this issue Jan 22, 2025 · 6 comments
Assignees
Labels

Comments

@erikpragt-connectid
Copy link

Serverless Java Container version: 2.1.1
Implementations: Spring Boot 3
Framework version: SpringBoot 3.4.0
Frontend service: ALB / Function URL
Deployment method: CDK

Similar to the issue here #1229, when we enable multivalue headers in target group, no request parameters are picked up.

When we enable multivalue headers, the headers aren't populated like:

    "headers": {
        "accept": "*/*",
        "custom-header": "TestHeader",
        "host": "api.internal.dev.connectid.com.au",
        "user-agent": "curl/8.7.1",
        "x-amzn-trace-id": "Root=1-677e191b-732189535eb8827449a7fda4",
        "x-forwarded-for": "10.10.1.49",
        "x-forwarded-port": "443",
        "x-forwarded-proto": "https"
    },

Instead, they are populated in a multiValueQueryStringParameters block, like this:

    "multiValueQueryStringParameters": {
        "eventName": [
            "myEvent"
        ]
    },

The code in AwsSpringHttpProcessingUtils, especially the generateRequest1 method, doesn't use the multiValueQueryStringParameters, but it's using the following code:

populateQueryStringparameters(v1Request.getQueryStringParameters(), httpRequest);

It would be great if this could be resolved.

@erikpragt-connectid
Copy link
Author

erikpragt-connectid commented Jan 22, 2025

I thought perhaps it can be solved with this change:

	@SuppressWarnings({ "unchecked", "rawtypes" })
	private static HttpServletRequest generateRequest1(String request, Context lambdaContext,
			SecurityContextWriter securityWriter, ObjectMapper mapper, ServletContext servletContext) {
		AwsProxyRequest v1Request = readValue(request, AwsProxyRequest.class, mapper);
		
		ServerlessHttpServletRequest httpRequest = new ServerlessHttpServletRequest(servletContext, v1Request.getHttpMethod(), v1Request.getPath());

		if(v1Request.getMultiValueQueryStringParameters() != null) {
			populateQueryStringParameters1(v1Request.getMultiValueQueryStringParameters(), httpRequest);
		}

		if (v1Request.getMultiValueHeaders() != null) {
			MultiValueMapAdapter headers = new MultiValueMapAdapter(v1Request.getMultiValueHeaders());
			httpRequest.setHeaders(headers);
		}
		if (StringUtils.hasText(v1Request.getBody())) {
			httpRequest.setContentType("application/json");
			httpRequest.setContent(v1Request.getBody().getBytes(StandardCharsets.UTF_8));
		}
		if (v1Request.getRequestContext() != null) {
			httpRequest.setAttribute(RequestReader.API_GATEWAY_CONTEXT_PROPERTY, v1Request.getRequestContext());
			httpRequest.setAttribute(RequestReader.ALB_CONTEXT_PROPERTY, v1Request.getRequestContext().getElb());
		}
		httpRequest.setAttribute(RequestReader.API_GATEWAY_STAGE_VARS_PROPERTY, v1Request.getStageVariables());
		httpRequest.setAttribute(RequestReader.API_GATEWAY_EVENT_PROPERTY, v1Request);
		httpRequest.setAttribute(RequestReader.LAMBDA_CONTEXT_PROPERTY, lambdaContext);
		httpRequest.setAttribute(RequestReader.JAX_SECURITY_CONTEXT_PROPERTY,
				securityWriter.writeSecurityContext(v1Request, lambdaContext));
		return httpRequest;
	}

and

	private static void populateQueryStringParameters1(MultiValuedTreeMap<String, String> requestParameters, ServerlessHttpServletRequest httpRequest) {
		if (!CollectionUtils.isEmpty(requestParameters)) {
			for (Entry<String, List<String>> entry : requestParameters.entrySet()) {
				String[] array = entry.getValue().toArray(new String[0]);
				httpRequest.setParameter(entry.getKey(), array);
			}
		}
	}

But I'm not 100% if this is the correct approach.

@deki deki added the bug label Jan 22, 2025
deki added a commit that referenced this issue Jan 29, 2025
deki added a commit that referenced this issue Jan 29, 2025
…ivalue headers enabled - removed unused imports (#1256)
@deki
Copy link
Collaborator

deki commented Jan 29, 2025

Thanks for the bug report. I pushed a fix but noticed the test coverage is insufficient. These behaviors were already implemented for the old request mapping in https://github.com/aws/serverless-java-container/blob/main/aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsProxyHttpServletRequestTest.java and both logic and tests should be carried over to the new Spring-specific part.

@erikpragt-connectid
Copy link
Author

Hi @deki , thanks for the quick fix on that!

Any chance we could get an updated version of this? We've currently moved our request parameters to header values, but it's not great. Even a snapshot version would help.

@deki
Copy link
Collaborator

deki commented Jan 31, 2025

Sure, will provide you a 2.1.2 release within the next days.

deki added a commit that referenced this issue Feb 4, 2025
deki added a commit that referenced this issue Feb 4, 2025
@deki deki self-assigned this Feb 4, 2025
@deki
Copy link
Collaborator

deki commented Feb 4, 2025

2.1.2 is released. Created #1278 for test coverage and additional related fixes.

@deki deki closed this as completed Feb 4, 2025
@erikpragt-connectid
Copy link
Author

Awesome! Thanks again @deki for the quick action on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants