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

Odd patterns #335

Open
soletan opened this issue Oct 4, 2024 · 0 comments
Open

Odd patterns #335

soletan opened this issue Oct 4, 2024 · 0 comments

Comments

@soletan
Copy link

soletan commented Oct 4, 2024

I'm using path-to-regexp for processing routing configurations as part of a framework. The compiled pattern seems to be odd based on options end and trailing.

If I get things right,

  • setting end option to false is meant to create a regexp that's accepting paths starting with provided pattern. They may contain additional stuff.
  • setting trailing option to false does not require a delimiter directly following the matching part of provided pattern nor does it fail if a delimiter is found nonetheless in case end is false, too.

Based on that,

  • providing false for both options and pattern / creates a regexp /^(?:\/)(?=\/|$)/ which is matching either / or anything starting with //. It isn't matching /foo. I would expect /foo to match as I explicitly set end to false. OTOH, I wouldn't expect // to match as it isn't a valid path.

    expect( pathToRegExp( '/', { end: false, trailing: false } ).regexp.test( "/foo" ) ).toBeTrue();
    expect( pathToRegExp( '/', { end: false, trailing: false } ).regexp.test( "//" ) ).toBeFalse();
  • providing false for both options and pattern /foo, the resulting regexp is /^(?:\/foo)(?:\/$)?(?=\/|$)/ which is matching /foo, but requires a delimiter following that pattern in case additional characters are found in a tested path which feels counterintuitive to having trailing set false.

    expect( pathToRegExp( '/foo', { end: false, trailing: false } ).regexp.test( "/food" ) ).toBeTrue();

    However, I might be misreading the intention of trailing option here.

  • when e.g. processing path names read from configurations of a framework based on path-to-regexp, I have to strip off trailing delimiters found in patterns to be converted with pathToRegexp() as a precaution to prevent broken patterns unless end option is true. For example, when I get /foo/ to be converted with end option set false, /foo/bar isn't matching the resulting regexp /^(?:\/foo\/)(?=\/|$)/ but /foo//bar/ is.

    expect( pathToRegExp( '/foo/', { end: false, trailing: false } ).regexp.test( "/foo/bar" ) ).toBeTrue();
    expect( pathToRegExp( '/foo/', { end: false, trailing: false } ).regexp.test( "/foo//bar" ) ).toBeFalse();

    If trailing option is true in this scenario. the resulting regexp is /^(?:\/foo\/)(?:\/$)?(?=\/|$)/ which is accepting /foo// in addition to /foo//bar.

    expect( pathToRegExp( '/foo/', { end: false, trailing: true } ).regexp.test( "/foo//" ) ).toBeFalse();

Of course, dealing with trailing delimiters found in provided input pattern may be something a consuming application is in charge of. This might circumvent all the issues described here. Passing empty string as input resulting from stripping off "trailing" slashes in / feels odd, though. Eventually, having an according statement in the docs regarding such a prerequisite would be preferrable at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant