From 18e163d985dc00ace381c26e812f5c8a0525e286 Mon Sep 17 00:00:00 2001 From: JorisDebonnet Date: Tue, 28 Jul 2015 03:54:49 +0200 Subject: [PATCH] Url-safe alternative for base64_encode in resampled Image filenames --- core/Convert.php | 33 +++++++++++++-- model/Image.php | 12 +++--- tests/core/ConvertTest.php | 65 +++++++++++++++++++++++++++++ tests/forms/HtmlEditorFieldTest.php | 2 +- tests/model/ImageTest.php | 8 ++-- 5 files changed, 106 insertions(+), 14 deletions(-) diff --git a/core/Convert.php b/core/Convert.php index 73235e79e..b230abc31 100644 --- a/core/Convert.php +++ b/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 + ); + } } diff --git a/model/Image.php b/model/Image.php index 72e7d9a9c..e17a79556 100644 --- a/model/Image.php +++ b/model/Image.php @@ -701,7 +701,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 @@ -711,7 +711,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)) { @@ -836,11 +836,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{$generateFuncs})(?P" . $base64Match . ")\-)+" + 'FullPattern' => "/^((?P{$generateFuncs})(?P" . $base64url_match . ")\-)+" . preg_quote($filename) . "$/i", - 'GeneratorPattern' => "/(?P{$generateFuncs})(?P" . $base64Match . ")\-/i" + 'GeneratorPattern' => "/(?P{$generateFuncs})(?P" . $base64url_match . ")\-/i" ); } @@ -877,7 +877,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 diff --git a/tests/core/ConvertTest.php b/tests/core/ConvertTest.php index 3bdbad71e..2ccf7e19b 100644 --- a/tests/core/ConvertTest.php +++ b/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 tag.'; $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 = <<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)) + ); + } } diff --git a/tests/forms/HtmlEditorFieldTest.php b/tests/forms/HtmlEditorFieldTest.php index e828864b6..52bb3aa79 100644 --- a/tests/forms/HtmlEditorFieldTest.php +++ b/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.'); diff --git a/tests/model/ImageTest.php b/tests/model/ImageTest.php index e9d87d8dc..5ba108662 100644 --- a/tests/model/ImageTest.php +++ b/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());