Permalink
Browse files

API HTTP::urlRewriter with (string)$code deprecated in 3.1. Fixed reg…

…ressions 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 c56a80d

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.
  • Loading branch information...
1 parent 3439e30 commit 1ca15d039915bfea606316bfed922246dc3d7235 @tractorcow tractorcow committed with chillu Jan 23, 2013
Showing with 126 additions and 16 deletions.
  1. +48 −16 control/HTTP.php
  2. +78 −0 tests/control/HTTPTest.php
View
@@ -41,20 +41,43 @@ public static function filename2url($filename) {
*/
public static function absoluteURLs($html) {
$html = str_replace('$CurrentPageURL', $_SERVER['REQUEST_URI'], $html);
- return HTTP::urlRewriter($html, '(substr($URL,0,1) == "/") ? ( Director::protocolAndHost() . $URL ) :'
- . ' ( (preg_match("/^[A-Za-z]+:/", $URL)) ? $URL : Director::absoluteBaseURL() . $URL )' );
+ return HTTP::urlRewriter($html, function($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'
- * 'myRewriter($URL)'
- * '(substr($URL,0,1)=="/") ? "../" . substr($URL,1) : $URL'
+ * <ul>
+ * <li><code>'"../../" . $URL'</code></li>
+ * <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 @@ public static function urlRewriter($content, $code) {
$regExps[] = "/(<{$tagPrefix}[^>]*$attrib *= *')([^']*)(')/i";
$regExps[] = "/(<{$tagPrefix}[^>]*$attrib *= *)([^\"' ]*)( )/i";
}
- $regExps[] = '/(background-image:[^;]*url *\()([^)]+)(\))/i';
- $regExps[] = '/(background:[^;]*url *\()([^)]+)(\))/i';
- $regExps[] = '/(list-style-image:[^;]*url *\()([^)]+)(\))/i';
- $regExps[] = '/(list-style:[^;]*url *\()([^)]+)(\))/i';
+ // Replace css styles
+ // @todo - http://www.css3.info/preview/multiple-backgrounds/
+ $styles = array('background-image', 'background', 'list-style-image', 'list-style', 'content');
+ 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
- stripslashes($matches[1]) .
- str_replace('$URL', stripslashes($matches[2]), $code) .
- stripslashes($matches[3]);
+ if(is_callable($code)) {
+ $rewritten = $code($matches[2]);
+ } else {
+ // 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);
}
View
@@ -120,4 +120,82 @@ public function testGetMimeType() {
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);
+ }
}

0 comments on commit 1ca15d0

Please sign in to comment.