I'm wanting to pull in some images from a remote URL which looks like they are controlled by some form of digital asset manager - and to make life difficult the images have no explicit file extension on them.

Examples:

http://bhs.scene7.com/is/image/BHS/1213193_a?$product294x250$
http://s7v1.scene7.com/is/image/Littlewoods/A911P_SP748_28_PU666?fmt=jpeg&qlt=90&wid=245&hei=410&color=255,255,255&size=245,410

Passing these remote image urls causes filefield_sources to throw back an error about no extension ("The remote URL must be a file and have an extension."). If no extension is found, can we not fall back to checking the HTTP headers before giving up? The above serve back with Content-Type:image/jpeg so the browser knows what to do with them.

I've not experimented yet, but maybe the adding of the extension could be done in remote.inc/_filefield_source_remote_filename() whilst its also getting the real filename from the HTTP headers?

Comments

budda’s picture

Looking at the code some more I see there is already some degree of support for extension-less URLs!

    // We prefer to use the Content-Disposition header, because we can then
    // use URLs like http://example.com/get_file/23 which would otherwise be
    // rejected because the URL basename lacks an extension.

Looking at the headers coming back from the scene7.com image urls above there's no "real" filename specified in them so the module falls back to using the url, which also has no file extension.

Content-Type:image/jpeg
Date:Sun, 17 Jul 2011 15:54:59 GMT
ETag:"b2593c366ba42b53c36126af71bdc1fb"
Expires:Sun, 17 Jul 2011 16:54:53 GMT
budda’s picture

Version: 7.x-1.4 » 7.x-1.x-dev
Status: Active » Needs review
StatusFileSize
new1.92 KB

Attached is a patch to resolve the missing file extensions using the MIME header.

It just occurred to me that maybe I should have used the API function file_get_mimetype() ? It would cause a further file access though I believe.

clemens.tolboom’s picture

Status: Needs review » Needs work
+++ b/sources/remote.inc
@@ -174,11 +174,18 @@ function filefield_source_remote_value($element, &$item) {
+    ¶

whitespace

+++ b/sources/remote.inc
@@ -174,11 +174,18 @@ function filefield_source_remote_value($element, &$item) {
-      form_error($element, t('The remote URL must be a file and have an extension.'));
+      form_error($element, t('The remote URL must be a file and have an extension.' . $filename));

No space before $filename.

Better use array(%file => $filename) within t function.

budda’s picture

Status: Needs work » Needs review
StatusFileSize
new2.72 KB

Removed the $filename debug as it was not needed.

mnemyx’s picture

Could this patch also be available for the Drupal 6 version?

clemens.tolboom’s picture

Status: Needs review » Needs work
+++ b/sources/remote.inc
@@ -139,6 +139,10 @@ function filefield_source_remote_value($element, &$item) {
+    ¶

whitespace (multiple times)

+++ b/sources/remote.inc
@@ -139,6 +139,10 @@ function filefield_source_remote_value($element, &$item) {
+    $useragent="Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.1) Gecko/20061204 Firefox/2.0.0.1";
+    curl_setopt($ch, CURLOPT_USERAGENT, $useragent);

Why are these needed. You should document the reasons why.

+++ b/sources/remote.inc
@@ -152,7 +156,7 @@ function filefield_source_remote_value($element, &$item) {
-          form_error($element, t('The remote file could not be transfered due to an HTTP error (@code).', array('@code' => $info['http_code'])));
+          form_error($element, t('The remote file could not be transfered due to an HTTP error (@code). ' . $url, array('@code' => $info['http_code'])));

You should add the url to the array as an placeholder for the t-function.

+++ b/sources/remote.inc
@@ -235,6 +246,8 @@ function _filefield_source_remote_parse_header(&$ch, $header) {
+  }elseif(preg_match('/Content-Type:.*?image\/(\w+)/', $header, $matches)) {

elseif should start on a new line.
space before the opening (

clemens.tolboom’s picture

budda’s picture

No I think the two issues are different.

Flickr seems to be about catering for dodgy http headers with newslines.
This issue is about files with no extension falling back to using mime headers to add an extension on to the filename before saving.

budda’s picture

Assigned: Unassigned » budda
Status: Needs work » Needs review
StatusFileSize
new1.61 KB

Re-rolled patch on a clean install and no debug / cruft from experimenting.

quicksketch’s picture

Status: Needs review » Needs work

This patch should probably use file_mimetype_mapping() to lookup MIME-type/extension pairs and work with any extension, rather than being specific to image/* (which won't always line up with the extension exactly anyway).

quicksketch’s picture

Title: "The remote URL must be a file and have an extension." - why? » Handle remote files without extensions based on Content-Type header
Category: bug » feature
Status: Needs work » Fixed
StatusFileSize
new2.22 KB

I've committed this patch based on budda's work that leverages Drupal's built-in MIME-type to extension mappings. Unfortunately because this mapping is not available to Drupal 6, we don't have the option to add this patch there also (though we could use budda's original approach perhaps). For now I've committed this to the 7.x-1.x branch only; it's probably time to leave D6 alone except for bug fixes anyway.

Status: Fixed » Closed (fixed)

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