Index: modules/image/image.test =================================================================== RCS file: /cvs/drupal/drupal/modules/image/image.test,v retrieving revision 1.26 diff -u -r1.26 image.test --- modules/image/image.test 9 Aug 2010 19:55:57 -0000 1.26 +++ modules/image/image.test 22 Aug 2010 15:18:36 -0000 @@ -121,7 +121,7 @@ } function setUp() { - parent::setUp(); + parent::setUp('image_module_test'); $this->style_name = 'style_foo'; image_style_save(array('name' => $this->style_name)); @@ -131,12 +131,13 @@ * Test image_style_path(). */ function testImageStylePath() { - $actual = image_style_path($this->style_name, 'public://foo/bar.gif'); - $expected = 'public://styles/' . $this->style_name . '/foo/bar.gif'; + $scheme = 'public'; + $actual = image_style_path($this->style_name, "$scheme://foo/bar.gif"); + $expected = "$scheme://styles/" . $this->style_name . "/$scheme/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'; + $expected = "$scheme://styles/" . $this->style_name . "/$scheme/foo/bar.gif"; $this->assertEqual($actual, $expected, t('Got the path for a relative file path.')); } @@ -161,12 +162,10 @@ // 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'); - $d = 'temporary://'; - file_prepare_directory($d, FILE_CREATE_DIRECTORY); // Create the directories for the styles. - $d = $scheme . '://styles/' . $this->style_name; - $status = file_prepare_directory($d, FILE_CREATE_DIRECTORY); + $directory = $scheme . '://styles/' . $this->style_name; + $status = file_prepare_directory($directory, 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. @@ -174,46 +173,24 @@ $file = reset($files); $image_info = image_get_info($file->uri); $original_uri = file_unmanaged_copy($file->uri, $scheme . '://', FILE_EXISTS_RENAME); + // Let the image_module_test module know about this file, so it can claim + // ownership in hook_file_download(). + variable_set('image_module_test_file_download', $original_uri); $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_uri = $scheme . '://styles/' . $this->style_name . '/' . basename($original_uri); + // Get the URL of a file that has not been generated and try to create it. + $generated_uri = $scheme . '://styles/' . $this->style_name . '/' . $scheme . '/'. 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, $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 . ':' . drupal_hash_base64($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.')); - $this->assertTrue($this->drupalGetHeader('Retry-After'), t('Retry-After header received.')); - lock_release($lock_name); + $generate_url = image_style_url($this->style_name, $original_uri); // Fetch the URL that generates the file. - $this->drupalGet($expected_generate_url); - $this->assertTrue(file_exists($generated_uri), t('Generated file was created.')); + $this->drupalGet($generate_url); + $this->assertResponse(200, t('Image was generated at the URL.')); + $this->assertTrue(file_exists($generated_uri), t('Generated file does exist after we accessed it.')); $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_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_uri), t('URL returns expected file.')); - $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.')); } } @@ -684,7 +661,7 @@ // Ensure the derrivative image is generated so we do not have to deal with // image style callback paths. $this->drupalGet(image_style_url('thumbnail', $image_uri)); - $image_info['path'] = image_style_url('thumbnail', $image_uri); + $image_info['path'] = image_style_path('thumbnail', $image_uri); $image_info['getsize'] = FALSE; $default_output = theme('image', $image_info); $this->drupalGet('node/' . $nid); Index: modules/image/image.module =================================================================== RCS file: /cvs/drupal/drupal/modules/image/image.module,v retrieving revision 1.46 diff -u -r1.46 image.module --- modules/image/image.module 22 Aug 2010 13:52:58 -0000 1.46 +++ modules/image/image.module 22 Aug 2010 15:18:34 -0000 @@ -71,10 +71,24 @@ function image_menu() { $items = array(); - $items['image/generate/%image_style'] = array( + // Generate image derivatives of publicly available files. + // If clean URLs are disabled, image derivatives will always be served + // through the menu system. + // If clean URLs are enabled and the image derivative already exists, + // PHP will be bypassed. + $items[file_directory_path('public') . '/styles/%image_style'] = array( 'title' => 'Generate image style', - 'page callback' => 'image_style_generate', - 'page arguments' => array(2), + 'page callback' => 'image_style_deliver', + 'page arguments' => array(count(explode('/', file_directory_path())) + 1), + 'access callback' => TRUE, + 'type' => MENU_CALLBACK, + ); + // Generate and deliver image derivatives of private files. + // These image derivatives are always delivered through the menu system. + $items['system/files/styles/%image_style'] = array( + 'title' => 'Generate image style', + 'page callback' => 'image_style_deliver', + 'page arguments' => array(3), 'access callback' => TRUE, 'type' => MENU_CALLBACK, ); @@ -233,6 +247,25 @@ } /** + * Implements hook_form_FORM_ID_alter(). + */ +function image_form_system_file_system_settings_alter(&$form, &$form_state) { + $form['#submit'][] = 'image_system_file_system_settings_submit'; +} + +/** + * Submit handler for the file system settings form. + * + * Adds a menu rebuild after the public file path has been changed, so that the + * menu router item depending on that file path will be regenerated. + */ +function image_system_file_system_settings_submit($form, &$form_state) { + if ($form['file_public_path']['#default_value'] !== $form_state['values']['file_public_path']) { + variable_set('menu_rebuild_needed', TRUE); + } +} + +/** * Implements hook_flush_caches(). */ function image_flush_caches() { @@ -609,32 +642,43 @@ /** * Menu callback; Given a style and image path, generate a derivative. * - * This menu callback is always served after checking a token to prevent - * generation of unnecessary images. After generating an image transfer it to - * the requesting agent via file_transfer(). + * After generating an image, transfer it to the requesting agent. + * + * @param $style + * The image style */ -function image_style_generate() { - $args = func_get_args(); - $style = array_shift($args); - $style_name = $style['name']; - $scheme = array_shift($args); - $path = implode('/', $args); - - $path = $scheme . '://' . $path; - $path_hash = drupal_hash_base64($path); - $destination = image_style_path($style['name'], $path); - - // Check that it's a defined style and that access was granted by - // image_style_url(). - if (!$style || !cache_get('access:' . $style_name . ':' . $path_hash, 'cache_image')) { - drupal_access_denied(); +function image_style_deliver($style, $scheme) { + // Check that the style is defined and the scheme is valid. + if (!$style || !file_stream_wrapper_valid_scheme($scheme)) { drupal_exit(); } + + $args = func_get_args(); + array_shift($args); + array_shift($args); + $target = implode('/', $args); + + $image_uri = $scheme . '://' . $target; + $derivative_uri = image_style_path($style['name'], $image_uri); + + // If using the private scheme, let other modules provide headers and + // control access to the file. + if ($scheme == 'private') { + if (file_exists($derivative_uri)) { + file_download($scheme, file_uri_target($derivative_uri)); + } + else { + $headers = module_invoke_all('file_download', $image_uri); + if (in_array(-1, $headers) || empty($headers)) { + return drupal_access_denied(); + } + } + } - // Don't start generating the image if the derivate already exists or if + // Don't start generating the image if the derivative already exists or if // generation is in progress in another thread. - $lock_name = 'image_style_generate:' . $style_name . ':' . $path_hash; - if (!file_exists($destination)) { + $lock_name = 'image_style_deliver:' . $style['name'] . ':' . drupal_hash_base64($image_uri); + if (!file_exists($derivative_uri)) { $lock_acquired = lock_acquire($lock_name); if (!$lock_acquired) { // Tell client to retry again in 3 seconds. Currently no browsers are known @@ -648,18 +692,18 @@ // Try to generate the image, unless another thread just did it while we were // acquiring the lock. - $success = file_exists($destination) || image_style_create_derivative($style, $path, $destination); + $success = file_exists($derivative_uri) || image_style_create_derivative($style, $image_uri, $derivative_uri); if (!empty($lock_acquired)) { lock_release($lock_name); } if ($success) { - $image = image_load($destination); + $image = image_load($derivative_uri); file_transfer($image->source, array('Content-Type' => $image->info['mime_type'], 'Content-Length' => $image->info['file_size'])); } else { - watchdog('image', 'Unable to generate the derived image located at %path.', array('%path' => $destination)); + watchdog('image', 'Unable to generate the derived image located at %path.', array('%path' => $derivative_uri)); drupal_add_http_header('Status', '500 Internal Server Error'); print t('Error generating image.'); drupal_exit(); @@ -742,13 +786,6 @@ /** * Return the URL for an image derivative given a style and image path. * - * This function is the default image generation method. It returns a URL for - * an image that can be used in an tag. When the browser requests the - * image at image/generate/[style_name]/[scheme]/[path] the image is generated - * if it does not already exist and then served to the browser. This allows - * each image to have its own PHP instance (and memory limit) for generation of - * the new image. - * * @param $style_name * The name of the style to be used with this image. * @param $path @@ -756,29 +793,18 @@ * @return * The absolute URL where a style image can be downloaded, suitable for use * in an tag. Requesting the URL will cause the image to be created. - * @see image_style_generate() + * @see image_style_deliver() */ function image_style_url($style_name, $path) { - $destination = image_style_path($style_name, $path); - - // If the image already exists use that rather than regenerating it. - if (file_exists($destination)) { - return file_create_url($destination); - } - - // Disable page cache for this request. This prevents anonymous users from - // needlessly hitting the image generation URL when the image already exists. - drupal_page_is_cacheable(FALSE); - - // Set a cache entry to grant access to this style/image path. This will be - // checked by image_style_generate(). - cache_set('access:' . $style_name . ':' . drupal_hash_base64($path), 1, 'cache_image', REQUEST_TIME + 600); - $scheme = file_uri_scheme($path); - $target = file_uri_target($path); - - // Generate a callback path for the image. - $url = url('image/generate/' . $style_name . '/' . $scheme . '/' . $target, array('absolute' => TRUE)); + if ($scheme === 'private') { + $target = file_uri_target($path); + $url = url('system/files/styles/' . $style_name . '/' . $scheme . '/' . $target, array('absolute' => TRUE)); + } + else { + $destination = image_style_path($style_name, $path); + $url = url(file_directory_path($scheme) . '/' . file_uri_target($destination), array('absolute' => TRUE)); + } return $url; } @@ -805,7 +831,7 @@ $path = $uri; $scheme = variable_get('file_default_scheme', 'public'); } - return $scheme . '://styles/' . $style_name . '/' . $path; + return $scheme . '://styles/' . $style_name . '/' . $scheme . '/' . $path; } /** @@ -1068,7 +1094,7 @@ if (!file_exists($style_path)) { $style_path = image_style_url($style_name, $path); } - $variables['path'] = file_create_url($style_path); + $variables['path'] = $style_path; $variables['getsize'] = FALSE; return theme('image', $variables); } Index: modules/image/tests/image_module_test.module =================================================================== RCS file: modules/image/tests/image_module_test.module diff -N modules/image/tests/image_module_test.module --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ modules/image/tests/image_module_test.module 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,14 @@ + 'image_module_test'); + } + return -1; +} Index: modules/image/tests/image_module_test.info =================================================================== RCS file: modules/image/tests/image_module_test.info diff -N modules/image/tests/image_module_test.info --- /dev/null 1 Jan 1970 00:00:00 -0000 +++ modules/image/tests/image_module_test.info 1 Jan 1970 00:00:00 -0000 @@ -0,0 +1,8 @@ +; $Id$ +name = Image test +description = Provides hook implementations for testing Image module functionality. +package = Core +version = VERSION +core = 7.x +files[] = image_module_test.module +hidden = TRUE