path-to-regexp icon indicating copy to clipboard operation
path-to-regexp copied to clipboard

Global route syntax causes parse error

Open zone117x opened this issue 4 years ago • 5 comments

I ran into this problem while using path-to-regex in my own app, and was able to reproduce it in the unit tests in this repo.

For example, when using the route:

  app.get('/test/*', (req, res) => {
    res.sendStatus(200);
  });

The parsing error is thrown:

  ● path-to-regexp › rules › '/test/*' › encountered a declaration exception

    TypeError: Unexpected MODIFIER at 6, expected END

      155 |     if (value !== undefined) return value;
      156 |     const { type: nextType, index } = tokens[i];
    > 157 |     throw new TypeError(`Unexpected ${nextType} at ${index}, expected ${type}`);
          |           ^
      158 |   };
      159 | 
      160 |   const consumeText = (): string => {

      at mustConsume (src/index.ts:157:11)
      at parse (src/index.ts:228:5)
      at stringToRegexp (src/index.ts:495:25)
      at Object.pathToRegexp (src/index.ts:617:10)
      at Suite.<anonymous> (src/index.spec.ts:2804:33)
      at src/index.spec.ts:2802:7
          at Array.forEach (<anonymous>)
      at Suite.<anonymous> (src/index.spec.ts:2799:11)
      at Suite.<anonymous> (src/index.spec.ts:2798:3)
      at Object.<anonymous> (src/index.spec.ts:2705:1)

See Express documentation for global/wildcard route definitions here: https://expressjs.com/en/4x/api.html#router.methods

A PR was opened here to demo the bug: https://github.com/pillarjs/path-to-regexp/pull/245

zone117x avatar Mar 16 '21 16:03 zone117x

The wildcard is not a supported feature for the version you’re testing. Are you sure it’s the same error as express? Express uses the 0.1.x branch of code, so a test on master isn’t related. If it’s from express, have you done something to force a more recent version of this package by changing resolution somewhere?

blakeembrey avatar Mar 16 '21 16:03 blakeembrey

See also: https://github.com/pillarjs/path-to-regexp#compatibility-with-express--4x

blakeembrey avatar Mar 16 '21 16:03 blakeembrey

Thanks for the info. I didn't see the compatibility docs. I'm using these routes in an Express v4.x app, and was expecting this library to mirror the functionality.

I see now that this wildcard route is not supported here. Curious if you have any recommendations for me -- should I use the Express v5-alpha, or avoid using route syntax that is incompatible with this lib?

Thanks for the quick response ❤️

zone117x avatar Mar 16 '21 16:03 zone117x

Also, has Express stated this behavior is a bug, or only this library?

For example, in their docs they show this route:

router.all('/api/*', requireAuthentication)

I'd be interested in some kind of way to "opt-in" to Express v4.x compatible parsing, via a config option or otherwise. I looked into trying to support this parsing in the code for a possible PR but got lost. Any tips?

zone117x avatar Mar 16 '21 16:03 zone117x

This isn't a bug, just a feature change. It might be flagged somewhere, and it's something that could be added back. Opting in to express 4.x compatible parsing probably wouldn't be possible though, there's some really iffy behavior such as regexp characters being valid anywhere in the string that would cause problems.

blakeembrey avatar Mar 17 '21 02:03 blakeembrey