From 1a4a3ec577c976e22fd2e93a7339afe6eddf24db Mon Sep 17 00:00:00 2001 From: Dries Buytaert Date: Mon, 20 Jul 2009 21:52:49 +0000 Subject: [PATCH] - Patch #524530 by c960657: fixed the headers sent by the image module. --- modules/image/image.module | 13 +++++--- modules/image/image.test | 67 ++++++++++++++++++++++++++------------ 2 files changed, 55 insertions(+), 25 deletions(-) diff --git a/modules/image/image.module b/modules/image/image.module index f56b97b54c8..a6803a667da 100644 --- a/modules/image/image.module +++ b/modules/image/image.module @@ -69,8 +69,8 @@ function image_file_download($filepath) { $headers = module_invoke_all('file_download', $original_path); if (!in_array(-1, $headers)) { return array( - 'Content-Type: ' . $info['mime_type'], - 'Content-Length: ' . $info['file_size'], + 'Content-Type' => $info['mime_type'], + 'Content-Length' => $info['file_size'], ); } } @@ -306,13 +306,17 @@ function image_style_generate() { // Don't start generating the image if it is already in progress. $cid = 'generate:' . $style_name . ':' . $path_md5; if (cache_get($cid, 'cache_image')) { + // Tell client to retry again in 3 seconds. Currently no browsers are known + // to support Retry-After. + drupal_set_header('503 Service Unavailable'); + drupal_set_header('Retry-After', 3); print t('Image generation in progress, please try again shortly.'); exit(); } // If the image has already been generated then send it. if ($image = image_load($destination)) { - file_transfer($image->source, array('Content-type: ' . $image->info['mime_type'], 'Content-length: ' . $image->info['file_size'])); + file_transfer($image->source, array('Content-Type' => $image->info['mime_type'], 'Content-Length' => $image->info['file_size'])); } // Set a cache entry designating this image as being in-process. @@ -322,11 +326,12 @@ function image_style_generate() { if (image_style_create_derivative($style, $source, $destination)) { $image = image_load($destination); cache_clear_all($cid, 'cache_image'); - file_transfer($image->source, array('Content-type: ' . $image->info['mime_type'], 'Content-length: ' . $image->info['file_size'])); + file_transfer($image->source, array('Content-Type' => $image->info['mime_type'], 'Content-Length' => $image->info['file_size'])); } else { cache_clear_all($cid, 'cache_image'); watchdog('image', 'Unable to generate the derived image located at %path.', $destination); + drupal_set_header('500 Internal Server Error'); print t('Error generating image.'); exit(); } diff --git a/modules/image/image.test b/modules/image/image.test index 0ad1de431be..e58389a1131 100644 --- a/modules/image/image.test +++ b/modules/image/image.test @@ -32,8 +32,8 @@ */ class ImageStylesPathAndUrlUnitTest extends DrupalWebTestCase { protected $style_name; - protected $image_with_generated; - protected $image_without_generated; + protected $image_info; + protected $image_filepath; public static function getInfo() { return array( @@ -47,28 +47,25 @@ class ImageStylesPathAndUrlUnitTest extends DrupalWebTestCase { parent::setUp(); $this->style_name = 'style_foo'; + image_style_save(array('name' => $this->style_name)); // Create the directories for the styles. - $status = file_check_directory($d = file_directory_path() .'/styles/' . $this->style_name, FILE_CREATE_DIRECTORY); + $status = file_check_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.' )); - // Make two copies of the file... + // Create a working copy of the file. $file = reset($this->drupalGetTestFiles('image')); - $this->image_without_generated = file_unmanaged_copy($file->filepath, NULL, FILE_EXISTS_RENAME); - $this->assertNotIdentical(FALSE, $this->image_without_generated, t('Created the without generated image file.')); - $this->image_with_generated = file_unmanaged_copy($file->filepath, NULL, FILE_EXISTS_RENAME); - $this->assertNotIdentical(FALSE, $this->image_with_generated, t('Created the with generated image file.')); - // and create a "generated" file for the one. - $status = file_unmanaged_copy($file->filepath, image_style_path($this->style_name, $this->image_with_generated), FILE_EXISTS_REPLACE); - $this->assertNotIdentical(FALSE, $status, t('Created a file where the generated image should be.')); + $this->image_info = image_get_info($file->filepath); + $this->image_filepath = file_unmanaged_copy($file->filepath, 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_without_generated); - $expected = file_directory_path() . '/styles/' . $this->style_name . '/' . basename($this->image_without_generated); + $actual = image_style_path($this->style_name, $this->image_filepath); + $expected = file_directory_path() . '/styles/' . $this->style_name . '/' . basename($this->image_filepath); $this->assertEqual($actual, $expected, t('Got the path for a file.')); } @@ -76,15 +73,43 @@ class ImageStylesPathAndUrlUnitTest extends DrupalWebTestCase { * Test image_style_url(). */ function testImageStyleUrl() { - // Test it with no generated file. - $actual = image_style_url($this->style_name, $this->image_without_generated); - $expected = url('image/generate/' . $this->style_name . '/' . $this->image_without_generated, array('absolute' => TRUE)); - $this->assertEqual($actual, $expected, t('Got the generate URL for a non-existent 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 = file_directory_path() . '/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->image_filepath, array('absolute' => TRUE)); + $this->drupalGet($expected_generate_url); + $this->assertResponse(403, t('Access to generate URL was denied.')); - // Now test it with a generated file. - $actual = image_style_url($this->style_name, $this->image_with_generated); - $expected = file_create_url(image_style_path($this->style_name, $this->image_with_generated)); - $this->assertEqual($actual, $expected, t('Got the download URL for an existing file.')); + // Check that a generate URL is returned. + $actual_generate_url = image_style_url($this->style_name, $this->image_filepath); + $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 cache_image). + $cid = 'generate:' . $this->style_name . ':' . md5($this->image_filepath); + cache_set($cid, $generated_path, 'cache_image'); + $this->drupalGet($expected_generate_url); + $this->assertResponse(503, t('Service Unavailable response received.')); + $this->assertTrue($this->drupalGetHeader('Retry-After'), t('Retry-After header received.')); + cache_clear_all($cid, 'cache_image'); + + // 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->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.')); + + // 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); + $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.')); } }