pattern string canonicalization with wildcards can result in different meanings #145

wanderview · 2021-10-26T20:40:21Z

Currently, when generating canonical pattern strings the algorithm always translates .* regex values into wildcards like *. Unfortunately, this can result in a change of meaning if it does not consider the values before the regexp group.

Consider, this:

(foo)(.*)

Gets "canonicalized" to:

(foo)*

If this is parsed again the * is treated as a modifier of (foo) instead of as a wildcard.

The algorithm should be fixed to look at the preceding part to determine if its safe to convert a .* regexp into a wildcard character.

The text was updated successfully, but these errors were encountered:

wanderview · 2021-10-26T20:47:43Z

Another way this can happen is if an intermediate empty group is removed. For example:

(foo){}*

Currently becomes:

(foo)*

But should be:

(foo)(.*)

wanderview · 2021-10-26T21:20:02Z

This can also produce pattern strings that don't even parse. Consider:

*{}**?

Becomes:

**?

This fuzzer exercises `Parse()` and `Pattern::GeneratePatternString()`. It also features a property check for idempotency, based on my expectation that canonical patterns are fixed points of the parse-then-canonicalize procedure. Locally, I'm already seeing a check failure on the input string "*\xcd*{}?*?". It canonicalizes to "*\xcd**?", which promptly fails to parse, giving the following status: Unexpected modifier '?' at index 4, expected end of pattern. It turns out that those 0xcd bytes are invalid UTF-8 sequences, and the documentation for `Parse()` states that its input must be valid UTF-8. In a followup CL, I'll make the tokenizer fail on invalid UTF-8 inputs, which means this fuzzer doesn't need to check that its input is a valid UTF-8 sequence before attempting a parse. There is also a larger bug here, which is that `GeneratePatternString()` can generate strings that are not parseable. This led wanderview@ to file a spec bug for urlpattern: whatwg/urlpattern#145 (comment). Bug: 1263594 Change-Id: I277b61a8fd36af4027bacde332c7081f543dac64 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3244979 Commit-Queue: Dan McArdle <dmcardle@chromium.org> Reviewed-by: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#935538}

wanderview · 2021-10-29T14:25:58Z

Another way generating strings can change the meaning... This:

{:k\\A}

Becomes:

{:kA}

Which changes from a named :k group with a suffix A to a single named :kA group.

wanderview · 2021-12-02T22:16:42Z

Another type of problem here with:

{:foo}(.*)

Assuming we fix this not to convert (.*) to *, we also need to avoid accidentally changing it to:

:foo(.*)

That changes the pattern from two groups to a single group with a custom name and wildcard regexp.

This CL fixes a particular issue in an overall class of problems described in: whatwg/urlpattern#145 This particular CL only addresses the case where a wildcard could mistakenly be interpreted as a modifier for a preceding group. For example, we should not change `(foo)(.*)` to `(foo)*`. There will be more follow-up CLs to fix other issues mentioned in the github issue. Bug: 1263673 Change-Id: I5c99aa6b1fe46cc5905d823ef8184375b832c92a

This CL fixes a particular issue in an overall class of problems described in: whatwg/urlpattern#145 This particular CL only addresses the case where a wildcard could mistakenly be interpreted as a modifier for a preceding group. For example, we should not change `(foo)(.*)` to `(foo)*`. There will be more follow-up CLs to fix other issues mentioned in the github issue. Bug: 1263673 Change-Id: I5c99aa6b1fe46cc5905d823ef8184375b832c92a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3313642 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#947747}

This CL fixes another problem case from: whatwg/urlpattern#145 In this case we need to preserve `{ ... }` braces around a named group followed by a group that may be emitted in regexp `( ... )` group. We don't want `{:foo}(.*)` to be changed to `:foo(.*)` since that changes the meaning from two groups to a single group. Bug: 1263673 Change-Id: Ifc073d7bfb10f437180393b34cc3bf3c869d7f68

wanderview · 2021-12-06T22:02:46Z

Another slight variation on the problem:

{:foo}bar

Becomes:

:foobar

This CL fixes another problem case from: whatwg/urlpattern#145 In this case we need to preserve `{ ... }` braces around a named group followed by a group that may be emitted in regexp `( ... )` group. We don't want `{:foo}(.*)` to be changed to `:foo(.*)` since that changes the meaning from two groups to a single group. Bug: 1263673 Change-Id: Ifc073d7bfb10f437180393b34cc3bf3c869d7f68

This CL fixes another problem case from: whatwg/urlpattern#145 In this case we need to preserve `{ ... }` braces around a named group followed by a group that may be emitted in regexp `( ... )` group. We don't want `{:foo}(.*)` to be changed to `:foo(.*)` since that changes the meaning from two groups to a single group. Bug: 1263673 Change-Id: Ifc073d7bfb10f437180393b34cc3bf3c869d7f68 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3315419 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#949705}

This CL fixes another problem case from: whatwg/urlpattern#145 In this case we need to detect when a group suffix could be mistaken for trailing custom name characters. When this happens we should escape the first character of the suffix. For example: `{:foo\\bar}` instead of `{:foobar}`. Bug: 1263673 Change-Id: I2aa2d043ef4c71433fdc0be113fcafddf5ad3532 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3318605 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#949707}

This CL fixes another problem case from: whatwg/urlpattern#145 In this case we need to preserve `{ ... }` braces around a named group followed by a group that may be emitted in regexp `( ... )` group. We don't want `{:foo}(.*)` to be changed to `:foo(.*)` since that changes the meaning from two groups to a single group. Bug: 1263673 Change-Id: Ifc073d7bfb10f437180393b34cc3bf3c869d7f68 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3315419 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#949705}

…#31882) This CL fixes another problem case from: whatwg/urlpattern#145 In this case we need to preserve `{ ... }` braces around a named group followed by a group that may be emitted in regexp `( ... )` group. We don't want `{:foo}(.*)` to be changed to `:foo(.*)` since that changes the meaning from two groups to a single group. Bug: 1263673 Change-Id: Ifc073d7bfb10f437180393b34cc3bf3c869d7f68 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3315419 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#949705} Co-authored-by: Ben Kelly <wanderview@chromium.org>

This CL fixes another problem case from: whatwg/urlpattern#145 In this case we need to detect when a group suffix could be mistaken for trailing custom name characters. When this happens we should escape the first character of the suffix. For example: `{:foo\\bar}` instead of `{:foobar}`. Bug: 1263673 Change-Id: I2aa2d043ef4c71433fdc0be113fcafddf5ad3532 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3318605 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#949707}

This CL adds tests for more cases raised in: whatwg/urlpattern#145 It turns out we don't need any further code changes to pass these test cases since the parser already removes empty groups. Bug: 1263673 Change-Id: I7b8da0da23e30780a09fc5132d36f4154b42b0db

This CL fixes another problem case from: whatwg/urlpattern#145 In this case we need to detect when fixed text following a `:foo` group could be mistaken as party of the name. When this happens we should emit `{ }` braces around the group to isolate it. For example, `{:foo}bar` instead of `:foobar`. Bug: 1263673 Change-Id: I9c9bfb736c6e8880bceeae9280f0fc30f794cea8

This fixes one of the cases outlined in #145. When we have a matching group following by a wildcard we must be careful not to emit `*` for the wildcard. We don't want it to be mistaken for modifier on the previous group. So we want to generate `(foo)(.*)` and not `(foo)*`. This change corresponds to this implementation change in chromium: https://chromium-review.googlesource.com/c/chromium/src/+/3313642

This change fixes another case from #145. Here we need protect a `:foo` group with `{}` braces if its followed by a matching group that may be expressed as `(bar)`. Without this protection the second group may be interpreted as a custom regexp for the first group. We want to generate `{:foo}(bar)` and not `:foo(bar)`. For the most part this change corresponds to this chromium commit: https://chromium-review.googlesource.com/c/chromium/src/+/3315419 Note, however, this spec change includes an additional fix to check for the next part being a segment wildcard. That additional check will need to be added to the implementation in another change.

This commit fixes all of the issues described in: whatwg/urlpattern#145 It corresponds to the following implementation CLs: https://chromium-review.googlesource.com/c/chromium/src/+/3313642 https://chromium-review.googlesource.com/c/chromium/src/+/3315419 https://chromium-review.googlesource.com/c/chromium/src/+/3318605 https://chromium-review.googlesource.com/c/chromium/src/+/3321520 https://chromium-review.googlesource.com/c/chromium/src/+/3328146 https://chromium-review.googlesource.com/c/chromium/src/+/3378528 This commit also includes a fix to path-to-regexp capture in this upstream branch commit: wanderview/path-to-regexp@d1d81d9

This commit fixes all of the issues described in: whatwg/urlpattern#145 It corresponds to the following implementation CLs: https://chromium-review.googlesource.com/c/chromium/src/+/3313642 https://chromium-review.googlesource.com/c/chromium/src/+/3315419 https://chromium-review.googlesource.com/c/chromium/src/+/3318605 https://chromium-review.googlesource.com/c/chromium/src/+/3321520 https://chromium-review.googlesource.com/c/chromium/src/+/3328146 https://chromium-review.googlesource.com/c/chromium/src/+/3378528 This commit also includes a fix to path-to-regexp capture in this upstream branch commit: wanderview/path-to-regexp@d1d81d9 Finally, this commit brings the polyfill tests up-to-date with the upstream WPT tests.

wanderview · 2022-01-20T21:21:53Z

Another one of these was found by the fuzzer:

https://bugs.chromium.org/p/chromium/issues/detail?id=1289115

In this case if an implicit prefix is before :foo group it can incorrectly prevent us from protecting from trailing fixed characters.

This fixes another issue related to: whatwg/urlpattern#145 In this case we were incorrectly looking at the prefix value when trying to determine if we should protect against characters following a `:foo` named group. We don't need the prefix check because we already have the `needs_grouping` check. With this check in place we would incorrectly allow following characters when there is an implicit prefix, like `/` in a pathname. Bug: 1289115 Change-Id: Id8a88b55a31c3a0e72310574fe4d7a6fc7781d68

This fixes another issue with generating canonical pattern strings as described in #145. In this case the prefix check incorrectly prevented us from protecting a named group from following character if there was an implicit prefix. For example, for a pathname `/:foo\\bar` we would incorrectly produce `/:foobar` instead of `{/:foo}bar`.

This fixes another issue related to: whatwg/urlpattern#145 In this case we were incorrectly looking at the prefix value when trying to determine if we should protect against characters following a `:foo` named group. We don't need the prefix check because we already have the `needs_grouping` check. With this check in place we would incorrectly allow following characters when there is an implicit prefix, like `/` in a pathname. Bug: 1289115 Change-Id: Id8a88b55a31c3a0e72310574fe4d7a6fc7781d68 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3405148 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#961663}

This fixes another issue related to: whatwg/urlpattern#145 In this case we were incorrectly looking at the prefix value when trying to determine if we should protect against characters following a `:foo` named group. We don't need the prefix check because we already have the `needs_grouping` check. With this check in place we would incorrectly allow following characters when there is an implicit prefix, like `/` in a pathname. Bug: 1289115 Change-Id: Id8a88b55a31c3a0e72310574fe4d7a6fc7781d68 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3405148 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#961663}

This fixes another issue related to: whatwg/urlpattern#145 In this case we were incorrectly looking at the prefix value when trying to determine if we should protect against characters following a `:foo` named group. We don't need the prefix check because we already have the `needs_grouping` check. With this check in place we would incorrectly allow following characters when there is an implicit prefix, like `/` in a pathname. Bug: 1289115 Change-Id: Id8a88b55a31c3a0e72310574fe4d7a6fc7781d68

This fixes another issue with generating canonical pattern strings as described in #145. In this case the prefix check incorrectly prevented us from protecting a named group from following character if there was an implicit prefix. For example, for a pathname `/:foo\\bar` we would incorrectly produce `/:foobar` instead of `{/:foo}bar`.

…receding non-prefix '/'., a=testonly Automatic update from web-platform-tests URLPattern: Emit '{ }' when there is a preceding non-prefix '/'. This CL fixes another problem where generating a pattern string accidentally changes the meaning of the pattern: whatwg/urlpattern#145 (comment) In this case we want to make sure that a character that can be treated as an implicit group prefix, e.g. `/`, is not incorrectly converted into a prefix. For example, if the original string is `/{:foo?}` we don't want to generate `/:foo?` since that would make the `/` optional. This CL also includes an additional fix that permits emitting `*` for wildcards when there is a prefix. Previously we were being too conservative in some situations. For example, we can emit `*/*` instead of `*/(.*)`. Fixed: 1285620 Change-Id: Idef288bdfdfaa0ae60f320ec326ee2f07d448681 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3378528 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#957730} -- wpt-commits: 4928cc8d516868267da4be9a5b9a5f7856dd0b65 wpt-pr: 32314

…n GeneratePatternString()., a=testonly Automatic update from web-platform-tests URLPattern: Fix erroneous prefix check in GeneratePatternString(). This fixes another issue related to: whatwg/urlpattern#145 In this case we were incorrectly looking at the prefix value when trying to determine if we should protect against characters following a `:foo` named group. We don't need the prefix check because we already have the `needs_grouping` check. With this check in place we would incorrectly allow following characters when there is an implicit prefix, like `/` in a pathname. Bug: 1289115 Change-Id: Id8a88b55a31c3a0e72310574fe4d7a6fc7781d68 -- wpt-commits: 1d7733d6c22d336244c2a4d7c0a9aaa3087f4490 wpt-pr: 32481

…receding non-prefix '/'., a=testonly Automatic update from web-platform-tests URLPattern: Emit '{ }' when there is a preceding non-prefix '/'. This CL fixes another problem where generating a pattern string accidentally changes the meaning of the pattern: whatwg/urlpattern#145 (comment) In this case we want to make sure that a character that can be treated as an implicit group prefix, e.g. `/`, is not incorrectly converted into a prefix. For example, if the original string is `/{:foo?}` we don't want to generate `/:foo?` since that would make the `/` optional. This CL also includes an additional fix that permits emitting `*` for wildcards when there is a prefix. Previously we were being too conservative in some situations. For example, we can emit `*/*` instead of `*/(.*)`. Fixed: 1285620 Change-Id: Idef288bdfdfaa0ae60f320ec326ee2f07d448681 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3378528 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#957730} -- wpt-commits: 4928cc8d516868267da4be9a5b9a5f7856dd0b65 wpt-pr: 32314

…n GeneratePatternString()., a=testonly Automatic update from web-platform-tests URLPattern: Fix erroneous prefix check in GeneratePatternString(). This fixes another issue related to: whatwg/urlpattern#145 In this case we were incorrectly looking at the prefix value when trying to determine if we should protect against characters following a `:foo` named group. We don't need the prefix check because we already have the `needs_grouping` check. With this check in place we would incorrectly allow following characters when there is an implicit prefix, like `/` in a pathname. Bug: 1289115 Change-Id: Id8a88b55a31c3a0e72310574fe4d7a6fc7781d68 -- wpt-commits: 1d7733d6c22d336244c2a4d7c0a9aaa3087f4490 wpt-pr: 32481

This fuzzer exercises `Parse()` and `Pattern::GeneratePatternString()`. It also features a property check for idempotency, based on my expectation that canonical patterns are fixed points of the parse-then-canonicalize procedure. Locally, I'm already seeing a check failure on the input string "*\xcd*{}?*?". It canonicalizes to "*\xcd**?", which promptly fails to parse, giving the following status: Unexpected modifier '?' at index 4, expected end of pattern. It turns out that those 0xcd bytes are invalid UTF-8 sequences, and the documentation for `Parse()` states that its input must be valid UTF-8. In a followup CL, I'll make the tokenizer fail on invalid UTF-8 inputs, which means this fuzzer doesn't need to check that its input is a valid UTF-8 sequence before attempting a parse. There is also a larger bug here, which is that `GeneratePatternString()` can generate strings that are not parseable. This led wanderview@ to file a spec bug for urlpattern: whatwg/urlpattern#145 (comment). Bug: 1263594 Change-Id: I277b61a8fd36af4027bacde332c7081f543dac64 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3244979 Commit-Queue: Dan McArdle <dmcardle@chromium.org> Reviewed-by: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#935538} NOKEYCHECK=True GitOrigin-RevId: f3f28fa666fa735b5eee62a070a372a14661d2cf

This CL fixes a particular issue in an overall class of problems described in: whatwg/urlpattern#145 This particular CL only addresses the case where a wildcard could mistakenly be interpreted as a modifier for a preceding group. For example, we should not change `(foo)(.*)` to `(foo)*`. There will be more follow-up CLs to fix other issues mentioned in the github issue. Bug: 1263673 Change-Id: I5c99aa6b1fe46cc5905d823ef8184375b832c92a Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3313642 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#947747} NOKEYCHECK=True GitOrigin-RevId: b55a4670af3ca1e1746c744aa8a212cd6acee02a

This CL fixes another problem case from: whatwg/urlpattern#145 In this case we need to preserve `{ ... }` braces around a named group followed by a group that may be emitted in regexp `( ... )` group. We don't want `{:foo}(.*)` to be changed to `:foo(.*)` since that changes the meaning from two groups to a single group. Bug: 1263673 Change-Id: Ifc073d7bfb10f437180393b34cc3bf3c869d7f68 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3315419 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#949705} NOKEYCHECK=True GitOrigin-RevId: 349f07c20945a1179230a296a33acf8404b418b9

This CL fixes another problem case from: whatwg/urlpattern#145 In this case we need to detect when a group suffix could be mistaken for trailing custom name characters. When this happens we should escape the first character of the suffix. For example: `{:foo\\bar}` instead of `{:foobar}`. Bug: 1263673 Change-Id: I2aa2d043ef4c71433fdc0be113fcafddf5ad3532 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3318605 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#949707} NOKEYCHECK=True GitOrigin-RevId: a3bf96bb292071926a248edc7520230d461bf5bc

This CL fixes another problem case from: whatwg/urlpattern#145 In this case we need to detect when fixed text following a `:foo` group could be mistaken as party of the name. When this happens we should emit `{ }` braces around the group to isolate it. For example, `{:foo}bar` instead of `:foobar`. Bug: 1263673 Change-Id: I9c9bfb736c6e8880bceeae9280f0fc30f794cea8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3321520 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#950813} NOKEYCHECK=True GitOrigin-RevId: afd5c7343a40565908f177c0f6028be5ab22b173

This CL adds tests for more cases raised in: whatwg/urlpattern#145 It turns out we don't need any further code changes to pass these test cases since the parser already removes empty groups. Bug: 1263673 Change-Id: I7b8da0da23e30780a09fc5132d36f4154b42b0db Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3321903 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#950832} NOKEYCHECK=True GitOrigin-RevId: 1e207d853f411608e885d7f96ae93cb8f79b33ce

This CL fixes a minor issue with a previous change landed to address the problems in: whatwg/urlpattern#145 We previously made a change to produce `{:foo}bar` instead of `:foobar`, etc. That change, however, also emitted the `{}` even when the custom name had a regexp associated with it; e.g. `:foo(baz)`. In that case we don't need the protection of the `{}` since the regexp placement prevents confusion with the following group. This CL fixes this minor issue by checking if the part's type is kSegmentWildcard. It also adds some tests to validate this behavior. Bug: 1263673 Change-Id: I613009c59a621e394ea697611077bf1fc34c5c43 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3328146 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#951229} NOKEYCHECK=True GitOrigin-RevId: f81ca326f62770e75b1a4fab800ac992717c36d5

This CL fixes another problem where generating a pattern string accidentally changes the meaning of the pattern: whatwg/urlpattern#145 (comment) In this case we want to make sure that a character that can be treated as an implicit group prefix, e.g. `/`, is not incorrectly converted into a prefix. For example, if the original string is `/{:foo?}` we don't want to generate `/:foo?` since that would make the `/` optional. This CL also includes an additional fix that permits emitting `*` for wildcards when there is a prefix. Previously we were being too conservative in some situations. For example, we can emit `*/*` instead of `*/(.*)`. Fixed: 1285620 Change-Id: Idef288bdfdfaa0ae60f320ec326ee2f07d448681 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3378528 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#957730} NOKEYCHECK=True GitOrigin-RevId: 552dd792a4598e6a94c1be24b25245089ff9e681

This fixes another issue related to: whatwg/urlpattern#145 In this case we were incorrectly looking at the prefix value when trying to determine if we should protect against characters following a `:foo` named group. We don't need the prefix check because we already have the `needs_grouping` check. With this check in place we would incorrectly allow following characters when there is an implicit prefix, like `/` in a pathname. Bug: 1289115 Change-Id: Id8a88b55a31c3a0e72310574fe4d7a6fc7781d68 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3405148 Reviewed-by: Jeremy Roman <jbroman@chromium.org> Commit-Queue: Ben Kelly <wanderview@chromium.org> Cr-Commit-Position: refs/heads/main@{#961663} NOKEYCHECK=True GitOrigin-RevId: 1734a2a738499b8e7f49771a84945db7f3089bb2

chromium-wpt-export-bot mentioned this issue Dec 2, 2021

URLPattern: Don't emit * when it can be confused for a modifier. web-platform-tests/wpt#31861

Merged

chromium-wpt-export-bot mentioned this issue Dec 3, 2021

URLPattern: Emit {} around custom names followed by a regexp group. web-platform-tests/wpt#31882

Merged

chromium-wpt-export-bot mentioned this issue Dec 8, 2021

URLPattern: Escape some suffixes trailing :foo groups. web-platform-tests/wpt#31965

Merged

chromium-wpt-export-bot mentioned this issue Dec 8, 2021

URLPattern: Add tests cases for patterns with empty groups. web-platform-tests/wpt#31967

Merged

chromium-wpt-export-bot mentioned this issue Dec 8, 2021

URLPattern: Emit {} when plain text follows a :foo group. web-platform-tests/wpt#31968

Merged

wanderview mentioned this issue Dec 9, 2021

Don't emit '*' when it can be confused for a modifier. (#145) #151

Merged

wanderview mentioned this issue Jan 13, 2022

Fix canonical pattern string generation. (#53) kenchris/urlpattern-polyfill#59

Merged

wanderview closed this as completed Jan 14, 2022

chromium-wpt-export-bot mentioned this issue Jan 20, 2022

URLPattern: Fix erroneous prefix check in GeneratePatternString(). web-platform-tests/wpt#32481

Merged

wanderview reopened this Jan 20, 2022

wanderview mentioned this issue Jan 20, 2022

Remove erroneous prefix check when generating string. (#145) #161

Merged

wanderview closed this as completed Jan 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pattern string canonicalization with wildcards can result in different meanings #145

pattern string canonicalization with wildcards can result in different meanings #145

wanderview commented Oct 26, 2021 •

edited

Loading

wanderview commented Oct 26, 2021 •

edited

Loading

wanderview commented Oct 26, 2021 •

edited

Loading

wanderview commented Oct 29, 2021

wanderview commented Dec 2, 2021

wanderview commented Dec 6, 2021

wanderview commented Jan 20, 2022

pattern string canonicalization with wildcards can result in different meanings #145

pattern string canonicalization with wildcards can result in different meanings #145

Comments

wanderview commented Oct 26, 2021 • edited Loading

wanderview commented Oct 26, 2021 • edited Loading

wanderview commented Oct 26, 2021 • edited Loading

wanderview commented Oct 29, 2021

wanderview commented Dec 2, 2021

wanderview commented Dec 6, 2021

wanderview commented Jan 20, 2022

wanderview commented Oct 26, 2021 •

edited

Loading

wanderview commented Oct 26, 2021 •

edited

Loading

wanderview commented Oct 26, 2021 •

edited

Loading