make_clickable doesn't handle closing parenthesis character just before the 'dot' on a file URL.

[rhellewell@…]

The make_clickable() function will convert a text URL into a clickable HREF. This works well, even with text URLs that contain a space.

But consider this URL when typed into a post or comment box:

 https://www.example.com/some-page(2).jpg

That URL will 'break' (when converted to an HREF by make_clickable) because of the closing parenthesis character just before the 'dot', turning the clickable part of the URL into

https://www.example.com/some-page(2

with the '.jpg' shown as text (outside of the HREF 'target')

This was tested with with WP 6.6.1 on PHP 8.1.28 and verified by another StackOverFlow user (see ​https://wordpress.stackexchange.com/q/426740/29416 )

[parthvataliya]

I attached the patch resolving the issue. Please review this patch and if it is fine then I'm happy to create PR.

[hellofromTonya]

Hello @rhellewellgmailcom,

Welcome to WordPress Core's Trac.

I'm doing triage for tickets reported against 6.6.x version.

In reviewing changes from 6.6.2, not seeing anything that affected or could be related to the reported issue.

Tested it on 6.5.4 and 6.6.2. In both cases the closing parenthesis ) was removed.

Updating Version to empty for now, i.e. until the version is known.

BTW ​the Version indicates the WordPress release version that introduced or the first version affected by the reported issue.

[rhellewell@…]

Note that the issue is not the removal of the closing paren from the text of the url, but that the example text is not turned into a valid URL. The HREF value is truncated to 'break' the URL.

So the HREF value of the text when converted by make_clickable

https://www.example.com/some-image(2).jpg

is turned into a clickable URL of ​https://www.example.com/some-image(2 . Which breaks the URL.

[hellofromTonya]

I was curious if there are test dataset(s) for this function covering (). Yes, there is.

Starting here in the tests, there are multiple datasets testing for URLs with () including. These were added via #10990 to specifically test for URLs with brackets within the URL.

Looking at #10990, it's scope was to "Include trailing ) in make_clickable if it is part of the URL" and was committed in 2.9.0.

[hellofromTonya]

Thanks for clarifying @rhellewellgmailcom.

To isolate it to just the make_clickable() function, I added a wp-content/mu-plugins/test.php file with the following code:

<?php

add_action( 'init', function() {
        $test_url = 'https://www.example.com/some-page(2).jpg';
        $results  = make_clickable( $test_url );
        var_dump( $results );
        exit;
});

On both WordPress 6.5.4 and 6.6.2, the returned string from make_clickable() was:

<a href="https://www.example.com/some-page(2)" rel="nofollow">https://www.example.com/some-page(2)</a>.jpg

Interesting.

In looking at the tests mentioned in comment:6, none of those tests cover a use case where the (blah) appears before a . extension. Interesting.

make_clickable() tests: Adds datasets for URLs with () within the path and with extensions.

Trac ticket: https://core.trac.wordpress.org/ticket/62037

[hellofromTonya]

​PR 7343 adds tests to valid URL that has () within its path and has an extension. As reported in this ticket, the following dataset

'URL with brackets in path before the extension' => array(
	'text'     => 'http://example-image(2).jpg',
	'expected' => '<a href="http://example-image(2).jpg" rel="nofollow">http://example-image(2).jpg</a>',
),

fails with this report:

1) Tests_Formatting_MakeClickable::test_urls with data set "URL with brackets in path before the extension" ('http://example-image(2).jpg', '<a href="http://example-image...pg</a>')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'<a href="http://example-image(2).jpg" rel="nofollow">http://example-image(2).jpg</a>'
+'<a href="http://example-image(2)" rel="nofollow">http://example-image(2)</a>.jpg'

whereas this dataset

'URL with brackets within path and with a extension' => array(
	'text'     => 'http://example-(2)-image.jpg',
	'expected' => '<a href="http://example-(2)-image.jpg" rel="nofollow">http://example-(2)-image.jpg</a>',
),

passes.

This is a reproducible bug. Moving it into the 6.7 milestone.

[hellofromTonya]

Thank you @parthvataliya for the patch. Testing it with and without ​PR 7343, it causes 2 datasets to fail:

This dataset:

'URL with brackets in path: trailing .)'     => array(
	'text'     => 'blah blah http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software).) blah blah',
	'expected' => 'blah blah <a href="http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software)" rel="nofollow">http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software)</a>.) blah blah',
),

with these results reported:

1) Tests_Formatting_MakeClickable::test_urls with data set "URL with brackets in path: trailing .)" ('blah blah http://en.wikipedia...h blah', 'blah blah <a href="http://en....h blah')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'blah blah <a href="http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software)" rel="nofollow">http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software)</a>.) blah blah'
+'blah blah <a href="http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software)." rel="nofollow">http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software).</a>) blah blah'

/var/www/tests/phpunit/tests/formatting/makeClickable.php:76

and this dataset:

'URL with brackets in path: trailing .)moreurl' => array(
	'text'     => 'blah blah http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software).)moreurl blah blah',
	'expected' => 'blah blah <a href="http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software)" rel="nofollow">http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software)</a>.)moreurl blah blah',
),

with these results reported:

2) Tests_Formatting_MakeClickable::test_urls with data set "URL with brackets in path: trailing .)moreurl" ('blah blah http://en.wikipedia...h blah', 'blah blah <a href="http://en....h blah')
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'blah blah <a href="http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software)" rel="nofollow">http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software)</a>.)moreurl blah blah'
+'blah blah <a href="http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software)." rel="nofollow">http://en.wikipedia.org/wiki/PC_Tools_(Central_Point_Software).</a>)moreurl blah blah'

/var/www/tests/phpunit/tests/formatting/makeClickable.php:76

Notice how it included the ., repositioning it into the URL, rather than having it as an endstop period.

[parthvataliya]

Thank you, @hellofromTonya, for testing the patch.

I’ve attached the updated patch for the issue. I’ve also tested it with the datasets mentioned, and everything looks good. Could you please test the new patch?

Thank you.

[hellofromTonya]

Replying to parthvataliya:

I’ve attached the updated patch for the issue. I’ve also tested it with the datasets mentioned, and everything looks good. Could you please test the new patch?

Hey @parthvataliya, 62037-1.patch​ has what appears to local testing in the Twenty Twenty theme functions file. Should it include the fix within make_clickable()?

[parthvataliya]

Thank you, @hellofromTonya, for testing the previous patch.

Apologies, I mistakenly uploaded the wrong patch earlier. I’ve now attached the correct one. I’ve tested it with the all test cases, and everything looks good. Could you please test this new patch?

Thank you again.

[coquardcyr]

@hellofromTonya I ran the tests locally with the new patch and no test seems to fail.

[hellofromTonya]

Patch: ​https://github.com/WordPress/wordpress-develop/pull/7343

@parthvataliya I added your 62037-2.patch​ to PR 7343] to provide a complete patch with tests and for the code to run through all of CI jobs.

This patch is now ready for testing, feedback, and test reports.

Hmm, the tests started failing after merging in trunk and making the whitespace change to the comments. As this is a bug, I'll leave it for post beta 1.

Failures are unrelated, committing.

[peterwilsoncc]

In 59143:

Formatting: Improve parenthesis handling in make_clickable().

Improve the regular expression for making links clickable to account for parenthesis in links containing an extension, for example: http://wordpress.org/my-image(2).jpg.

Props coquardcyr, hellofromtonya, parthvataliya, rhellewellgmailcom.
Fixes #62037.

Merged r59143 / ​https://github.com/WordPress/wordpress-develop/commit/9b6c034e2fbb3bb876932cf3770a93791a7a22f3