middyjs / middy

🛵 The stylish Node.js middleware engine for AWS Lambda 🛵
https://middy.js.org
MIT License
3.73k stars 377 forks source link

Http Router - multiple variables causes issues for the path in the regex #899

Closed yannickrocks closed 2 years ago

yannickrocks commented 2 years ago

Describe the bug In the Http-Router package, the following will not resolve correctly to create dyanmic routes

E.g. Imagine you have 2 function URL with the following paths:

The line of code path = path.replace(regexpDynamicWildcards, '/?.*').replace(regexpDynamicParameters, '/.+') will resolve both paths to /^\/varA\/.+$/

This is an issue as if you are trying to use the second path, it will use the first path's parameters which can cause issues further down the line.

THE FIX

change the following

OLD

const regexpDynamicParameters = /\/\{.+\}/g;

NEW

const regexpDynamicParameters = /\/\{[^\/]+}/g;

Regex helper: https://regex101.com/r/Xk1x6P/1

willfarrell commented 2 years ago

Would you like to put a PR together with a unit test for this?

yannickrocks commented 2 years ago

@willfarrell Working on it today.

We also have found a way to populate the path parameters if that is something people would be interested in.

yannickrocks commented 2 years ago

PR is here @willfarrell https://github.com/middyjs/middy/pull/901

willfarrell commented 2 years ago

We also have found a way to populate the path parameters if that is something people would be interested in.

Can you open a new issue for this?

When used with API Gateway the pathParameters are pre-parsed which is why it's not included currently. This also giving a perf boost compared to other routers. Are you using this with ALB or Function URLs? Would love to heard more about your use case.

yannickrocks commented 2 years ago

HI @willfarrell Sure thing, We are using Function URLs to allow lambdas to call other lambdas internally. :D