Fix setting invalid hosts and paths for CloudFront.

justafish · May 1, 2015 · c196ee4 · c196ee4
1 parent 83038a0
commit c196ee4
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 16 deletions.
diff --git a/src/StreamWrapper.php b/src/StreamWrapper.php
@@ -241,26 +241,15 @@ public function getExternalUrl() {
     // operation.
     // $args = array_merge($args, module_invoke_all('amazons3_url_info', $local_path, $args));
 
-    // Generate a standard URL.
-    $url = Url::factory(static::$client->getObjectUrl($this->uri->getBucket(), $path, $expiry, $args));
-
-    // CNAME support.
-    if ($this->config->getDomain() != $url->getHost()) {
-      $url->setHost($this->config->getDomain());
-    }
-
-    // Real CloudFront credentials are required to test this, so we ignore
-    // testing this.
     // @codeCoverageIgnoreStart
     if ($expiry && $this->config->isCloudFront()) {
-      $cf = $this->config->getCloudFront();
-      $options = array(
-        'url' => (string) $url,
-        'expires' => $expiry,
-      );
-      $url = Url::factory($cf->getSignedUrl($options));
+      $url = $this->getCloudFrontUrl($path, $expiry);
     }
     // @codeCoverageIgnoreEnd
+    else {
+      // Generate a standard URL.
+      $url = $this->getS3Url($path, $expiry, $args);
+    }
 
     return (string) $url;
   }
@@ -426,4 +415,69 @@ protected function useTorrent() {
   protected function usePresigned() {
     return $this->config->getPresignedPaths()->match($this->getLocalPath());
   }
+
+  /**
+   * Replace the host in a URL with the configured domain.
+   *
+   * @param Url $url
+   *   The URL to modify.
+   */
+  protected function injectCname($url) {
+    if ($this->config->getDomain() != $url->getHost()) {
+      $url->setHost($this->config->getDomain());
+    }
+  }
+
+  /**
+   * Get a CloudFront URL for an S3 key.
+   *
+   * @param $key
+   *   The S3 object key.
+   * @param int $expiry
+   *   (optional) Expiry time for the URL, as a Unix timestamp.
+   * @return \Guzzle\Http\Url
+   *   The CloudFront URL.
+   */
+  protected function getCloudFrontUrl($key, $expiry = NULL) {
+    // Real CloudFront credentials are required to test this, so we ignore
+    // testing this.
+    // @codeCoverageIgnoreStart
+    $cf = $this->config->getCloudFront();
+    $url = new Url('https', $this->config->getDomain());
+    $url->setPath($key);
+    $this->injectCname($url);
+    $options = array(
+      'url' => (string) $url,
+      'expires' => $expiry,
+    );
+    $url = Url::factory($cf->getSignedUrl($options));
+    return $url;
+    // @codeCoverageIgnoreEnd
+  }
+
+  /**
+   * Get a regular S3 URL for a key.
+   *
+   * @param string $key
+   *   The S3 object key.
+   * @param int $expiry
+   *   (optional) Expiry time for the URL, as a Unix timestamp.
+   * @param array $args
+   *   (optional) Array of additional arguments to pass to getObjectUrl().
+   *
+   * @return \Guzzle\Http\Url
+   *   An https URL to access the key.
+   */
+  protected function getS3Url($key, $expiry = NULL, array $args = array()) {
+    $url = Url::factory(
+      static::$client->getObjectUrl(
+        $this->uri->getBucket(),
+        $key,
+        $expiry,
+        $args
+      )
+    );
+    $this->injectCname($url);
+    return $url;
+  }
 }
diff --git a/tests/StreamWrapperTest.php b/tests/StreamWrapperTest.php
@@ -191,6 +191,7 @@ public function testExternalImageStyleUri() {
    * @covers \Drupal\amazons3\StreamWrapper::getExternalUrl
    * @covers \Drupal\amazons3\StreamWrapper::getLocalPath
    * @covers \Drupal\amazons3\StreamWrapper::forceDownload
+   * @covers \Drupal\amazons3\StreamWrapper::getS3Url
    */
   public function testExternalUri() {
     $wrapper = new StreamWrapper();
@@ -329,6 +330,7 @@ public function testBasenameUriNotSet() {
 
   /**
    * @covers \Drupal\amazons3\StreamWrapper::getExternalUrl
+   * @covers \Drupal\amazons3\StreamWrapper::getS3Url
    * @covers \Drupal\amazons3\StreamWrapper::getContentDispositionAttachment
    * @covers \Drupal\amazons3\StreamWrapper::forceDownload
    */
@@ -364,6 +366,7 @@ public function testSaveAsExcluded() {
    * @covers \Drupal\amazons3\StreamWrapper::getExternalUrl
    * @covers \Drupal\amazons3\StreamWrapper::getContentDispositionAttachment
    * @covers \Drupal\amazons3\StreamWrapper::forceDownload
+   * @covers \Drupal\amazons3\StreamWrapper::getS3Url
    */
   public function testSaveAsAll() {
     $config = StreamWrapperConfiguration::fromConfig([
@@ -402,6 +405,7 @@ public function testAttachmentSpace() {
    *
    * @covers \Drupal\amazons3\StreamWrapper::getExternalUrl
    * @covers \Drupal\amazons3\StreamWrapper::useTorrent
+   * @covers \Drupal\amazons3\StreamWrapper::getS3Url
    */
   public function testTorrentPath() {
     $config = StreamWrapperConfiguration::fromConfig([
@@ -419,6 +423,7 @@ public function testTorrentPath() {
   /**
    * @covers \Drupal\amazons3\StreamWrapper::getExternalUrl
    * @covers \Drupal\amazons3\StreamWrapper::usePresigned
+   * @covers \Drupal\amazons3\StreamWrapper::getS3Url
    */
   public function testPresignedPath() {
     $config = StreamWrapperConfiguration::fromConfig([
@@ -443,6 +448,8 @@ public function testPresignedPath() {
 
   /**
    * @covers \Drupal\amazons3\StreamWrapper::getExternalUrl
+   * @covers \Drupal\amazons3\StreamWrapper::injectCname
+   * @covers \Drupal\amazons3\StreamWrapper::getS3Url
    */
   public function testCustomDomain() {
     $config = StreamWrapperConfiguration::fromConfig([
@@ -459,6 +466,7 @@ public function testCustomDomain() {
 
   /**
    * @covers \Drupal\amazons3\StreamWrapper::getExternalUrl
+   * @covers \Drupal\amazons3\StreamWrapper::injectCname
    */
   public function testNoCustomDomain() {
     $config = StreamWrapperConfiguration::fromConfig([