registry team mailing list archive

[Wim Leers <work@xxxxxxxxxxxx>]

Wim Leers has proposed merging lp:~wimleers/pressflow/hook_file_url_alter_bugfix into lp:pressflow.

Requested reviews:
  Wim Leers (wimleers)
  David Strauss (davidstrauss)
Related bugs:
  #597718 file.inc function file_create_url() not correct
  https://bugs.launchpad.net/bugs/597718


Includes the bugfixes for the bugs reported in https://bugs.launchpad.net/pressflow/+bug/597718. Only the file_create_url() function in includes/file.inc is affected.

Big thanks go to jcmarco, whose insight has uncovered flaws in my initial patch.

Note: the exact same code for file_create_url() is used in the CDN module's (http://drupal.org/project/cdn) core patch, as of version 2.0 beta 6 (http://drupal.org/node/933908). No more problems have been reported.
-- 
https://code.launchpad.net/~wimleers/pressflow/hook_file_url_alter_bugfix/+merge/39054
Your team Registry Administrators is subscribed to branch lp:pressflow.

=== modified file 'includes/file.inc'
--- includes/file.inc	2010-08-11 21:05:18 +0000
+++ includes/file.inc	2010-10-22 15:57:50 +0000
@@ -48,7 +48,7 @@
  *   A string containing the Drupal path (i.e. path relative to the Drupal
  *   root directory) of the file to generate the URL for.
  * @return
- *   A string containing a URL that can be used to download the file.
+ *   A string containing a URL that can be used to access the file.
  */
 function file_create_url($path) {
   // Clean up Windows paths.
@@ -56,21 +56,14 @@
 
   drupal_alter('file_url', $path);
 
-  // If any module has altered the path, then return the alteration.
-  if ($path != $old_path) {
+  // Return path if it was altered by any module, or if it already is a
+  // root-relative or a protocol-relative URL.
+  if ($path != $old_path || drupal_substr($path, 0, 1) == '/' || drupal_substr($path, 0, 2) == '//') {
     return $path;
   }
 
-  // Otherwise serve the file from Drupal's web server. This point will only
-  // be reached when either no custom_file_url_rewrite() function has been
-  // defined, or when that function returned FALSE, thereby indicating that it
-  // cannot (or doesn't wish to) rewrite the URL. This is typically because
-  // the file doesn't match some conditions to be served from a CDN or static
-  // file server, or because the file has not yet been synced to the CDN or
-  // static file server.
-
   // Shipped files.
-  if (strpos($path, file_directory_path() . '/') !== 0) {
+  if (preg_match("#^(/?)(misc|modules|sites|themes)/#", $path) && (strpos($path, file_directory_path() . '/') !== 0)) {
     return base_path() . $path;
   }
   // Created files.
@@ -83,7 +76,9 @@
         // rewritten to be served relatively to system/files (which is a menu
         // callback that streams the file) instead of relatively to the file
         // directory path.
-        $path = file_directory_strip($path);
+        if (strpos($path, file_directory_path() . '/') === 0) {
+          $path = trim(substr($path, strlen(file_directory_path())), '\\/');
+        }
         return url('system/files/' . $path, array('absolute' => TRUE));
     }
   }
@@ -1046,23 +1041,6 @@
 }
 
 /**
- * Remove a possible leading file directory path from the given path.
- *
- * @param $path
- *   Path to a file that may be in Drupal's files directory.
- * @return
- *   String with Drupal's files directory removed from it.
- */
-function file_directory_strip($path) {
-  // Strip file_directory_path from $path. We only include relative paths in
-  // URLs.
-  if (strpos($path, file_directory_path() . '/') === 0) {
-    $path = trim(substr($path, strlen(file_directory_path())), '\\/');
-  }
-  return $path;
-}
-
-/**
  * Determine the maximum file upload size by querying the PHP settings.
  *
  * @return