Allow provider result of 302

Markus Sandy - August 9, 2007 - 00:27
Project:Embedded Media Field
Version:6.x-1.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed
Description

emfield_request_xml checks for a HTTP return result of 200. I have written a provider for the Internet Archive using their API, but they return 302 during redirection. Every thing works fine if I patch emfield_request_xml at line 447 of the current release (v 1.2 2007/07/26 14:20:34) to allow for a 302 result.

Before:

  if ($result->code != 200) {

After:

  if (($result->code != 200)  && ($result->code != 302)) {

Please let me know if there are any issues with doing this.

I'm new at patches, but I'll try and figure that out and add it here.

Here is a page showing the provider in action:

http://ladrupal.org/blog/markus_sandy/cck_embedded_media_field_internet_...

or

http://tinyurl.com/2y3tap

#1

winston - May 3, 2009 - 05:02

Can this perhaps be closed out?

Since emfield_request_xml calls drupal_http_request (see http://api.drupal.org/api/function/drupal_http_request/5), and drupal_http_request handles redirects (including 302), testing for 302 here shouldn't be necessary.

And if it is not dealing with 302 perhaps this is a core issue.

#2

Alex UA - May 9, 2009 - 19:54
Status:active» closed

I belleve Winston is correct. I'm closing this out. Thanks!

#3

winston - July 4, 2009 - 04:44
Version:5.x-1.x-dev» 6.x-1.x-dev
Category:feature request» bug report
Status:closed» needs review

Actually winston is NOT correct! I know it's a shocker ;)

Markus Sandy is in fact correct. It appears that emfield_request_xml is breaking in the way that is uses the results from the core "drupal_http_request" function. Apologies to markus sandy for not picking up on that.

The trick here is that the drupal_http_request function DOES properly handle 302 by recursively attempting again on the redirected location (up to $retry which defaults to 3 times). HOWEVER, after it comes back from that it does not change the code to 200. Here is an example of what the drupal_http_request returns (in print_r) format in a case where the file it is hitting returns a 302, but on the redirect successfully returns the ultimate target file...

Note: The original request URL is "http://www.archive.org/download/Magellan/Magellan_files.xml"

stdClass Object
(
    [request] => GET /1/items/Magellan/Magellan_files.xml HTTP/1.0
Host: ia310807.us.archive.org
User-Agent: Drupal (+http://drupal.org/)
Content-Length: 0
    [data] => <?xml version="1.0" encoding="UTF-8"?>...rest of xml file contents successfully retrieved...
    [headers] => Array
        (
            [Connection] => close
            [Content-Type] => application/xml
            [Accept-Ranges] => bytes
            [ETag] => "-1010543525"
            [Last-Modified] => Mon, 10 Nov 2008 02:17:35 GMT
            [Content-Length] => 3167
            [Date] => Sat, 04 Jul 2009 04:28:59 GMT
            [Server] => lighttpd/1.4.18
        )
    [code] => 302
    [redirect_code] => 200
    [redirect_url] => http://ia310807.us.archive.org/1/items/Magellan/Magellan_files.xml
)
stdClass Object
(
    [request] => GET /1/items/Magellan/Magellan_meta.xml HTTP/1.0
Host: ia310807.us.archive.org
User-Agent: Drupal (+http://drupal.org/)
Content-Length: 0

Note that it successfully returned the xml file and indicated that via [redirect_code] => 200, but still indicated [code] => 302 since that was the initial report from the file. This makes total sense on the part of the core drupal_http_request as the calling application may very well want to know that a redirect occurred so that the ultimate target can be stored if needed ([redirect_url] shows where it actually ended up getting the file from after the redirect).

So where does that leave us. Well markus sandy's report of error is certainly correct. Emfield is not using drupal_http_request correctly (unless the intent is to fail even when a standard 302 redirect would work). However I disagree with his proposed solution. The source code for drupal_http_request contains the correct answer for us and it is quite simple! If drupal_http_request fails even after retries/redirects then it returns $result->error = $text;

So we simply need to test for a non-empty $result->error! Yeah.

Anyway here is the one-line patch to fix this problem in emfield.module. I did a quick test which appears to confirm that this is the correct way to go.

markus sandy, again my apologies.

Note that I modified this to the latest version (best practice I believe?) Presumably a similar patch against the 5.x branch would also work.

AttachmentSize
emfield-165731.patch 588 bytes

#4

Rob Loach - July 24, 2009 - 16:26
Status:needs review» needs work

Shouldn't that be:

<?php
if (!empty($result->error)) {
?>

;-)

#5

winston - July 26, 2009 - 22:53
Status:needs work» needs review

Thank you Rob. Here is it again with that correction.

AttachmentSize
emfield-165731.patch 597 bytes

#6

aaron - August 4, 2009 - 19:12
Status:needs review» fixed

got it, thanks!

#7

System Message - August 18, 2009 - 19:20
Status:fixed» closed

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

 
 

Drupal is a registered trademark of Dries Buytaert.