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

fix: do not partial match url params extraction #22281

Open
wants to merge 7 commits into
base: 5.x-dev
Choose a base branch
from

Conversation

Lazyuki
Copy link

@Lazyuki Lazyuki commented Jun 3, 2024

Description:

Fix for #16137.

Custom Action Dimensions have an option to automatically extract values from the URL Search Parameters. As mentioned in the docs, it does not allow using Regex because it's supposed to be a "simple" match. One would assume it's doing a simple check to fetch the exact match for the URL parameter.
However, it's actually constructing a partial match with the regex \?.*pattern=([^&]*). This works fine in most cases, but if the pattern is included in another pattern's word ending, this regex picks that up instead.

So imagine trying to extract the type parameter from the URL

https://example.com/search?type=foo&date=bar&contenttype=baz

It should match type=foo and returns foo in theory, but current regex actually matches type=baz at the end from contenttype=baz because the regex is only searching for a parameter that ends with the specified pattern, in this case type. To fix this, we know that valid URLs don't have ? or & unless they are for search params (as they should be URL encoded otherwise), we can construct the regex [?&]pattern=([^&]*) instead to make sure the exact pattern is matched in the URL.

Review

@sgiehl sgiehl added the Needs Review PRs that need a code review label Jun 13, 2024
Copy link
Contributor

This issue is in "needs review" but there has been no activity for 7 days. ping @matomo-org/core-reviewers

@github-actions github-actions bot added the Stale The label used by the Close Stale Issues action label Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review Stale The label used by the Close Stale Issues action
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants