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

Wrong handling of paths without params #11

Closed
asulykos opened this issue Jun 10, 2020 · 4 comments · Fixed by markwylde/simple-react-app#6 · 4 remaining pull requests
Closed

Wrong handling of paths without params #11

asulykos opened this issue Jun 10, 2020 · 4 comments · Fixed by markwylde/simple-react-app#6 · 4 remaining pull requests
Labels

Comments

@asulykos
Copy link

Rewrite rules having no parameters, automatically applied to every path (even to the non-matching ones.)

The problem is in getTargetUrl function of util.js.

The toUrl is automatically set to the to parameter, and will be returned even if the path doesn't match. For non-matching paths the intact url parameter should be returned instead.

function getTargetUrl (from, to, url) {
  const { pathToRegexp } = require('path-to-regexp')
  const fromParams = []
  const re = pathToRegexp(from, fromParams)
  const fromMatches = re.exec(url)
  let toUrl = to // <--------- it should be 'url' if 'fromMatches' is undefined

  for (const [index, fromParam] of fromParams.entries()) {
    if (fromMatches && fromMatches[index + 1]) {
      fromParam.value = fromMatches[index + 1]
    }
  }

  /* replace named params */
  for (const fromParam of fromParams) {
    if (typeof fromParam.name === 'string') {
      if (fromParam.value) {
        toUrl = toUrl.replace(new RegExp(`:${fromParam.name}`, 'g'), fromParam.value)
      } else {
        toUrl = url
      }
    }
  }

  /* replace positional params */
  for (const fromParam of fromParams) {
    if (typeof fromParam.name !== 'string') {
      toUrl = url.replace(re, toUrl)
    }
  }

  return toUrl
}
@75lb
Copy link
Member

75lb commented Jun 11, 2020

Hi, could you post an example of a rewrite rule I can use to reproduce this bug please?

@asulykos
Copy link
Author

asulykos commented Jun 11, 2020

Let's consider the following rules:

rewrite: [
  {
    from: '/login/(.*)',
    to: 'dist/login/$1'
  },
  {
    from: '/login',
    to: 'dist/login/index.html'
  }
]

Here with the first rule, I'd like to rewrite every path starting with /login/ to my dist folder path. This rule alone works, but as soon as I add the second one, everything will be redirected to 'dist/login/index.html'. The second rule doesn't have params to be replaced. getTargetUrl() will return the unmodified 'toUrl' which has been set to 'to' at the beginning of the function. (This 'to' variable holds 'dist/login/index.html'.)

@75lb 75lb closed this as completed in a38c90d Jun 11, 2020
@75lb
Copy link
Member

75lb commented Jun 11, 2020

Fixed and released in lws-rewrite v3.1.1 - please upgrade.

Thanks, let me know if you find any more issues!

75lb added a commit to lwsjs/local-web-server that referenced this issue Jun 11, 2020
@75lb 75lb added the bug label Jun 11, 2020
@asulykos
Copy link
Author

Thank you very much, that was really quick. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment