API HTTP::urlRewriter with (string)$code deprecated in 3.1. Fixed regressions and CSS urls.

urlRewriter will expect a callable as a second parameter,
but will work with the current api and simply raise a deprecation error.

HTTP::absoluteURLs now correctly rewrites urls into absolute urls. Resolves introduced in c56a80d6ce

HTTP::absoluteURLs now handles additional cases where urls were not translated.

Test cases for HTTP::absoluteURLs added for both css and attribute links.

Cleaned up replacement expression and improved documentation.

control/HTTP.php

@@ -41,20 +41,43 @@ class HTTP {
 	 */
 	public static function absoluteURLs($html) {
 		$html = str_replace('$CurrentPageURL', $_SERVER['REQUEST_URI'], $html);
-		return HTTP::urlRewriter($html, '(substr($URL,0,1) == "/") ? ( Director::protocolAndHost() . $URL ) :'
+		return HTTP::urlRewriter($html, function($url) {
-			. ' ( (preg_match("/^[A-Za-z]+:/", $URL)) ? $URL : Director::absoluteBaseURL() . $URL )' );
+			return Director::absoluteURL($url, true);
+		});
 	}
 
-	/*
+	/**
-	 * Rewrite all the URLs in the given content, evaluating the given string as PHP code
+	 * Rewrite all the URLs in the given content, evaluating the given string as PHP code.
 	 *
 	 * Put $URL where you want the URL to appear, however, you can't embed $URL in strings
 	 * Some example code:
-	 *  '"../../" . $URL'
+	 * <ul>
-	 *  'myRewriter($URL)'
+	 * <li><code>'"../../" . $URL'</code></li>
-	 *  '(substr($URL,0,1)=="/") ? "../" . substr($URL,1) : $URL'
+	 * <li><code>'myRewriter($URL)'</code></li>
+	 * <li><code>'(substr($URL,0,1)=="/") ? "../" . substr($URL,1) : $URL'</code></li>
+	 * </ul>
+	 * 
+	 * As of 3.2 $code should be a callable which takes a single parameter and returns
+	 * the rewritten URL. e.g.
+	 * 
+	 * <code>
+	 * function($url) { 
+	 *		return Director::absoluteURL($url, true);
+	 * }
+	 * </code>
+	 * 
+	 * @param string $content The HTML to search for links to rewrite
+	 * @param string|callable $code Either a string that can evaluate to an expression
+	 * to rewrite links (depreciated), or a callable that takes a single 
+	 * parameter and returns the rewritten URL
+	 * @return The content with all links rewritten as per the logic specified in $code
 	 */
 	public static function urlRewriter($content, $code) {
+		if(!is_callable($code)) {
+			Deprecation::notice(3.1, 'HTTP::urlRewriter expects a callable as the second parameter');
+		}
+		
+		// Replace attributes
 		$attribs = array("src","background","a" => "href","link" => "href", "base" => "href");
 		foreach($attribs as $tag => $attrib) {
 			if(!is_numeric($tag)) $tagPrefix = "$tag ";
@@ -64,19 +87,28 @@ class HTTP {
 			$regExps[] = "/(<{$tagPrefix}[^>]*$attrib *= *')([^']*)(')/i";
 			$regExps[] = "/(<{$tagPrefix}[^>]*$attrib *= *)([^\"' ]*)( )/i";
 		}
-		$regExps[] = '/(background-image:[^;]*url *\()([^)]+)(\))/i';
+		// Replace css styles
-		$regExps[] = '/(background:[^;]*url *\()([^)]+)(\))/i';
+		// @todo - http://www.css3.info/preview/multiple-backgrounds/
-		$regExps[] = '/(list-style-image:[^;]*url *\()([^)]+)(\))/i';
+		$styles = array('background-image', 'background', 'list-style-image', 'list-style', 'content');
-		$regExps[] = '/(list-style:[^;]*url *\()([^)]+)(\))/i';
+		foreach($styles as $style) {
+			$regExps[] = "/($style:[^;]*url *\(\")([^\"]+)(\"\))/i";
+			$regExps[] = "/($style:[^;]*url *\(')([^']+)('\))/i";
+			$regExps[] = "/($style:[^;]*url *\()([^\"\)')]+)(\))/i";
+		}
 
-		// Make
+		// Callback for regexp replacement
 		$callback = function($matches) use($code) {
-			return
+			if(is_callable($code)) {
-				stripslashes($matches[1]) .
+				$rewritten = $code($matches[2]);
-				str_replace('$URL', stripslashes($matches[2]), $code) .
+			} else {
-				stripslashes($matches[3]);
+				// Expose the $URL variable to be used by the $code expression
+				$URL = $matches[2];
+				$rewritten = eval("return ($code);");
+			}
+			return $matches[1] . $rewritten . $matches[3];
 		};
 		
+		// Execute each expression
 		foreach($regExps as $regExp) {
 			$content = preg_replace_callback($regExp, $callback, $content);
 		}

tests/control/HTTPTest.php

@@ -120,4 +120,82 @@ class HTTPTest extends SapphireTest {
 			HTTP::get_mime_type(FRAMEWORK_DIR.'/tests/control/files/file.psd'));
 		$this->assertEquals('audio/x-wav', HTTP::get_mime_type(FRAMEWORK_DIR.'/tests/control/files/file.wav'));
 	}
+	
+	/**
+	 * Test that absoluteURLs correctly transforms urls within CSS to absolute
+	 */
+	public function testAbsoluteURLsCSS() {
+		$this->withBaseURL('http://www.silverstripe.org/', function($test){
+			
+			// background-image
+			// Note that using /./ in urls is absolutely acceptable
+			$test->assertEquals(
+				'<div style="background-image: url(\'http://www.silverstripe.org/./images/mybackground.gif\');">Content</div>',
+				HTTP::absoluteURLs('<div style="background-image: url(\'./images/mybackground.gif\');">Content</div>')
+			);
+			
+			// background
+			$test->assertEquals(
+				'<div style="background: url(\'http://www.silverstripe.org/images/mybackground.gif\');">Content</div>',
+				HTTP::absoluteURLs('<div style="background: url(\'images/mybackground.gif\');">Content</div>')
+			);
+			
+			// list-style-image
+			$test->assertEquals(
+				'<div style=\'background: url(http://www.silverstripe.org/list.png);\'>Content</div>',
+				HTTP::absoluteURLs('<div style=\'background: url(list.png);\'>Content</div>')
+			);
+			
+			// list-style
+			$test->assertEquals(
+				'<div style=\'background: url("http://www.silverstripe.org/./assets/list.png");\'>Content</div>',
+				HTTP::absoluteURLs('<div style=\'background: url("./assets/list.png");\'>Content</div>')
+			);
+		});
+	}
+	
+	/**
+	 * Test that absoluteURLs correctly transforms urls within html attributes to absolute
+	 */
+	public function testAbsoluteURLsAttributes() {
+		$this->withBaseURL('http://www.silverstripe.org/', function($test){
+			
+			// links
+			$test->assertEquals(
+				'<a href=\'http://www.silverstripe.org/blog/\'>SS Blog</a>',
+				HTTP::absoluteURLs('<a href=\'/blog/\'>SS Blog</a>')
+			);
+			
+			// background
+			// Note that using /./ in urls is absolutely acceptable
+			$test->assertEquals(
+				'<div background="http://www.silverstripe.org/./themes/silverstripe/images/nav-bg-repeat-2.png">SS Blog</div>',
+				HTTP::absoluteURLs('<div background="./themes/silverstripe/images/nav-bg-repeat-2.png">SS Blog</div>')
+			);
+			
+			// image
+			$test->assertEquals(
+				'<img src=\'http://www.silverstripe.org/themes/silverstripe/images/logo-org.png\' />',
+				HTTP::absoluteURLs('<img src=\'themes/silverstripe/images/logo-org.png\' />')
+			);
+			
+			// link
+			$test->assertEquals(
+				'<link href=http://www.silverstripe.org/base.css />',
+				HTTP::absoluteURLs('<link href=base.css />')
+			);
+		});
+	}
+	
+	/**
+	 * Run a test while mocking the base url with the provided value
+	 * @param string $url The base URL to use for this test
+	 * @param callable $callback The test to run
+	 */
+	protected function withBaseURL($url, $callback) {
+		$oldBase = Director::$alternateBaseURL;
+		Director::setBaseURL($url);
+		$callback($this);
+		Director::setBaseURL($oldBase);
+	}
 }