summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDries Buytaert2010-08-22 22:04:18 (GMT)
committerDries Buytaert2010-08-22 22:04:18 (GMT)
commit7c57f94315b7f476f37877970b57098fe8e56aab (patch)
tree60f68d4a60f668d533e021232e64923c4f947723
parent80befb6c7e27d32b3e91cfce09bfe8e71b2c6203 (diff)
- Patch #851878 by justinrandell, flobruit, ksenzee, aaron, litwol: serve image derivatives from the same url they are generated from.
-rw-r--r--modules/image/image.module143
-rw-r--r--modules/image/image.test60
2 files changed, 107 insertions, 96 deletions
diff --git a/modules/image/image.module b/modules/image/image.module
index 1b6d0b5..739713e 100644
--- a/modules/image/image.module
+++ b/modules/image/image.module
@@ -71,10 +71,24 @@ function image_help($path, $arg) {
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 @@ 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() {
@@ -609,32 +642,48 @@ function image_style_options($include_empty = TRUE) {
/**
* 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();
+ }
+ if (count($headers)) {
+ foreach ($headers as $name => $value) {
+ drupal_add_http_header($name, $value);
+ }
+ }
+ }
+ }
- // 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 +697,18 @@ function image_style_generate() {
// 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 +791,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 <img> 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 +798,18 @@ function image_style_flush($style) {
* @return
* The absolute URL where a style image can be downloaded, suitable for use
* in an <img> 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 +836,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;
}
/**
@@ -1068,7 +1099,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 a/modules/image/image.test b/modules/image/image.test
index ec00a72..3c0c9ca 100644
--- a/modules/image/image.test
+++ b/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,27 @@ 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.'));
+ if ($scheme == 'private') {
+ $this->assertEqual($this->drupalGetHeader('X-Image-Owned-By'), 'image_module_test', t('Expected custom header has been added.'));
+ }
}
}
@@ -684,7 +664,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);