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

#1
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
I belleve Winston is correct. I'm closing this out. Thanks!
#3
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.
#4
Shouldn't that be:
<?phpif (!empty($result->error)) {
?>
;-)
#5
Thank you Rob. Here is it again with that correction.
#6
got it, thanks!
#7
Automatically closed -- issue fixed for 2 weeks with no activity.