Permalink
Browse files

Use preg_replace_callback over preg_replace with e modifier

  • Loading branch information...
1 parent e4cb25b commit c56a80d6ce5294a5339fb45d62bccf45cc5dcdc3 @simonwelsh simonwelsh committed Dec 20, 2012
Showing with 24 additions and 14 deletions.
  1. +15 −10 control/HTTP.php
  2. +6 −3 core/Convert.php
  3. +3 −1 email/Mailer.php
View
@@ -60,20 +60,25 @@ public static function urlRewriter($content, $code) {
if(!is_numeric($tag)) $tagPrefix = "$tag ";
else $tagPrefix = "";
- $regExps[] = "/(<{$tagPrefix}[^>]*$attrib *= *\")([^\"]*)(\")/ie";
- $regExps[] = "/(<{$tagPrefix}[^>]*$attrib *= *')([^']*)(')/ie";
- $regExps[] = "/(<{$tagPrefix}[^>]*$attrib *= *)([^\"' ]*)( )/ie";
+ $regExps[] = "/(<{$tagPrefix}[^>]*$attrib *= *\")([^\"]*)(\")/i";
+ $regExps[] = "/(<{$tagPrefix}[^>]*$attrib *= *')([^']*)(')/i";
+ $regExps[] = "/(<{$tagPrefix}[^>]*$attrib *= *)([^\"' ]*)( )/i";
}
- $regExps[] = '/(background-image:[^;]*url *\()([^)]+)(\))/ie';
- $regExps[] = '/(background:[^;]*url *\()([^)]+)(\))/ie';
- $regExps[] = '/(list-style-image:[^;]*url *\()([^)]+)(\))/ie';
- $regExps[] = '/(list-style:[^;]*url *\()([^)]+)(\))/ie';
+ $regExps[] = '/(background-image:[^;]*url *\()([^)]+)(\))/i';
+ $regExps[] = '/(background:[^;]*url *\()([^)]+)(\))/i';
+ $regExps[] = '/(list-style-image:[^;]*url *\()([^)]+)(\))/i';
+ $regExps[] = '/(list-style:[^;]*url *\()([^)]+)(\))/i';
// Make
- $code = 'stripslashes("$1") . (' . str_replace('$URL', 'stripslashes("$2")', $code) . ') . stripslashes("$3")';
-
+ $callback = function($matches) use($code) {
+ return
+ stripslashes($matches[1]) .
+ str_replace('$URL', stripslashes($matches[2]), $code) .
+ stripslashes($matches[3]);
+ };
+
foreach($regExps as $regExp) {
- $content = preg_replace($regExp, $code, $content);
+ $content = preg_replace_callback($regExp, $callback, $content);
}
return $content;
View
@@ -244,9 +244,12 @@ public static function html2raw($data, $preserveLinks = false, $wordWrap = 60, $
// Expand hyperlinks
if(!$preserveLinks && !$config['PreserveLinks']) {
- $data = preg_replace('/<a[^>]*href\s*=\s*"([^"]*)">(.*?)<\/a>/ie', "Convert::html2raw('\\2').'[\\1]'",
- $data);
- $data = preg_replace('/<a[^>]*href\s*=\s*([^ ]*)>(.*?)<\/a>/ie', "Convert::html2raw('\\2').'[\\1]'", $data);
+ $data = preg_replace_callback('/<a[^>]*href\s*=\s*"([^"]*)">(.*?)<\/a>/i', function($matches) {
+ return Convert::html2raw($matches[2]) . "[$matches[1]]";
+ }, $data);
+ $data = preg_replace_callback('/<a[^>]*href\s*=\s*([^ ]*)>(.*?)<\/a>/i', function($matches) {
+ return Convert::html2raw($matches[2]) . "[$matches[1]]";
+ }, $data);
}
// Replace images with their alt tags
View
@@ -368,7 +368,9 @@ function encodeFileForEmail($file, $destFileName = false, $disposition = NULL, $
function QuotedPrintable_encode($quotprint) {
$quotprint = (string)str_replace('\r\n',chr(13).chr(10),$quotprint);
$quotprint = (string)str_replace('\n', chr(13).chr(10),$quotprint);
- $quotprint = (string)preg_replace("~([\x01-\x1F\x3D\x7F-\xFF])~e", "sprintf('=%02X', ord('\\1'))", $quotprint);
+ $quotprint = (string)preg_replace_callback("~([\x01-\x1F\x3D\x7F-\xFF])~e", function($matches) {
+ return sprintf('=%02X', ord($matches[1]));
+ }, $quotprint);
//$quotprint = (string)str_replace('\=0D=0A',"=0D=0A",$quotprint);
$quotprint = (string)str_replace('=0D=0A',"\n",$quotprint);
$quotprint = (string)str_replace('=0A=0D',"\n",$quotprint);

7 comments on commit c56a80d

Owner

wilr replied Jan 17, 2013

I believe this has broken rewriting links in emails (HTTP::urlRewriter()) now generates for example - <a href="(substr(http://external.com,0,1) == "/") ? ( Director::protocolAndHost() . http://external.com ) : ( (preg_match("/^[A-Za-z]+:/", http://external.com)) ? http://external.com : Director::absoluteBaseURL() . http://external.com )"> rather than the link. A simple test case to check -

https://gist.github.com/4553562

Contributor

tractorcow replied Jan 23, 2013

This seems to be the case. Would it not make more sense for urlRewriter to take a callable as a second parameter, rather than a piece of code encode in a string?

Owner

chillu replied Jan 23, 2013

That'd be a nice API addition, but its more important that we fix the regression at hand.

Contributor

simonwelsh replied Jan 23, 2013

Okay, so I misunderstood how this worked and that the argument does actually get evaled. Thus, to match the original method, just wrap the str_replace() in eval(). This feels really dirty, but is exactly what was happening before just with an /e instead of an eval().

We need to get away from using /e due to its deprecation status in PHP5.5.

Contributor

tractorcow replied Jan 23, 2013

I'll work on a fix and some test cases today.

Contributor

tractorcow replied Jan 23, 2013

Hi Ingo, I apologise for making an API tweak in my fix, but in my investigation I found that using eval as a replacement is probably not the ideal long term solution.

See silverstripe#1129 for my fix and test cases.

This fixes all current code without requiring an api change, until 3.2 when using $code as a string will raise a deprecation warning.

Please sign in to comment.