Merge pull request #4443 from JorisDebonnet/base64url

Url-safe alternative for base64_encode in resampled Image filenames

core/Convert.php

@@ -290,8 +290,8 @@ class Convert {
 	/**
 	 * Create a link if the string is a valid URL
 	 *
-	 * @param string The string to linkify
-	 * @return A link to the URL if string is a URL
+	 * @param string $string The string to linkify
+	 * @return string A link to the URL if string is a URL
 	 */
 	public static function linkIfMatch($string) {
 		if( preg_match( '/^[a-z+]+\:\/\/[a-zA-Z0-9$-_.+?&=!*\'()%]+$/', $string ) )
@@ -305,7 +305,9 @@ class Convert {
 	 *
 	 * @param string $data Input data
 	 * @param bool $preserveLinks
-	 * @param int $wordwrap
+	 * @param int $wordWrap
+	 * @param array $config
+	 * @return string
 	 */
 	public static function html2raw($data, $preserveLinks = false, $wordWrap = 0, $config = null) {
 		$defaultConfig = array(
@@ -414,8 +416,33 @@ class Convert {
 	 * sequences including \r, \r\n, \n, or unicode newline characters
 	 * @param string $nl The newline sequence to normalise to. Defaults to that
 	 * specified by the current OS
+	 * @return string
 	 */
 	public static function nl2os($data, $nl = PHP_EOL) {
 		return preg_replace('~\R~u', $nl, $data);
 	}
+
+	/**
+	 * Encode a value into a string that can be used as part of a filename. 
+	 * All string data must be UTF-8 encoded.
+	 *
+	 * @param mixed $val Value to be encoded
+	 * @return string
+	 */
+	public static function base64url_encode($val) {
+		return rtrim(strtr(base64_encode(json_encode($val)), '+/', '~_'), '=');
+	}
+
+	/**
+	 * Decode a value that was encoded with Convert::base64url_encode.
+	 *
+	 * @param string $val Value to be decoded
+	 * @return mixed Original value
+	 */
+	public static function base64url_decode($val) {
+		return json_decode(
+			base64_decode(str_pad(strtr($val, '~_', '+/'), strlen($val) % 4, '=', STR_PAD_RIGHT)),
+			true
+		);
+	}
 }

model/Image.php

@@ -689,7 +689,7 @@ class Image extends File implements Flushable {
 	}
 
 	/**
-	 * Return the filename for the cached image, given it's format name and arguments.
+	 * Return the filename for the cached image, given its format name and arguments.
 	 * @param string $format The format name.
 	 * @return string
 	 * @throws InvalidArgumentException
@@ -699,7 +699,7 @@ class Image extends File implements Flushable {
 		array_shift($args);
 		$folder = $this->ParentID ? $this->Parent()->Filename : ASSETS_DIR . "/";
 
-		$format = $format . base64_encode(json_encode($args, JSON_NUMERIC_CHECK));
+		$format = $format . Convert::base64url_encode($args);
 		$filename = $format . "-" . $this->Name;
 		$patterns = $this->getFilenamePatterns($this->Name);
 		if (!preg_match($patterns['FullPattern'], $filename)) {
@@ -824,11 +824,11 @@ class Image extends File implements Flushable {
 		}
 		// All generate functions may appear any number of times in the image cache name.
 		$generateFuncs = implode('|', $generateFuncs);
-		$base64Match = "[a-zA-Z0-9\/\r\n+]*={0,2}";
+		$base64url_match = "[a-zA-Z0-9_~]*={0,2}";
 		return array(
-				'FullPattern' => "/^((?P<Generator>{$generateFuncs})(?P<Args>" . $base64Match . ")\-)+"
+				'FullPattern' => "/^((?P<Generator>{$generateFuncs})(?P<Args>" . $base64url_match . ")\-)+"
 									. preg_quote($filename) . "$/i",
-				'GeneratorPattern' => "/(?P<Generator>{$generateFuncs})(?P<Args>" . $base64Match . ")\-/i"
+				'GeneratorPattern' => "/(?P<Generator>{$generateFuncs})(?P<Args>" . $base64url_match . ")\-/i"
 		);
 	}
 
@@ -865,7 +865,7 @@ class Image extends File implements Flushable {
 					$generatorArray = array();
 					foreach ($subMatches as $singleMatch) {
 						$generatorArray[] = array('Generator' => $singleMatch['Generator'],
-						'Args' => json_decode(base64_decode($singleMatch['Args'])));
+						'Args' => Convert::base64url_decode($singleMatch['Args']));
 					}
 
 						// Using array_reverse is important, as a cached image will

tests/core/ConvertTest.php

@@ -36,6 +36,9 @@ class ConvertTest extends SapphireTest {
 			'Normal text is not escaped');
 	}
 
+	/**
+	 * Tests {@link Convert::html2raw()}
+	 */
 	public function testHtml2raw() {
 		$val1 = 'This has a <strong>strong tag</strong>.';
 		$this->assertEquals('This has a *strong tag*.', Convert::html2raw($val1),
@@ -139,6 +142,9 @@ PHP
 		$this->assertEquals('This is some normal text.', Convert::xml2raw($val2), 'Normal text is not escaped');
 	}
 
+	/**
+	 * Tests {@link Convert::xml2raw()}
+	 */
 	public function testArray2JSON() {
 		$val = array(
 			'Joe' => 'Bloggs',
@@ -152,6 +158,9 @@ PHP
 			'Array is encoded in JSON');
 	}
 
+	/**
+	 * Tests {@link Convert::json2array()}
+	 */
 	public function testJSON2Array() {
 		$val = '{"Joe":"Bloggs","Tom":"Jones","My":{"Complicated":"Structure"}}';
 		$decoded = Convert::json2array($val);
@@ -161,6 +170,9 @@ PHP
 		$this->assertContains('Structure', $decoded['My']['Complicated']);
 	}
 
+	/**
+	 * Tests {@link Convert::testJSON2Obj()}
+	 */
 	public function testJSON2Obj() {
 		$val = '{"Joe":"Bloggs","Tom":"Jones","My":{"Complicated":"Structure"}}';
 		$obj = Convert::json2obj($val);
@@ -170,6 +182,7 @@ PHP
 	}
 
 	/**
+	 * Tests {@link Convert::testRaw2URL()}
 	 * @todo test toASCII()
 	 */
 	public function testRaw2URL() {
@@ -196,6 +209,9 @@ PHP
 		$this->assertEquals($expected, $actual, $message);
 	}
 
+	/**
+	 * Tests {@link Convert::nl2os()}
+	 */
 	public function testNL2OS() {
 
 		foreach(array("\r\n", "\r", "\n") as $nl) {
@@ -229,6 +245,9 @@ PHP
 		}
 	}
 
+	/**
+	 * Tests {@link Convert::raw2js()}
+	 */
 	public function testRaw2JS() {
 		// Test attempt to break out of string
 		$this->assertEquals(
@@ -255,6 +274,9 @@ PHP
 		);
 	}
 
+	/**
+	 * Tests {@link Convert::raw2json()}
+	 */
 	public function testRaw2JSON() {
 
 		// Test object
@@ -281,6 +303,9 @@ PHP
 		);
 	}
 
+	/**
+	 * Tests {@link Convert::xml2array()}
+	 */
 	public function testXML2Array() {
 		// Ensure an XML file at risk of entity expansion can be avoided safely
 		$inputXML = <<<XML
@@ -331,4 +356,44 @@ XML
 			$result
 		);
 	}
+
+	/**
+	 * Tests {@link Convert::base64url_encode()} and {@link Convert::base64url_decode()}
+	 */
+	public function testBase64url() {
+		$data = 'Wëīrð characters ☺ such as ¤Ø¶÷╬';
+		// This requires this test file to have UTF-8 character encoding
+		$this->assertEquals(
+			$data, 
+			Convert::base64url_decode(Convert::base64url_encode($data))
+		);
+		
+		$data = 654.423;
+		$this->assertEquals(
+			$data,
+			Convert::base64url_decode(Convert::base64url_encode($data))
+		);
+		
+		$data = true;
+		$this->assertEquals(
+			$data,
+			Convert::base64url_decode(Convert::base64url_encode($data))
+		);
+		
+		$data = array('simple','array','¤Ø¶÷╬');
+		$this->assertEquals(
+			$data,
+			Convert::base64url_decode(Convert::base64url_encode($data))
+		);
+		
+		$data = array(
+			'a'  => 'associative',
+			4    => 'array',
+			'☺' => '¤Ø¶÷╬'
+		);
+		$this->assertEquals(
+			$data,
+			Convert::base64url_decode(Convert::base64url_encode($data))
+		);
+	}
 }

tests/forms/HtmlEditorFieldTest.php

@@ -88,7 +88,7 @@ class HtmlEditorFieldTest extends FunctionalTest {
 		$this->assertEquals(10, (int)$xml[0]['width'], 'Width tag of resized image is set.');
 		$this->assertEquals(20, (int)$xml[0]['height'], 'Height tag of resized image is set.');
 
-		$neededFilename = 'assets/_resampled/ResizedImage' . base64_encode(json_encode(array(10,20))) .
+		$neededFilename = 'assets/_resampled/ResizedImage' . Convert::base64url_encode(array(10,20)) .
 			'-HTMLEditorFieldTest_example.jpg';
 
 		$this->assertEquals($neededFilename, (string)$xml[0]['src'], 'Correct URL of resized image is set.');

tests/model/ImageTest.php

@@ -287,7 +287,7 @@ class ImageTest extends SapphireTest {
 		$imageFirst = $image->Pad(200,200,'CCCCCC');
 		$imageFilename = $imageFirst->getFullPath();
 			// Encoding of the arguments is duplicated from cacheFilename
-		$neededPart = 'Pad' . base64_encode(json_encode(array(200,200,'CCCCCC')));
+		$neededPart = 'Pad' . Convert::base64url_encode(array(200,200,'CCCCCC'));
 		$this->assertContains($neededPart, $imageFilename, 'Filename for cached image is correctly generated');
 	}
 
@@ -310,7 +310,7 @@ class ImageTest extends SapphireTest {
 
 		$imageThird = $imageSecond->Pad(600,600,'0F0F0F');
 		// Encoding of the arguments is duplicated from cacheFilename
-		$argumentString = base64_encode(json_encode(array(600,600,'0F0F0F')));
+		$argumentString = Convert::base64url_encode(array(600,600,'0F0F0F'));
 		$this->assertNotNull($imageThird);
 		$this->assertContains($argumentString, $imageThird->getFullPath(),
 			'Image contains background color for padded resizement');
@@ -352,8 +352,8 @@ class ImageTest extends SapphireTest {
 		$this->assertTrue(file_exists($p), 'Resized image exists after creation call');
 
 		// Encoding of the arguments is duplicated from cacheFilename
-		$oldArgumentString = base64_encode(json_encode(array(200)));
-		$newArgumentString = base64_encode(json_encode(array(300)));
+		$oldArgumentString = Convert::base64url_encode(array(200));
+		$newArgumentString = Convert::base64url_encode(array(300));
 
 		$newPath = str_replace($oldArgumentString, $newArgumentString, $p);
 		$newRelative = str_replace($oldArgumentString, $newArgumentString, $image_generated->getFileName());