From c5b0174d39e345198ed4eb34867cd9da6b42c0fe Mon Sep 17 00:00:00 2001 From: kellysutton Date: Sat, 30 May 2015 14:07:48 -0700 Subject: [PATCH 1/2] Add support for fully-qualified URLs. Fix up some tests to respect the actual-expected parameter ordering Leave some comments around our hacks --- src/Imgix/UrlHelper.php | 14 +++++++++---- tests/Imgix/Tests/UrlBuilderTest.php | 30 +++++++++++++++++++++++++--- tests/Imgix/Tests/UrlHelperTest.php | 8 ++++---- 3 files changed, 41 insertions(+), 11 deletions(-) diff --git a/src/Imgix/UrlHelper.php b/src/Imgix/UrlHelper.php index ff392db..46440fb 100644 --- a/src/Imgix/UrlHelper.php +++ b/src/Imgix/UrlHelper.php @@ -1,7 +1,7 @@ domain = $domain; - $this->path = substr($path, 0, 1) !== "/" ? ("/" . $path) : $path; + $this->path = substr($path, 0, 4) === "http" ? urlencode($path) : $path; + $this->path = substr($this->path, 0, 1) !== "/" ? ("/" . $this->path) : $this->path; $this->scheme = $scheme; $this->signKey = $signKey; $this->params = $params; @@ -42,7 +43,7 @@ public function getURL() { $query = join("&", $queryPairs); if ($this->signKey) { - $delim = $query === "" ? "" : "?"; + $delim = "?"; $toSign = $this->signKey . $this->path . $delim . $query; $sig = md5($toSign); if ($query) { @@ -62,6 +63,7 @@ public static function encodeURIComponent($str) { return strtr(rawurlencode($str), $revert); } + // TODO: This is almost entirely garbage. We should not be manually building URLs public static function join_url($parts, $encode=true) { $url = ''; if (!empty($parts['scheme'])) { @@ -91,7 +93,11 @@ public static function join_url($parts, $encode=true) { $url .= $parts['path']; } if (isset($parts['query']) && strlen($parts['query']) > 0) { - $url .= '?' . $parts['query']; + if (substr($parts['query'], 0, 2) === "s=") { + $url .= '?&' . $parts['query']; // imgix idiosyncracy for signing URLs when only the signature exists. Our query string must begin with '?&s=' + } else { + $url .= '?' . $parts['query']; + } } if (isset($parts['fragment'])) { $url .= '#' . $parts['fragment']; diff --git a/tests/Imgix/Tests/UrlBuilderTest.php b/tests/Imgix/Tests/UrlBuilderTest.php index c4cf8f7..c64d62d 100644 --- a/tests/Imgix/Tests/UrlBuilderTest.php +++ b/tests/Imgix/Tests/UrlBuilderTest.php @@ -39,7 +39,7 @@ public function testExamplePlain() { $params = array("w" => 100, "h" => 100); $url = $builder->createURL("bridge.png", $params); - $this->assertEquals($url, "http://demos.imgix.net/bridge.png?h=100&w=100"); + $this->assertEquals("http://demos.imgix.net/bridge.png?h=100&w=100", $url); } public function testExamplePlainHttps() { @@ -49,7 +49,7 @@ public function testExamplePlainHttps() { $params = array("w" => 100, "h" => 100); $url = $builder->createURL("bridge.png", $params); - $this->assertEquals($url, "https://demos.imgix.net/bridge.png?h=100&w=100"); + $this->assertEquals("https://demos.imgix.net/bridge.png?h=100&w=100", $url); } public function testExamplePlainSecure() { @@ -58,7 +58,31 @@ public function testExamplePlainSecure() { $params = array("w" => 100, "h" => 100); $url = $builder->createURL("bridge.png", $params); - $this->assertEquals($url, "http://demos.imgix.net/bridge.png?h=100&w=100&s=bb8f3a2ab832e35997456823272103a4"); + $this->assertEquals("http://demos.imgix.net/bridge.png?h=100&w=100&s=bb8f3a2ab832e35997456823272103a4", $url); + } + + public function testWithFullyQualifiedUrl() { + $builder = new UrlBuilder("demos.imgix.net", true); + $builder->setSignKey("test1234"); + $url = $builder->createUrl("http://media.giphy.com/media/jCMq0p94fgBIk/giphy.gif"); + + $this->assertEquals("https://demos.imgix.net/http%3A%2F%2Fmedia.giphy.com%2Fmedia%2FjCMq0p94fgBIk%2Fgiphy.gif?&s=ffc3359566fe1dc6445ad17d17b98951", $url); + } + + public function testWithFullyQualifiedUrlWithSpaces() { + $builder = new UrlBuilder("demos.imgix.net", true); + $builder->setSignKey("test1234"); + $url = $builder->createUrl("https://my-demo-site.com/files/133467012/avatar icon.png"); + + $this->assertEquals("https://demos.imgix.net/https%3A%2F%2Fmy-demo-site.com%2Ffiles%2F133467012%2Favatar+icon.png?&s=f6a4e1504af365564014564f1d7e13de", $url); + } + + public function testWithFullyQualifiedUrlWithParams() { + $builder = new UrlBuilder("demos.imgix.net", true); + $builder->setSignKey("test1234"); + $url = $builder->createUrl("https://my-demo-site.com/files/133467012/avatar icon.png?some=chill¶ms=1"); + + $this->assertEquals("https://demos.imgix.net/https%3A%2F%2Fmy-demo-site.com%2Ffiles%2F133467012%2Favatar+icon.png%3Fsome%3Dchill%26params%3D1?&s=259b9ca6206721752ad7a3ce50f08dd2", $url); } } ?> \ No newline at end of file diff --git a/tests/Imgix/Tests/UrlHelperTest.php b/tests/Imgix/Tests/UrlHelperTest.php index ea4a223..13482b9 100644 --- a/tests/Imgix/Tests/UrlHelperTest.php +++ b/tests/Imgix/Tests/UrlHelperTest.php @@ -8,26 +8,26 @@ public function testHelperBuildSignedURLWithHashMapParams() { $params = array("w" => 500); $uh = new URLHelper("securejackangers.imgix.net", "chester.png", "http", "Q61NvXIy", $params); - $this->assertEquals($uh->getURL(), "http://securejackangers.imgix.net/chester.png?w=500&s=0ddf97bf1a266a1da6c30c6ce327f917"); + $this->assertEquals("http://securejackangers.imgix.net/chester.png?w=500&s=0ddf97bf1a266a1da6c30c6ce327f917", $uh->getURL()); } public function testHelperBuildSignedURLWithHashMapAndNoParams() { $params = array(); $uh = new URLHelper("securejackangers.imgix.net", "chester.png", "http", "Q61NvXIy", $params); - $this->assertEquals($uh->getURL(), "http://securejackangers.imgix.net/chester.png?s=cff7bdfd1b32d82e6b516f7fd3b4f1f4"); + $this->assertEquals("http://securejackangers.imgix.net/chester.png?&s=711dfe95b041008a3c6f460a40052282", $uh->getURL()); } public function testHelperBuildSignedURLWithHashSetterParams() { $uh = new URLHelper("securejackangers.imgix.net", "chester.png", "http", "Q61NvXIy"); $uh->setParameter("w", 500); - $this->assertEquals($uh->getURL(), "http://securejackangers.imgix.net/chester.png?w=500&s=0ddf97bf1a266a1da6c30c6ce327f917"); + $this->assertEquals("http://securejackangers.imgix.net/chester.png?w=500&s=0ddf97bf1a266a1da6c30c6ce327f917", $uh->getURL()); } public function testHelperBuildSignedURLWithHashSetterParamsHttps() { $uh = new URLHelper("securejackangers.imgix.net", "chester.png", "https", "Q61NvXIy"); $uh->setParameter("w", 500); - $this->assertEquals($uh->getURL(), "https://securejackangers.imgix.net/chester.png?w=500&s=0ddf97bf1a266a1da6c30c6ce327f917"); + $this->assertEquals("https://securejackangers.imgix.net/chester.png?w=500&s=0ddf97bf1a266a1da6c30c6ce327f917", $uh->getURL()); } } From 44aaa6414afab1150f76bee359746e4fbe13eea7 Mon Sep 17 00:00:00 2001 From: kellysutton Date: Sat, 30 May 2015 14:16:40 -0700 Subject: [PATCH 2/2] Kill some copypasta to include an http_build_url method, while still protecting ourselves from that imgix signature idiosynracy --- composer.json | 5 +++-- composer.lock | 41 +++++++++++++++++++++++++++++++++-- src/Imgix/UrlHelper.php | 48 ++++++----------------------------------- 3 files changed, 49 insertions(+), 45 deletions(-) diff --git a/composer.json b/composer.json index a922bc0..2fb943f 100644 --- a/composer.json +++ b/composer.json @@ -6,7 +6,8 @@ "imgix" ], "require": { - "php": ">=5.3" + "php": ">=5.3", + "jakeasmith/http_build_url": "^1.0" }, "require-dev": { "phpunit/phpunit": "*" @@ -16,4 +17,4 @@ "Imgix\\": "src/" } } -} \ No newline at end of file +} diff --git a/composer.lock b/composer.lock index 233d0e8..4b7d3a0 100644 --- a/composer.lock +++ b/composer.lock @@ -4,8 +4,45 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#composer-lock-the-lock-file", "This file is @generated automatically" ], - "hash": "63e826441987c24766a36dfa52c7e0e3", - "packages": [], + "hash": "7441d910eae3b22d63087429d01de669", + "packages": [ + { + "name": "jakeasmith/http_build_url", + "version": "1.0.0", + "source": { + "type": "git", + "url": "https://github.com/jakeasmith/http_build_url.git", + "reference": "15bdd686e5178ddfa3e88de60fa585adffb292bb" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/jakeasmith/http_build_url/zipball/15bdd686e5178ddfa3e88de60fa585adffb292bb", + "reference": "15bdd686e5178ddfa3e88de60fa585adffb292bb", + "shasum": "" + }, + "require-dev": { + "phpunit/phpunit": "~3.7" + }, + "type": "library", + "autoload": { + "files": [ + "src/http_build_url.php" + ] + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Jake A. Smith", + "email": "theman@jakeasmith.com" + } + ], + "description": "Provides functionality for http_build_url() to environments without pecl_http.", + "time": "2015-05-06 12:27:20" + } + ], "packages-dev": [ { "name": "doctrine/instantiator", diff --git a/src/Imgix/UrlHelper.php b/src/Imgix/UrlHelper.php index 46440fb..e84a5f1 100644 --- a/src/Imgix/UrlHelper.php +++ b/src/Imgix/UrlHelper.php @@ -55,7 +55,7 @@ public function getURL() { $url_parts = array('scheme' => $this->scheme, 'host' => $this->domain, 'path' => $this->path, 'query' => $query); - return self::join_url($url_parts); + return self::joinURL($url_parts); } public static function encodeURIComponent($str) { @@ -63,47 +63,13 @@ public static function encodeURIComponent($str) { return strtr(rawurlencode($str), $revert); } - // TODO: This is almost entirely garbage. We should not be manually building URLs - public static function join_url($parts, $encode=true) { - $url = ''; - if (!empty($parts['scheme'])) { - $url .= $parts['scheme'] . ':'; - } - if (isset($parts['host'])) { - $url .= '//'; - if (isset($parts['user'])) { - $url .= $parts['user']; - if (isset($parts['pass'])) - $url .= ':' . $parts['pass']; - $url .= '@'; - } - if (preg_match('!^[\da-f]*:[\da-f.:]+$!ui', $parts['host'])) { - $url .= '[' . $parts['host'] . ']'; - } else { - $url .= $parts['host']; - } - if (isset($parts['port'])) { - $url .= ':' . $parts['port']; - } - if (!empty($parts['path']) && $parts['path'][0] != '/') { - $url .= '/'; - } - } - if (!empty($parts['path'])) { - $url .= $parts['path']; - } - if (isset($parts['query']) && strlen($parts['query']) > 0) { - if (substr($parts['query'], 0, 2) === "s=") { - $url .= '?&' . $parts['query']; // imgix idiosyncracy for signing URLs when only the signature exists. Our query string must begin with '?&s=' - } else { - $url .= '?' . $parts['query']; - } - } - if (isset($parts['fragment'])) { - $url .= '#' . $parts['fragment']; + public static function joinURL($parts) { + + // imgix idiosyncracy for signing URLs when only the signature exists. Our query string must begin with '?&s=' + if (substr($parts['query'], 0, 2) === "s=") { + $parts['query'] = "&" . $parts['query']; } - return $url; + return http_build_url($parts); } - }