Skip to content

Commit a259328

Browse files
matthiasmulliematthiasmullie
committed
Merge branch 'EreMaijala-fix-backtrack-limit'
2 parents 4c4ecee + 37446a6 commit a259328

File tree

4 files changed

+127
-32
lines changed

4 files changed

+127
-32
lines changed

‎src/CSS.php‎

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
namespace MatthiasMullie\Minify;
1414

1515
use MatthiasMullie\Minify\Exceptions\FileImportException;
16+
use MatthiasMullie\Minify\Exceptions\PatternMatchException;
1617
use MatthiasMullie\PathConverter\Converter;
1718
use MatthiasMullie\PathConverter\ConverterInterface;
1819

@@ -296,6 +297,8 @@ protected function importFiles($source, $content)
296297
* @param string[] $parents Parent paths, for circular reference checks
297298
*
298299
* @return string The minified data
300+
*
301+
* @throws PatternMatchException
299302
*/
300303
public function execute($path = null, $parents = array())
301304
{
@@ -733,36 +736,59 @@ protected function stripComments()
733736
* @param string $content The CSS content to strip the whitespace for
734737
*
735738
* @return string
739+
*
740+
* @throws PatternMatchException
736741
*/
737742
protected function stripWhitespace($content)
738743
{
739744
// remove leading & trailing whitespace
740-
$content = preg_replace('/^\s*/m', '', $content);
741-
$content = preg_replace('/\s*$/m', '', $content);
745+
$content = $this->pregReplace('/^\s*/m', '', $content);
746+
$content = $this->pregReplace('/\s*$/m', '', $content);
742747

743748
// replace newlines with a single space
744-
$content = preg_replace('/\s+/', ' ', $content);
749+
$content = $this->pregReplace('/\s+/', ' ', $content);
745750

746751
// remove whitespace around meta characters
747752
// inspired by stackoverflow.com/questions/15195750/minify-compress-css-with-regex
748-
$content = preg_replace('/\s*([\*$~^|]?+=|[{};,>~]|!important\b)\s*/', '$1', $content);
749-
$content = preg_replace('/([\[(:>\+])\s+/', '$1', $content);
750-
$content = preg_replace('/\s+([\]\)>\+])/', '$1', $content);
751-
$content = preg_replace('/\s+(:)(?![^\}]*\{)/', '$1', $content);
753+
$content = $this->pregReplace('/\s*([\*$~^|]?+=|[{};,>~]|!important\b)\s*/', '$1', $content);
754+
$content = $this->pregReplace('/([\[(:>\+])\s+/', '$1', $content);
755+
$content = $this->pregReplace('/\s+([\]\)>\+])/', '$1', $content);
756+
$content = $this->pregReplace('/\s+(:)(?![^\}]*\{)/', '$1', $content);
752757

753758
// whitespace around + and - can only be stripped inside some pseudo-
754759
// classes, like `:nth-child(3+2n)`
755760
// not in things like `calc(3px + 2px)`, shorthands like `3px -2px`, or
756761
// selectors like `div.weird- p`
757762
$pseudos = array('nth-child', 'nth-last-child', 'nth-last-of-type', 'nth-of-type');
758-
$content = preg_replace('/:(' . implode('|', $pseudos) . ')\(\s*([+-]?)\s*(.+?)\s*([+-]?)\s*(.*?)\s*\)/', ':$1($2$3$4$5)', $content);
763+
$content = $this->pregReplace('/:(' . implode('|', $pseudos) . ')\(\s*([+-]?)\s*(.+?)\s*([+-]?)\s*(.*?)\s*\)/', ':$1($2$3$4$5)', $content);
759764

760765
// remove semicolon/whitespace followed by closing bracket
761766
$content = str_replace(';}', '}', $content);
762767

763768
return trim($content);
764769
}
765770

771+
/**
772+
* Perform a preg_replace and check for errors.
773+
*
774+
* @param string $pattern Pattern
775+
* @param string $replacement Replacement
776+
* @param string $subject String to process
777+
*
778+
* @return string
779+
*
780+
* @throws PatternMatchException
781+
*/
782+
protected function pregReplace($pattern, $replacement, $subject)
783+
{
784+
$result = preg_replace($pattern, $replacement, $subject);
785+
if ($result === null) {
786+
throw PatternMatchException::fromLastError("Failed to replace with pattern '$pattern'");
787+
}
788+
789+
return $result;
790+
}
791+
766792
/**
767793
* Replace all occurrences of functions that may contain math, where
768794
* whitespace around operators needs to be preserved (e.g. calc, clamp).

‎src/Exceptions/PatternMatchException.php‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
/**
4+
* Pattern match exception.
5+
*
6+
* Please report bugs on https://github.com/matthiasmullie/minify/issues
7+
*
8+
* @author Ere Maijala <[email protected]>
9+
* @copyright Copyright (c) 2012, Matthias Mullie. All rights reserved
10+
* @license MIT License
11+
*/
12+
13+
namespace MatthiasMullie\Minify\Exceptions;
14+
15+
/**
16+
* Pattern Match Exception Class.
17+
*
18+
* @author Ere Maijala <[email protected]>
19+
*/
20+
class PatternMatchException extends BasicException
21+
{
22+
/**
23+
* Create an exception from preg_last_error.
24+
*
25+
* @param string $msg Error message
26+
*/
27+
public static function fromLastError($msg)
28+
{
29+
$msg .= ': Error ' . preg_last_error();
30+
if (PHP_MAJOR_VERSION >= 8) {
31+
$msg .= ' - ' . preg_last_error_msg();
32+
}
33+
34+
return new PatternMatchException($msg);
35+
}
36+
}

‎src/Minify.php‎

Lines changed: 35 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
namespace MatthiasMullie\Minify;
1414

1515
use MatthiasMullie\Minify\Exceptions\IOException;
16+
use MatthiasMullie\Minify\Exceptions\PatternMatchException;
1617
use Psr\Cache\CacheItemInterface;
1718

1819
/**
@@ -265,23 +266,9 @@ protected function registerPattern($pattern, $replacement = '')
265266
*/
266267
protected function stripMultilineComments()
267268
{
268-
// First extract comments we want to keep, so they can be restored later
269-
// PHP only supports $this inside anonymous functions since 5.4
270269
$minifier = $this;
271-
$callback = function ($match) use ($minifier) {
272-
$count = count($minifier->extracted);
273-
$placeholder = '/*' . $count . '*/';
274-
$minifier->extracted[$placeholder] = $match[0];
275-
276-
return $placeholder;
277-
};
278-
$this->registerPattern('/
279-
# optional newline
280-
\n?
281-
282-
# start comment
283-
\/\*
284-
270+
// Pattern for matching comments that we want to preserve
271+
$keepPattern = '/^
285272
# comment content
286273
(?:
287274
# either starts with an !
@@ -293,14 +280,24 @@ protected function stripMultilineComments()
293280
# there is either a @license or @preserve tag
294281
@(?:license|preserve)
295282
)
283+
/ixs';
284+
$callback = function ($match) use ($minifier, $keepPattern) {
285+
if (preg_match($keepPattern, $match[1])) {
286+
// Preserve the comment
287+
$count = count($minifier->extracted);
288+
$placeholder = '/*' . $count . '*/';
289+
$minifier->extracted[$placeholder] = $match[0];
290+
} else {
291+
// Discard the comment but keep any single line feed
292+
$placeholder = strncmp($match[0], "\n", 1) === 0 || substr($match[0], -1) === "\n"
293+
? "\n"
294+
: '';
295+
}
296296

297-
# then match to the end of the comment
298-
.*?\*\/\n?
299-
300-
/ixs', $callback);
297+
return $placeholder;
298+
};
301299

302-
// Then strip all other comments
303-
$this->registerPattern('/\/\*.*?\*\//s', '');
300+
$this->registerPattern('/\n?\/\*(.*?)\*\/\n?/s', $callback);
304301
}
305302

306303
/**
@@ -314,6 +311,8 @@ protected function stripMultilineComments()
314311
* @param string $content The content to replace patterns in
315312
*
316313
* @return string The (manipulated) content
314+
*
315+
* @throws PatternMatchException
317316
*/
318317
protected function replace($content)
319318
{
@@ -341,14 +340,20 @@ protected function replace($content)
341340
}
342341

343342
$match = null;
344-
if (preg_match($pattern, $content, $match, PREG_OFFSET_CAPTURE, $processedOffset)) {
343+
$matchResult = preg_match($pattern, $content, $match, PREG_OFFSET_CAPTURE, $processedOffset);
344+
if ($matchResult) {
345345
$matches[$i] = $match;
346346

347347
// we'll store the match position as well; that way, we
348348
// don't have to redo all preg_matches after changing only
349349
// the first (we'll still know where those others are)
350350
$positions[$i] = $match[0][1];
351351
} else {
352+
if ($matchResult === false) {
353+
throw PatternMatchException::fromLastError(
354+
"Failed to match pattern '$pattern' at $processedOffset"
355+
);
356+
}
352357
// if the pattern couldn't be matched, there's no point in
353358
// executing it again in later runs on this same content;
354359
// ignore this one until we reach end of content
@@ -445,6 +450,11 @@ protected function extractStrings($chars = '\'"', $placeholderPrefix = '')
445450
};
446451

447452
/*
453+
* Quantifier {0,65535} is used instead of *? to avoid exceeding
454+
* backtrack limit with large strings. 65535 is the maximum allowed
455+
* (see https://www.php.net/manual/en/regexp.reference.repetition.php)
456+
* and should be well sufficient for string representations here.
457+
*
448458
* The \\ messiness explained:
449459
* * Don't count ' or " as end-of-string if it's escaped (has backslash
450460
* in front of it)
@@ -456,7 +466,8 @@ protected function extractStrings($chars = '\'"', $placeholderPrefix = '')
456466
* considered as escape-char (times 2) and to get it in the regex,
457467
* escaped (times 2)
458468
*/
459-
$this->registerPattern('/([' . $chars . '])(.*?(?<!\\\\)(\\\\\\\\)*+)\\1/s', $callback);
469+
470+
$this->registerPattern('/([' . $chars . '])(.{0,65535}?(?<!\\\\)(\\\\\\\\)*+)\\1/s', $callback);
460471
}
461472

462473
/**

‎tests/CSS/CSSTest.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,21 @@ public function testSetConfig()
9898
$this->assertEquals($property->getValue($minifier), array('gif' => 'data:image/gif'));
9999
}
100100

101+
/**
102+
* Test minifier error handling.
103+
*/
104+
public function testErrorHandling()
105+
{
106+
// long whitespace that could exceed pcre.backtrack_limit
107+
$limit = (int) (ini_get('pcre.backtrack_limit') ?: 1000000);
108+
$src = '.a1::after{content: "2"}' . str_repeat(' ', $limit) . '.a1::after{content: "1"}';
109+
110+
$minifier = $this->getMinifier();
111+
$minifier->add($src);
112+
$this->expectException('MatthiasMullie\Minify\Exceptions\PatternMatchException');
113+
$minifier->minify();
114+
}
115+
101116
/**
102117
* @return array [input, expected result]
103118
*/
@@ -880,6 +895,13 @@ public static function dataProvider()
880895
'a{color:rgba(var(--bs-link-color-rgb),var(--bs-link-opacity,1));text-decoration:none}a:hover{--bs-link-color-rgb:var(--bs-link-hover-color-rgb)}a:not([href]):not([class]),a:not([href]):not([class]):hover{color:inherit;text-decoration:none}',
881896
);
882897

898+
// a large stylesheet that could exceed pcre.backtrack_limit
899+
$limit = (int) (ini_get('pcre.backtrack_limit') ?: 1000000);
900+
$tests[] = array(
901+
'.a1::after{content: "2"}/* Comment with apostrophe\' */' . str_repeat('a{}', (int) ($limit / 3)) . '/* Comment with apostrophe\' */.a1::after{content: "1"}',
902+
'.a1::after{content:"2"}.a1::after{content:"1"}',
903+
);
904+
883905
return $tests;
884906
}
885907

0 commit comments

Comments
 (0)