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

Lambda proxy is routing and passing decoded path variable when it should be encoded #656

Closed
jhonatanhulse opened this issue Oct 27, 2020 · 5 comments · Fixed by micronaut-projects/micronaut-core#4464

Comments

@jhonatanhulse
Copy link

jhonatanhulse commented Oct 27, 2020

Context:

Let's say we have the following user class with its respective controller:

package com.example

import groovy.transform.CompileStatic
import io.micronaut.core.annotation.Introspected

@CompileStatic
@Introspected
class User {

    String email
}
package com.example

import groovy.transform.CompileStatic
import io.micronaut.http.annotation.Controller
import io.micronaut.http.annotation.Get
import io.micronaut.http.annotation.PathVariable

@Controller('/users')
@CompileStatic
class UserController {

    Set<User> users = [
        new User(email: '[email protected]')
    ].toSet()

    @Get('/{+email}')
    User show(@PathVariable String email) {
        users.find { it.email == email }
    }
}

If we run the application purely without using micronaut-aws and its Lambda proxy, the UserController.show action will only find the user [email protected] if we encode the email. If we don't encode the email Micronaut will interpret the plus sign (+) as a whitespace (because it automatically decodes the path variable), therefore the user won't be found.

That means:

  1. GET - /users/email%2Balias%40gmail.com works as expected, user will be found
  2. GET - /users/[email protected] doesn't work, user won't be found (Micronaut will interpret the path parameter as email [email protected])

So far everything looks good, we just need to make sure emails are encoded when making the request

Problem:

Once you add the Lambda proxy, as explained in this guide https://guides.micronaut.io/mn-application-aws-lambda-java11-groovy/guide/index.html, the issue starts to happen. The following test for example will fail when it shouldn't:

package com.example

import com.amazonaws.serverless.proxy.internal.testutils.AwsProxyRequestBuilder
import com.amazonaws.serverless.proxy.internal.testutils.MockLambdaContext
import com.amazonaws.serverless.proxy.model.AwsProxyRequest
import com.amazonaws.serverless.proxy.model.AwsProxyResponse
import com.amazonaws.services.lambda.runtime.Context
import io.micronaut.function.aws.proxy.MicronautLambdaHandler
import io.micronaut.http.HttpMethod
import io.micronaut.http.HttpStatus
import spock.lang.AutoCleanup
import spock.lang.Shared
import spock.lang.Specification

import java.nio.charset.StandardCharsets

class UserControllerSpec extends Specification {

    @Shared
    @AutoCleanup
    MicronautLambdaHandler handler = new MicronautLambdaHandler()

    @Shared
    Context lambdaContext = new MockLambdaContext()

    void 'show should find the user'() {
        given:
        String email = '[email protected]'
        String encoded = URLEncoder.encode(email, StandardCharsets.UTF_8.toString())
        String path = "/users/${encoded}"

        when:
        AwsProxyRequest request = new AwsProxyRequestBuilder(path, HttpMethod.GET.toString()).build()
        AwsProxyResponse response = handler.handleRequest(request, lambdaContext)

        then:
        response.statusCode == HttpStatus.OK.code
    }
}

response.statusCode will be 404 when it should be 200.

image
image
image

We can also confirm that there is a bug in there when we encode the email twice like this:

package com.example

import com.amazonaws.serverless.proxy.internal.testutils.AwsProxyRequestBuilder
import com.amazonaws.serverless.proxy.internal.testutils.MockLambdaContext
import com.amazonaws.serverless.proxy.model.AwsProxyRequest
import com.amazonaws.serverless.proxy.model.AwsProxyResponse
import com.amazonaws.services.lambda.runtime.Context
import io.micronaut.function.aws.proxy.MicronautLambdaHandler
import io.micronaut.http.HttpMethod
import io.micronaut.http.HttpStatus
import spock.lang.AutoCleanup
import spock.lang.Shared
import spock.lang.Specification

import java.nio.charset.StandardCharsets

class UserControllerSpec extends Specification {

    @Shared
    @AutoCleanup
    MicronautLambdaHandler handler = new MicronautLambdaHandler()

    @Shared
    Context lambdaContext = new MockLambdaContext()

    void 'show should find the user'() {
        given:
        String email = '[email protected]'
        String encoded = URLEncoder.encode(email, StandardCharsets.UTF_8.toString())
        String encodedTwice = URLEncoder.encode(encoded, StandardCharsets.UTF_8.toString())
        String path = "/users/${encodedTwice}"

        when:
        AwsProxyRequest request = new AwsProxyRequestBuilder(path, HttpMethod.GET.toString()).build()
        AwsProxyResponse response = handler.handleRequest(request, lambdaContext)

        then:
        response.statusCode == HttpStatus.OK.code
    }
}

Now the test passes and the user is found.

Another thing to note is that if we change the route mapping in the UserController like this:

...
class UserControllerSpec extends Specification {
    ...
    @Get('/{email}') // plus sign removed
    User show(@PathVariable String email) {
    ....

The route won't even match, which makes me believe the problem is happening when routing and also when passing URL parameters.

Application example:

demo.zip

@gregorydickson
Copy link

+1

@ttzn
Copy link
Contributor

ttzn commented Nov 10, 2020

I had the exact same issue. Variable values are obviously decoded twice in the context of a Lambda handler, which uses MicronautAwsProxyRequest. This does not happen with NettyHttpRequest. Here is a bare-bones test case using @jhullse 's model and controller:

    void 'variables should be properly decoded for all HttpRequest types'() {
        given:
        NettyHttpServer embeddedServer = ApplicationContext.run(NettyHttpServer)
        String email = '[email protected]'
        String encoded = URLEncoder.encode(email, StandardCharsets.UTF_8.toString())
        String path = "/users/${encoded}"

        when:
        def lambdaContext = new MockLambdaContext()
        def awsProxyRequest = new AwsProxyRequestBuilder(path, HttpMethod.GET.toString()).build()
        HttpRequest<String> awsHttpRequest = new MicronautAwsProxyRequest<String>(path,
                awsProxyRequest,
                null,
                lambdaContext,
                null)
        Router router = embeddedServer.getApplicationContext().getBean(Router.class)
        UriRouteMatch<?, ?> awsMatch = router.find(awsHttpRequest).findFirst().get()

        def nettyRequest = new DefaultHttpRequest(HttpVersion.HTTP_1_1,
                io.netty.handler.codec.http.HttpMethod.GET, path)
        HttpRequest<String> nettyBasedHttpRequest = new NettyHttpRequest<>(nettyRequest,
                Mock(ChannelHandlerContext),
                embeddedServer.getEnvironment(),
                embeddedServer.getServerConfiguration())
        UriRouteMatch<?, ?> nettyMatch = router.find(nettyBasedHttpRequest).findFirst().get()

        then:
        awsMatch.getVariableValues() == nettyMatch.getVariableValues()
    }

It seems the root cause is that MicronautAwsProxyRequest's getPath method, which is called when building the route matching information, is based on the default HttpRequest implementation which returns getUri().getPath(), when it really should be doing getUri().getRawPath(). NettyHttpRequest does preserve the encoded URI.

I believe that fixing this requires an answer to the question: what is the actual contract of HttpRequest#getPath() ?

@graemerocher
Copy link
Contributor

Yes it should be using getRawPath() it seems

PRs welcome!

@ttzn
Copy link
Contributor

ttzn commented Nov 10, 2020

@graemerocher do you think we should just override the method in MicronautAwsProxyRequest, or we can safely patch the default impl all the way up in micronaut-http ? I don't know how much code might depend on that.

@AndrejMitrovic
Copy link

AndrejMitrovic commented Dec 9, 2022

Hi,

Sorry to bump an old issue. We're having a similar problem to this one but related to query parameters, when used with the AWS API Gateway. Logging has validated that the lambda routine receives a payload with a + in the queryStringParameters attribute of the JSON object which the API Gateway provides. But then our Controller receives a space character instead.

micronaut-projects/micronaut-core#4464 seems to fix the problem with path segments, but our specific problem is with query parameters.

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

Successfully merging a pull request may close this issue.

5 participants