We noticed that we were having a massive performance hit when our external images used an uppercase file extension (.JPG, .pNg ...) or used the .jpeg extension. As it turns out, these images were failing an extension test in imagecache_external_generate_path() function and were causing extra hits on the imagecache_external_fetch() function rather than serving the cached image.

<?php
if ($extension = pathinfo($url_parameters['path'], PATHINFO_EXTENSION)) {
  if (in_array($extension, array('jpg', 'png', 'gif'))) {
    $filename .= '.' . $extension;
  }
}
?>

The solution was to modify this extension test to permit these variants on the file extension:

<?php
if ($extension = strtolower(pathinfo($url_parameters['path'], PATHINFO_EXTENSION))) {
  if (in_array($extension, array('jpg', 'jpeg', 'png', 'gif'))) {
    $filename .= '.' . $extension;
  }
}
?>

Patch attached.

CommentFileSizeAuthor
#1 imagecache_external-uppercase-file-extensions-2502379.patch670 bytesPaul_Gregory
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Paul_Gregory’s picture

Paul_Gregory CreditAttribution: Paul_Gregory commented
Title: Uppercase of .jpeg file extensions causing big performance issues » Uppercase file extensions or .jpeg causing big performance issues
Issue summary: View changes
FileSize
imagecache_external-uppercase-file-extensions-2502379.patch670 bytes
swentel’s picture

swentel CreditAttribution: swentel commented

We should also default to an extension as well IMO.
Sometimes external uri's don't have the extension and you basically will download the images every single request.

   if ($extension = pathinfo($url_parameters['path'], PATHINFO_EXTENSION)) {
-    if (in_array($extension, array('jpg', 'png', 'gif'))) {
+    if (in_array($extension, array('jpg', 'png', 'gif', 'jpeg'))) {
       $filename .= '.' . $extension;
     }
   }
+  else {
+    $filename .= '.jpg';
+  }
Paul_Gregory’s picture

Paul_Gregory CreditAttribution: Paul_Gregory commented

@swentel That sounds fair but don't forget the strtolower() I put in my patch in the first place.

-  if ($extension = pathinfo($url_parameters['path'], PATHINFO_EXTENSION)) {
-    if (in_array($extension, array('jpg', 'png', 'gif'))) {
+  if ($extension = strtolower(pathinfo($url_parameters['path'], PATHINFO_EXTENSION))) {
+    if (in_array($extension, array('jpg', 'png', 'gif', 'jpeg'))) {
       $filename .= '.' . $extension;
     }
   }
+  else {
+    $filename .= '.jpg';
+  }
swentel’s picture

swentel CreditAttribution: swentel commented

Yeah sorry about that, that was actually code directly from a patch that I'm running local :)

  • BarisW committed ec0328d on 7.x-2.x authored by Paul_Gregory 
    Issue #2502379 by Paul_Gregory, swentel: Uppercase file extensions or ....
BarisW’s picture

BarisW CreditAttribution: BarisW as a volunteer commented
Status: Active » Fixed

Looks good to me. Thanks, both!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.