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

[4.x]: Twig replace filter doesn't accept forward slashes for regex like Craft 3 did #13618

Closed
gbowne-quickbase opened this issue Aug 29, 2023 · 1 comment

Comments

@gbowne-quickbase
Copy link

gbowne-quickbase commented Aug 29, 2023

What happened?

TL;DR: The test string within the twig extension replaceFilter changed between Craft 3 and Craft 4, and no longer accepts the forward slash / character as being valid for determining whether or not a string is a regex

Craft 3.x:

if (preg_match('/^\/.+\/[a-zA-Z]*$/', $search)) {

Craft 4.x:
$isRegex = fn(string $s) => (bool)preg_match('/^\/[^\/]+\/[imsxADSUXJun]*$/', $s);

It's explicitly in the [^\/] bit that excludes it!

Description

We have a macro we run to pull essential information out of a URL so that authors can just paste in what they get from a 3rd party video hosting site, and we can make sure the right code is rendered on the front end. Testing our Craft 4.5.2 branch, we're seeing that the forward slashes in URLs are failing the regex test, we're not getting results as expected. Even trying to escape the forward slash with a single or double backslash isn't getting the results we expect. A little background: for a while we had urls that were http:// or that pointed at an older library style, and as the video site updated its player, we needed to provide a different URL to get to the same content, and built the macro do to the work for us.

Steps to reproduce

{% set wistiaUrl = 'https://fast.wistia.net/embed/iframe/dggty9xszj?popover=true'
{% set wistiaRegex = '/(http(s?):)?\\/\\/fast\\.wistia\\.(net|com)\\/embed\\/(iframe|medias)\\//' %}
{% set videoId = wistiaUrl|replace(wistiaRegex,'') %}
{% set videoId = videoId|replace('?popover=true', '') %}

Expected behavior

We'd expect to just get dggty9xszj from this. As we do in Craft 3.8.x

Actual behavior

  1. Only the ?popover=true got removed by the non regex replace filter

Other things I've tried

  1. I did some simple tests to diagnose it and determine whether I was escaping the forward slash / improperly with the string 'this/that' that I would change into 'this or that' using a replace filter
    • {{ 'this/that'|replace('/\\//',' or ') }} = this/that <- double escape the forward slash, single quotes
    • {{ 'this/that'|replace("/\\//",' or ') }} = this/that <- double escape the forward slash, double quotes
    • {{ 'this/that'|replace('/\//',' or ') }} = this/that <- single escape the forward slash, single quotes
    • {{ 'this/that'|replace("/\//",' or ') }} = this/that <- single escape the forward slash, double quotes
    • {{ 'this/that'|replace('///',' or ') }} = this/that <- don't escape the forward slash at all, single quotes
    • {{ 'this/that'|replace("///",' or ') }} = this/that <- don't escape the forward slash at all, double quotes
    • {{ 'this/that'|replace('#/#',' or ') }} = this/that <- try to replace the starting/ending slashes in the regex with the # character
  2. As a workaround, I used a |replace('/', '#') on the incoming URL, then replaced the slashes in my regex with # and it behaved properly, but that's not a fix that feels good moving forward.
  3. I looked into the code in /src/web/twig/Extension.php for the replaceFilter and saw it's using a preg_match to determine if the $search is a regex or not. From what I can see, your test string ^\/[^\/]+\/[imsxADSUXJun]*$ will not pass if there's any sort of slash involved
    $isRegex = fn(string $s) => (bool)preg_match('/^\/[^\/]+\/[imsxADSUXJun]*$/', $s);

    In Craft 3, the test string was ^\/.+\/[a-zA-Z]*$ and this does tolerate slashes.

Craft CMS version

4.5.2

PHP version

8.2.8

Operating system and version

MacOs 13.5.1 (developers) and Centos Linux 7 (staging/production)

Database type and version

MySQL 5.7

Image driver and version

No response

Installed plugins and versions

@gbowne-quickbase gbowne-quickbase changed the title [4.x]: the twig replace filter with regex doesn't handle forward slashes like craft 3 [4.x]: Twig replace filter doesn't accept forward slashes for regex like Craft 3 did Aug 29, 2023
@brandonkelly
Copy link
Member

Thanks for reporting that! Craft 4.5.3 is out now with a fix.

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

No branches or pull requests

2 participants