The Drupal 7.20 security release introduced this change:

-  return file_create_url($uri);
+  $file_url = file_create_url($uri);
+  // Append the query string with the token.
+  return $file_url . (strpos($file_url, '?') !== FALSE ? '&' : '?') . drupal_http_build_query($token_query);

This means the query string is effectively not available for hook_file_url_alter() to be modified, i.e. it breaks file URL altering.


This breaks CDN module's Far Future expiration functionality.

CommentFileSizeAuthor
7.21_correct_file_create_url_usage.patch776 bytesWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, 7.21_correct_file_create_url_usage.patch, failed testing.

iamEAP’s picture

Version: 7.21 » 7.x-dev
Status: Needs work » Needs review
iamEAP’s picture

Status: Needs review » Needs work

The last submitted patch, 7.21_correct_file_create_url_usage.patch, failed testing.

jcisio’s picture

Status: Needs work » Closed (won't fix)

By passing file_create_url() after having added the token, the symbol ? or & is encoded with drupal_encode_path():

      // If this is not a properly formatted stream, then it is a shipped file.
      // Therefore, return the urlencoded URI with the base URL prepended.
      return $GLOBALS['base_url'] . '/' . drupal_encode_path($uri);

Unless we were to change this logic, this issue is won't fix for D7. The ability to remove the itok querystring comes then at the template level :(

I don't know about CDN FF functionality and how it breaks or could not be fixed.

pjcdawkins’s picture

Yes, this looks hard. Maybe you could do something super hackish like:

 // Add the query string before calling file_create_url(), so that it is available in
 // hook_file_url_alter().
 $query_string = drupal_http_build_query($token_query);
 $uri .= (strpos($uri, '?') !== FALSE ? '&' : '?') . $query_string;
 $uri = file_create_url($uri);
 // If file_create_url() has simply URL-encoded the token query, and if that was the
 // entire query string, restore it here.
 return str_replace(drupal_encode_path('?' . $query_string), '?' . $query_string, $uri);
jcisio’s picture

It is even not hackish enough. Think about user with Clean URLs off ;)

jcisio’s picture

Issue summary: View changes

Updated issue summary.