serverless-express icon indicating copy to clipboard operation
serverless-express copied to clipboard

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

Open eran-medan opened this issue 2 years ago • 8 comments

I'm having the same issue as in this comment https://github.com/vendia/serverless-express/issues/454#issuecomment-1322898350 (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

eran-medan avatar Apr 29 '23 21:04 eran-medan

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+}.

pocketcowboy avatar May 06 '23 07:05 pocketcowboy

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.

mohammedsahl avatar May 25 '23 14:05 mohammedsahl

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 https://github.com/vendia/serverless-express/issues/648#issuecomment-1563045194 or similar.

hisham avatar May 25 '23 17:05 hisham

Can you try the v5 branch https://github.com/vendia/serverless-express/pull/649? We'll merge this after additional verification

brett-vendia avatar May 25 '23 20:05 brett-vendia

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>"
  },
  ...
}

mohammedsahl avatar May 26 '23 16:05 mohammedsahl

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.

ddewaele avatar Jun 21 '24 11:06 ddewaele

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

shrinarpatvfx avatar Jul 09 '24 13:07 shrinarpatvfx