- Patch #443200 by c960657: fixed user pictures not working with private files.

7 files changed, 77 insertions, 49 deletions

diff --git a/modules/image/image.module b/modules/image/image.module
index 44ed325c9..a35a9e4dd 100644
--- a/modules/image/image.module
+++ b/modules/image/image.module
@@ -257,12 +257,12 @@ function image_file_download($uri) {
     // Get the style name from the second part.
     $style_name = array_shift($args);
     // Then the remaining parts are the path to the image.
-    $original_path = implode('/', $args);
+    $original_uri = file_uri_scheme($uri) . '://' . implode('/', $args);
 
     // Check that the file exists and is an image.
     if ($info = image_get_info($uri)) {
       // Check the permissions of the original to grant access to this image.
-      $headers = module_invoke_all('file_download', $original_path);
+      $headers = module_invoke_all('file_download', $original_uri);
       if (!in_array(-1, $headers)) {
         return array(
           // Send headers describing the image's size, and MIME-type...
@@ -272,7 +272,7 @@ function image_file_download($uri) {
           // value we/ use for the mod_expires settings in .htaccess) and
           // ensure that caching proxies do not share the image with other
           // users.
-          'Expires' => gmdate(DATE_RFC1123, time() + 1209600),
+          'Expires' => gmdate(DATE_RFC1123, REQUEST_TIME + 1209600),
           'Cache-Control' => 'max-age=1209600, private, must-revalidate',
         );
       }
@@ -755,7 +755,7 @@ function image_style_url($style_name, $path) {
 }
 
 /**
- * Return a relative path to an image when using a style.
+ * Return the URI of an image when using a style.
  *
  * The path returned by this function may not exist. The default generation
  * method only creates images when they are requested by a user's browser.
@@ -765,12 +765,18 @@ function image_style_url($style_name, $path) {
  * @param $uri
  *   The URI or path to the image.
  * @return
- *   The path to an image style image relative to Drupal's root.
+ *   The URI to an image style image.
  * @see image_style_url()
  */
 function image_style_path($style_name, $uri) {
-  $path = ($path = file_uri_target($uri)) ? $path : $uri;
-  $scheme = ($scheme = file_uri_scheme($uri)) ? $scheme : variable_get('file_default_scheme', 'public');
+  $scheme = file_uri_scheme($uri);
+  if ($scheme) {
+    $path = file_uri_target($uri);
+  }
+  else {
+    $path = $uri;
+    $scheme = variable_get('file_default_scheme', 'public');
+  }
   return $scheme . '://styles/' . $style_name . '/' . $path;
 }
 
diff --git a/modules/image/image.test b/modules/image/image.test
index 91091940d..1c8e1a5b0 100644
--- a/modules/image/image.test
+++ b/modules/image/image.test
@@ -47,48 +47,70 @@ class ImageStylesPathAndUrlUnitTest extends DrupalWebTestCase {
     parent::setUp();
 
     $this->style_name = 'style_foo';
-    $this->scheme = 'public';
     image_style_save(array('name' => $this->style_name));
-
-    // Create the directories for the styles.
-    $status = file_prepare_directory($d = file_directory_path() . '/styles/' . $this->style_name, FILE_CREATE_DIRECTORY);
-    $this->assertNotIdentical(FALSE, $status, t('Created the directory for the generated images for the test style.' ));
-
-    // Create a working copy of the file.
-    $file = reset($this->drupalGetTestFiles('image'));
-    $this->image_info = image_get_info($file->uri);
-    $this->image_filepath = file_unmanaged_copy($file->uri, NULL, FILE_EXISTS_RENAME);
-    $this->assertNotIdentical(FALSE, $this->image_filepath, t('Created the without generated image file.'));
   }
 
   /**
    * Test image_style_path().
    */
   function testImageStylePath() {
-    $actual = image_style_path($this->style_name, $this->image_filepath);
-    $expected = $this->scheme . '://styles/' . $this->style_name . '/' . basename($this->image_filepath);
-    $this->assertEqual($actual, $expected, t('Got the path for a file.'));
+    $actual = image_style_path($this->style_name, 'public://foo/bar.gif');
+    $expected = 'public://styles/' . $this->style_name . '/foo/bar.gif';
+    $this->assertEqual($actual, $expected, t('Got the path for a file URI.'));
+
+    $actual = image_style_path($this->style_name, 'foo/bar.gif');
+    $expected = 'public://styles/' . $this->style_name . '/foo/bar.gif';
+    $this->assertEqual($actual, $expected, t('Got the path for a relative file path.'));
+  }
+
+  /**
+   * Test image_style_url() with a file using the "public://" scheme.
+   */
+  function testImageStyleUrlAndPathPublic() {
+    $this->_testImageStyleUrlAndPath('public');
+  }
+
+  /**
+   * Test image_style_url() with a file using the "private://" scheme.
+   */
+  function testImageStyleUrlAndPathPrivate() {
+    $this->_testImageStyleUrlAndPath('private');
   }
 
   /**
    * Test image_style_url().
    */
-  function testImageStyleUrl() {
+  function _testImageStyleUrlAndPath($scheme) {
+    // Make the default scheme neither "public" nor "private" to verify the
+    // functions work for other than the default scheme. 
+    variable_set('file_default_scheme', 'temporary');
+    file_prepare_directory($d = 'temporary://', FILE_CREATE_DIRECTORY);
+
+    // Create the directories for the styles.
+    $status = file_prepare_directory($d = $scheme . '://styles/' . $this->style_name, FILE_CREATE_DIRECTORY);
+    $this->assertNotIdentical(FALSE, $status, t('Created the directory for the generated images for the test style.' ));
+
+    // Create a working copy of the file.
+    $file = reset($this->drupalGetTestFiles('image'));
+    $image_info = image_get_info($file->uri);
+    $original_uri = file_unmanaged_copy($file->uri, $scheme . '://', FILE_EXISTS_RENAME);
+    $this->assertNotIdentical(FALSE, $original_uri, t('Created the generated image file.'));
+
     // Get the URL of a file that has not been generated yet and try to access
     // it before image_style_url has been called.
-    $generated_path = $this->scheme . '://styles/' . $this->style_name . '/' . basename($this->image_filepath);
-    $this->assertFalse(file_exists($generated_path), t('Generated file does not exist.'));
-    $expected_generate_url = url('image/generate/' . $this->style_name . '/' . $this->scheme . '/' . basename($this->image_filepath), array('absolute' => TRUE));
+    $generated_uri = $scheme . '://styles/' . $this->style_name . '/' . basename($original_uri);
+    $this->assertFalse(file_exists($generated_uri), t('Generated file does not exist.'));
+    $expected_generate_url = url('image/generate/' . $this->style_name . '/' . $scheme . '/' . basename($original_uri), array('absolute' => TRUE));
     $this->drupalGet($expected_generate_url);
     $this->assertResponse(403, t('Access to generate URL was denied.'));
 
     // Check that a generate URL is returned.
-    $actual_generate_url = image_style_url($this->style_name, $this->image_filepath);
+    $actual_generate_url = image_style_url($this->style_name, $original_uri);
     $this->assertEqual($actual_generate_url, $expected_generate_url, t('Got the generate URL for a non-existent file.'));
 
     // Fetch the URL that generates the file while another process appears to
     // be generating the same file (this is signaled using a lock).
-    $lock_name = 'image_style_generate:' . $this->style_name . ':' . md5($this->image_filepath);
+    $lock_name = 'image_style_generate:' . $this->style_name . ':' . md5($original_uri);
     $this->assertTrue(lock_acquire($lock_name), t('Lock was acquired.'));
     $this->drupalGet($expected_generate_url);
     $this->assertResponse(503, t('Service Unavailable response received.'));
@@ -97,21 +119,21 @@ class ImageStylesPathAndUrlUnitTest extends DrupalWebTestCase {
 
     // Fetch the URL that generates the file.
     $this->drupalGet($expected_generate_url);
-    $this->assertTrue(file_exists($generated_path), t('Generated file was created.'));
-    $this->assertRaw(file_get_contents($generated_path), t('URL returns expected file.'));
-    $generated_image_info = image_get_info($generated_path);
+    $this->assertTrue(file_exists($generated_uri), t('Generated file was created.'));
+    $this->assertRaw(file_get_contents($generated_uri), t('URL returns expected file.'));
+    $generated_image_info = image_get_info($generated_uri);
     $this->assertEqual($this->drupalGetHeader('Content-Type'), $generated_image_info['mime_type'], t('Expected Content-Type was reported.'));
     $this->assertEqual($this->drupalGetHeader('Content-Length'), $generated_image_info['file_size'], t('Expected Content-Length was reported.'));
     $this->assertTrue(lock_may_be_available($lock_name), t('Lock was released.'));
 
     // Check that the URL points directly to the generated file.
-    $expected_generated_url = file_create_url($generated_path);
-    $actual_generated_url = image_style_url($this->style_name, $this->image_filepath);
+    $expected_generated_url = file_create_url($generated_uri);
+    $actual_generated_url = image_style_url($this->style_name, $original_uri);
     $this->drupalGet($expected_generated_url);
     $this->assertEqual($actual_generated_url, $expected_generated_url, t('Got the download URL for an existing file.'));
-    $this->assertRaw(file_get_contents($generated_path), t('URL returns expected file.'));
-    $this->assertEqual($this->drupalGetHeader('Content-Type'), $this->image_info['mime_type'], t('Expected Content-Type was reported.'));
-    $this->assertEqual($this->drupalGetHeader('Content-Length'), $this->image_info['file_size'], t('Expected Content-Length was reported.'));
+    $this->assertRaw(file_get_contents($generated_uri), t('URL returns expected file.'));
+    $this->assertEqual($this->drupalGetHeader('Content-Type'), $image_info['mime_type'], t('Expected Content-Type was reported.'));
+    $this->assertEqual($this->drupalGetHeader('Content-Length'), $image_info['file_size'], t('Expected Content-Length was reported.'));
   }
 }
 
diff --git a/modules/simpletest/drupal_web_test_case.php b/modules/simpletest/drupal_web_test_case.php
index bf8e8e78d..398e63d0b 100644
--- a/modules/simpletest/drupal_web_test_case.php
+++ b/modules/simpletest/drupal_web_test_case.php
@@ -1175,7 +1175,7 @@ class DrupalWebTestCase extends DrupalTestCase {
 
     if (preg_match('/simpletest\d+/', $db_prefix)) {
       // Delete temporary files directory.
-      file_unmanaged_delete_recursive(file_directory_path());
+      file_unmanaged_delete_recursive($this->originalFileDirectory . '/' . $db_prefix);
 
       // Remove all prefixed tables (all the tables in the schema).
       $schema = drupal_get_schema(NULL, TRUE);
diff --git a/modules/simpletest/tests/file_test.module b/modules/simpletest/tests/file_test.module
index 567508a4f..e9d4ce1f8 100644
--- a/modules/simpletest/tests/file_test.module
+++ b/modules/simpletest/tests/file_test.module
@@ -221,8 +221,8 @@ function file_test_file_validate($file) {
 /**
  * Implement hook_file_download().
  */
-function file_test_file_download($file) {
-  _file_test_log_call('download', array($file));
+function file_test_file_download($uri) {
+  _file_test_log_call('download', array($uri));
   return _file_test_get_return('download');
 }
 
diff --git a/modules/system/system.api.php b/modules/system/system.api.php
index 3a619a95b..a1695071a 100644
--- a/modules/system/system.api.php
+++ b/modules/system/system.api.php
@@ -1547,8 +1547,8 @@ function hook_file_delete($file) {
  * private file download method is selected. Modules can also provide headers
  * to specify information like the file's name or MIME type.
  *
- * @param $filepath
- *   String of the file's path.
+ * @param $uri
+ *   The URI of the file.
  * @return
  *   If the user does not have permission to access the file, return -1. If the
  *   user has permission, return an array with the appropriate headers. If the
@@ -1558,12 +1558,12 @@ function hook_file_delete($file) {
  * @see file_download()
  * @see upload_file_download()
  */
-function hook_file_download($filepath) {
+function hook_file_download($uri) {
   // Check if the file is controlled by the current module.
-  if (!file_prepare_directory($filepath)) {
-    $filepath = FALSE;
+  if (!file_prepare_directory($uri)) {
+    $uri = FALSE;
   }
-  $result = db_query("SELECT f.* FROM {file} f INNER JOIN {upload} u ON f.fid = u.fid WHERE uri = :filepath", array('filepath' => $filepath));
+  $result = db_query("SELECT f.* FROM {file} f INNER JOIN {upload} u ON f.fid = u.fid WHERE uri = :uri", array('uri' => $uri));
   foreach ($result as $file) {
     if (!user_access('view uploaded files')) {
       return -1;
diff --git a/modules/upload/upload.module b/modules/upload/upload.module
index 391d064b3..5a0c0c1c5 100644
--- a/modules/upload/upload.module
+++ b/modules/upload/upload.module
@@ -150,8 +150,8 @@ function _upload_file_limits($user) {
 /**
  * Implement hook_file_download().
  */
-function upload_file_download($filepath) {
-  $file = db_query("SELECT f.*, u.nid FROM {file} f INNER JOIN {upload} u ON f.fid = u.fid WHERE uri = :path", array(':path' => $filepath))->fetchObject();
+function upload_file_download($uri) {
+  $file = db_query("SELECT f.*, u.nid FROM {file} f INNER JOIN {upload} u ON f.fid = u.fid WHERE uri = :uri", array(':uri' => $uri))->fetchObject();
 
   if ($file && user_access('view uploaded files') && ($node = node_load($file->nid)) && node_access('view', $node)) {
     return array(
diff --git a/modules/user/user.module b/modules/user/user.module
index 424916ef4..5e4f60ade 100644
--- a/modules/user/user.module
+++ b/modules/user/user.module
@@ -747,9 +747,9 @@ function user_permission() {
  *
  * Ensure that user pictures (avatars) are always downloadable.
  */
-function user_file_download($filepath) {
-  if (strpos(file_uri_target($filepath), variable_get('user_picture_path', 'pictures') . '/picture-') === 0) {
-    $info = image_get_info($filepath);
+function user_file_download($uri) {
+  if (strpos(file_uri_target($uri), variable_get('user_picture_path', 'pictures') . '/picture-') === 0) {
+    $info = image_get_info($uri);
     return array('Content-Type' => $info['mime_type']);
   }
 }