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

Multiple rewrites not supported #39

Closed
tgrassl opened this issue Aug 30, 2021 · 4 comments
Closed

Multiple rewrites not supported #39

tgrassl opened this issue Aug 30, 2021 · 4 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@tgrassl
Copy link

tgrassl commented Aug 30, 2021

Hi,

There's an error in the rewritePath method. It already returns after the first replacement and thus never checks other rewrites.

Example:
Wrong: rewritePath('/api/test/xy/', { '/$': '', '^/api/test/': '' }) -> /api/test/xy
Correct: rewritePath('/api/test/xy/', { '/$': '', '^/api/test/': '' }) -> xy

export const rewritePath = (
  url: string,
  pathRewrite: { [key: string]: string }
) => {
  for (let patternStr in pathRewrite) {
    const pattern = RegExp(patternStr);
    const path = pathRewrite[patternStr];
    if (pattern.test(url as string)) {
      return url.replace(pattern, path); <--- returns to early
    }
  }
  return url;
};

possible fix:

export const rewritePath = (url: string, pathRewrite: { [key: string]: string }): string => {
  let rewrittenUrl = url;
  for (let patternStr in pathRewrite) {
    const pattern = new RegExp(patternStr);
    const path = pathRewrite[patternStr];
    if (pattern.test(url as string)) {
      rewrittenUrl = rewrittenUrl.replace(pattern, path);
    }
  }
  return rewrittenUrl;
};
@stegano stegano self-assigned this Sep 1, 2021
@stegano stegano added the good first issue Good for newcomers label Sep 1, 2021
@stegano
Copy link
Owner

stegano commented Sep 1, 2021

Hi @tgrassl
Thanks for your reporting

Cases like the above issue can happen enough, and I agree.

In the above case, since pathRewrite is an object(map) type that does not guarantee the order, different results may be produced depending on the nodejs version or execution environment.

To solve the above issue, in the next version, we plan to modify the rewritePath argument value to be passed as an object or an array so that the entered rule can be applied first.

Thanks 😄

@stegano
Copy link
Owner

stegano commented Sep 1, 2021

@all-contributors please add @tgrassl for bug

@allcontributors
Copy link
Contributor

@stegano

I've put up a pull request to add @tgrassl! 🎉

@stegano
Copy link
Owner

stegano commented Sep 1, 2021

I just released a package.

You can now use it like this(Pattern checks are run in the order they are entered into the array first.)

// pages/[...all].ts
...
export default (req: NextApiRequest, res: NextApiResponse) => (
  isDevelopment
    ? httpProxyMiddleware(req, res, {
      // You can use the `http-proxy` option
      target: 'https://www.example.com',
      // In addition, you can use the `pathRewrite` option provided by `next-http-proxy-middleware`
      pathRewrite: [{
        patternStr: '^/api/new',
        replaceStr: '/v2'
      }, {
        patternStr: '^/api',
        replaceStr: ''
      }],
    })
    : res.status(404).send(null)
);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants