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

requests to /foo/bar to an API Gateway (v1) resource of /foo/{proxy+} return 404 as they are routed to /bar #648

Open
eran-medan opened this issue Apr 29, 2023 · 7 comments

Comments

@eran-medan
Copy link

eran-medan commented Apr 29, 2023

I'm having the same issue as in this comment #454 (comment) (should be probably reopened?)

Upgraded from the legacy aws-serverless-express and for some reason it takes the path from pathParameters.proxy (which is bar) and not the request.path (which is /foo/bar)

I'll be happy to work on a PR if you indeed confirm it's a bug

@pocketcowboy
Copy link

Offending line is https://github.com/vendia/serverless-express/blob/0909ec4ffd09d9fd22538257b721b7d076d03732/src/event-sources/utils.js#LL7C21-L7C21

A quick workaround is to change the name of the greedy path variable in API Gateway e.g. rename {proxy+} to {route+}.

@mohammedsahl
Copy link

Facing the same issue as well. My team wasn't keen on messing with the greedy path variable names since that would entail a relatively uncomfortable change in API Gateway. Our routes are well established and running well.

Another quick workaround is to delete event.pathParameters.proxy if it exists and pass the edited event into serverlessExpress. This way util.js is forced to use event.path.

@hisham
Copy link

hisham commented May 25, 2023

To add to @mohammedsahl comment - vendia v4 does not seem to be compatible with how AWS Amplify sets up REST endpoints in API Gateway (https://docs.amplify.aws/cli/restapi/restapi/#create-a-rest-api). Since AWS Amplify allows for varying auth levels per endpoint, and the auth checks are done by API Gateway, it uses /foo/{proxy+} rather than /{proxy+}.

To make this lib work with amplify, you have to do what @mohammedsahl describes in #648 (comment) or similar.

@brett-vendia
Copy link
Contributor

Can you try the v5 branch #649? We'll merge this after additional verification

@mohammedsahl
Copy link

Hey @brett-vendia. Unfortunately, it still doesn't seem to work. This event.multiValueQueryStringParameters in our event is null and so the params passed to to url.format() are { pathname: '/<somePathParamter>', query: null }

If it helps our event has the following structure

{
  "resource": "/foo/{proxy+}",
  "path": "/foo/<PathParamter>",
  ...
  "queryStringParameters": null,
  "multiValueQueryStringParameters": null,
  "pathParameters": {
    "proxy": "<PathParamter>"
  },
  ...
}

@ddewaele
Copy link

ddewaele commented Jun 21, 2024

any updates on this ?

It's a bit unclear why paths are treated so differently.

API GW Resource API call NestJS incoming path NestJS path logic
/api/{proxy+} /api/dynamic dynamic pathParameters.proxy
/api/static /api/static /api/static path
/{proxy+} /api/dynamic /api/dynamic pathParameters.proxy

Given the 2 following events that API GW sends to Lambda proxy integration perhaps an option to prefer the path property over or the proxy property could be considered if people prefer this type of consistency.

Would happy to offer a PR for this if this is a direction you are interested in.

"resource": "/api/{proxy+}",
"path": "/api/dynamic",
"pathParameters": {
  "proxy": "dynamic"
},
"requestContext": {
  "resourcePath": "/api/{proxy+}",
  "path": "/prod/api/dynamic",
}

vs

"resource": "/api/static",
"path": "/api/static",
"pathParameters": null,
"requestContext": {
  "resourcePath": "/api/static",
  "path": "/prod/api/static",
}

I also don't think proxy is reserved or special keyword. As others pointed out, if could just as well be called {route+}.
So it's probably best avoided in code.

@shrinarpatvfx
Copy link

shrinarpatvfx commented Jul 9, 2024

Facing the same issue in July 2024 - Do we have found any solution for this bug?

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

No branches or pull requests

7 participants