diff --git modules/image/image.module modules/image/image.module index 9bacb6f..75df967 100644 --- modules/image/image.module +++ modules/image/image.module @@ -71,10 +71,19 @@ function image_help($path, $arg) { function image_menu() { $items = array(); - $items['image/generate/%image_style'] = array( + // Generate image derivatives of publicly available files. + $items[file_directory_path('public') . '/styles/%image_style'] = array( 'title' => 'Generate image style', 'page callback' => 'image_style_generate', - 'page arguments' => array(2), + 'page arguments' => array(count(explode('/', file_directory_path())) + 1), + 'access callback' => TRUE, + 'type' => MENU_CALLBACK, + ); + // Generate and deliver image derivatives of private files. + $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 +242,25 @@ function image_permission() { } /** + * 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() { @@ -615,11 +643,44 @@ function image_style_options($include_empty = TRUE) { } /** + * Menu callback; Delivers a derivative of a private image file. + * + * @see file_download() + */ +function image_style_deliver() { + // Keep a copy of the original function arguments to use when generating the + // image derivative. + $original_args = $args = func_get_args(); + $style = array_shift($args); + // The rest of the arguments are the file path from $_GET['q']. + $scheme = array_shift($args); + $target = implode('/', $args); + + $image_uri = $scheme . '://' . $target; + $derivative_uri = image_style_path($style['name'], $image_uri); + if (file_stream_wrapper_valid_scheme($scheme)) { + if (file_exists($derivative_uri)) { + file_download($scheme, file_uri_target($derivative_uri)); + } + else { + // Let other modules provide headers and control access to the file. + $headers = module_invoke_all('file_download', $image_uri); + if (in_array(-1, $headers)) { + return drupal_access_denied(); + } + if (count($headers)) { + call_user_func_array('image_style_generate', $original_args); + } + } + } + return drupal_not_found(); +} + +/** * 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 via + * file_transfer(). */ function image_style_generate() { $args = func_get_args(); @@ -629,19 +690,16 @@ function image_style_generate() { $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(); + // Check that it's a defined style. + if (!$style) { drupal_exit(); } - // 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; + $lock_name = 'image_style_generate:' . $style_name . ':' . drupal_hash_base64($path); if (!file_exists($destination)) { $lock_acquired = lock_acquire($lock_name); if (!$lock_acquired) { @@ -750,13 +808,6 @@ function image_style_flush($style) { /** * 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 @@ -767,26 +818,15 @@ function image_style_flush($style) { * @see image_style_generate() */ 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; } @@ -813,7 +853,7 @@ function image_style_path($style_name, $uri) { $path = $uri; $scheme = variable_get('file_default_scheme', 'public'); } - return $scheme . '://styles/' . $style_name . '/' . $path; + return $scheme . '://styles/' . $style_name . '/' . $scheme . '/' . $path; } /** @@ -1076,7 +1116,7 @@ function theme_image_style($variables) { 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); } diff --git modules/image/image.test modules/image/image.test index 5ca8be6..9b336eb 100644 --- modules/image/image.test +++ modules/image/image.test @@ -121,7 +121,7 @@ class ImageStylesPathAndUrlUnitTest extends DrupalWebTestCase { } 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 @@ class ImageStylesPathAndUrlUnitTest extends DrupalWebTestCase { * 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 @@ class ImageStylesPathAndUrlUnitTest extends DrupalWebTestCase { // 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 @@ class ImageStylesPathAndUrlUnitTest extends DrupalWebTestCase { $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 @@ class ImageFieldDisplayTestCase extends ImageFieldTestCase { // 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); diff --git modules/image/tests/image_module_test.info modules/image/tests/image_module_test.info new file mode 100644 index 0000000..144a904 --- /dev/null +++ modules/image/tests/image_module_test.info @@ -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 diff --git modules/image/tests/image_module_test.module modules/image/tests/image_module_test.module new file mode 100644 index 0000000..5aa9dad --- /dev/null +++ modules/image/tests/image_module_test.module @@ -0,0 +1,14 @@ + 'image_module_test'); + } + return -1; +}