Pattern matching URLs does not allow a query string

Created on 27 August 2024, about 1 month ago
Updated 5 September 2024, 24 days ago

Problem/Motivation

We needed a way to detect the "named captured variable" from a query string. This module does not support that yet.

Steps to reproduce

  • Add a redirect with redirect source in a query string. E.g., page\/old\?code=(?P<nr>[0-9a-z]+) (note that the ? of the query string is escaped.
  • Add a redirect target capturing this variable.
  • Visit the URL that matches the pattern.

Repeat the same with the "Redirect target" field.

Expected results: The captured variable is substituted in the target and redirect happens.

Actual results: The redirect does not happen.

Proposed resolution

Check for query string in the URL as well.

Remaining tasks

MR

User interface changes

NA

API changes

NA

Data model changes

NA

🐛 Bug report
Status

RTBC

Version

2.0

Component

Code

Created by

🇮🇳India ajits India

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @ajits
  • Assigned to ajits
  • 🇮🇳India ajits India

    Also, I am working on this.

  • 🇮🇳India ajits India

    Created a PR that should fix this. Also, adjusted the title.

  • Issue was unassigned.
  • Status changed to Needs review about 1 month ago
  • Pipeline finished with Success
    about 1 month ago
    Total: 140s
    #266160
  • Status changed to Needs work 27 days ago
  • 🇸🇮Slovenia paranojik

    Seems to work fine, however the redirect source field help text seems to be inaccurate.
    Enter an internal url regex path with named captures. For example: page\/old\/(?P&lt;nr&gt;[0-9a-z]+). Please be aware that certain characters such as "$", or "?" need to be escaped due to it being part of a replacement string.
    The special characters are to be escaped because they're part of the regular expression used for matching not because they're part of a replacement string.

  • Pipeline finished with Success
    25 days ago
    Total: 146s
    #272701
  • 🇮🇳India ajits India

    The target also needs to support the escaped `?`

  • Status changed to Needs review 25 days ago
  • 🇮🇳India ajits India

    Thank you for your review! I have adjusted the help text. I have also made the change so that the redirect target also supports the query string.

  • Status changed to RTBC 24 days ago
  • 🇸🇮Slovenia paranojik

    I think this is a very good enhancement. Thanks @ajits!
    Marking RTBC.

  • 🇩🇪Germany captain hindsight

    Hey, thanks for this improvement. I needed a redirect from one query parameter to another, i.e from \?foo=(?P<foo>[0-9a-z]+) to ?bar=<foo>.

    With the MR the redirect happens, but the named value is not replaced. I guess this is because $redirect->getRedirectUrl()->toUriString() in RegexRedirectRepository::replaceRegexWithActualUrl delivers encoded url parameters. Using $redirect_regex_url = preg_replace($pattern, $value, urldecode($redirect_regex_url)); solves my problem but maybe there is a better solution.

Production build 0.71.5 2024