[#23549] - SEF - Speed up replace CSS background image regexp
- Closed
- 4 Apr 2022
- Medium
- Build: staging
- # 23549
- Diff
- nextend:patch-2
- Success Hound No violations found. Woof! Details
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success continuous-integration/drone/pr the build was successful Details
User tests: Successful: Unsuccessful:
Pull Request for Issue #23548
In the original version, the regexp matches everything until it finds a : character. So instead using a plain wildcard, it would be better to match every character until a HTML tag close character found [^>].
Original
$regex = '#style=\s*([\'\"])(.*):' . $regex_url . '#m';
Updated
$regex = '#style=\s*([\'\"])([^>]*):' . $regex_url . '#m';
Test code
<?php
$protocols = '[a-zA-Z0-9\-]+:';
$regex_url = '\s*url\s*\(([\'\"]|\&\#0?3[49];)?(?!/|\&\#0?3[49];|' . $protocols . '|\#)([^\)\'\"]+)([\'\"]|\&\#0?3[49];)?\)';
$regex = '#style=\s*([\'\"])([^>]*):' . $regex_url . '#m';
$base = 'https://mybase.com/';
$buffer = '';
for ($i = 0; $i < 4000; $i++) {
$buffer .= '<div style="background-image: url(\'/picture.jpg\');">' . $i . '</div>';
}
echo preg_replace($regex, 'style=$1$2: url($3' . $base . '$4$5)', $buffer);
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Front End Plugins |
@franz-wohlkoenig, Well, if there is a test case which tests the original code, that test should work fine.
If there is no test case yet, then the test case would contain different style attributes which contains url() like:
<div style="background: #234323;"></div> -> No match
<div style="background-image: url('/picture.jpg');"></div> -> Match
<div style="background-image: url(\"/picture.jpg\");"></div> -> Match
<div style="background-image: url(\"/picture.jpg\");"></div> -> Match
<div style="background-image: url(\"http://abc.com/picture.jpg\");"></div> -> Match
My suggestion changes (.*) to ([^>]*). Instead of matching everything, it matches until the first occurrence of the > character. So my suggestion can fail, if there is a > in the style attribute. We should find out if style attribute can contain raw > character.
What I can think of:
- url() is not allowed to contain raw > characters as it is unsafe. encodeUri() translates it to %3E
| Labels |
Removed:
J3 Issue
|
||
Sorry that it took so long to respond. As performance improvements are a welcome addition in Joomla, I would like to ask you to rebase this pr to the 4.2 branch. Please also add clear instructions how this pr can be tested as it contains a change in an important part of Joomla, which will likely affect many users when something doesn't work as before. In the meantime I'm closing the pr, when ready, please reopen again. Thanks for your help making Joomla better.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-04-04 11:09:49 |
| Closed_By | ⇒ | laoneo | |
| Labels |
Added:
?
Removed: ? |
||
Well that bug still exists, at least in the current 3.x branch and affects performance heavily or stops the site from working at all. The problem is well described, a solution is presented as well. Please take care of this bug instead of just closing the pr without further looking into it. I would like to contribute, but I am not familiar with writing test cases.
@nextend please give Instructions how this Pull Request can be tested.