Posted by gregmac on October 14, 2007 at 8:38pm
8 followers
| Project: | Drupal core |
| Version: | 6.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | TR |
| Status: | closed (fixed) |
| Issue tags: | needs backport to D6 |
Issue Summary
drupal_http_request() only checks for CRLF as a header/body seperator. Although it parses headers that have LF-only, it actually uses the sequence CRLFCRLF to split the headers and the response.
Although this strictly compiles with section 2.2 of RFC 1945 (http://www.faqs.org/rfcs/rfc1945.html) it does not implement the recommendation in Appendix B that states applications should be tolerant and allow for LF-only responses.
As a side note, I am having this issue trying to work with Canada Post's SellOnline HTTP service. I have notified them of the issue but still feel this is easily fixed in Drupal, and so here is a patch:
--- common.inc.orig 2007-10-14 15:58:22.000000000 -0400
+++ common.inc 2007-10-14 15:59:28.000000000 -0400
@@ -472,7 +472,7 @@
fclose($fp);
// Parse response.
- list($split, $result->data) = explode("\r\n\r\n", $response, 2);
+ list($split, $result->data) = preg_split("/\r\n\r\n|\n\n|\r\r/", $response, 2);
$split = preg_split("/\r\n|\n|\r/", $split);
list($protocol, $code, $text) = explode(' ', trim(array_shift($split)), 3);
Comments
#1
Please attach a patch, see http://drupal.org/patch/create.
#2
Patch attached (same as code in first post..)
#3
If we want to commit this to CVS HEAD for inclusion in Drupal 6, we'll want to document this in the code. It's non-trivial and without a short explanation, we won't be able to understand what that code is about. I suggest we include a small paragraph of text that summarizes the LF handling. Thanks gregmac.
#4
So to use Canada Post sellonline shipping api do I need to apply this patch? I'm using E-commerce 5.x 3.4 from Oct 17
Does it then work?
Any help is greatly appreciated (this is crazy)
#5
Needs code commenting, as mentioned in #3.
#6
This bug still exists as of Drupal 6.10. See, for example, the results of using drupal_http_request() to retrieve http://catnyp.nypl.org/record=b1000001
#7
Bug still exists in D7 and D8. Here is the patch re-rolled against D8, with some comments added. I'd really like to get this finalized and committed. If there are objections to making this change I'd like to hear them so I can address those concerns.
I'd like to note that all common tools that I've ever used, e.g. wget, curl, lynx, etc. properly handle the case where a non-compliant web server uses \n\n or \r\r instead of \r\n\r\n to separate the header and the body of the response. drupal_http_request() should too.
#8
Committed to 7.x and 8.x. Thanks for the extra code comments. Wish we could just use cURL but let's have that discussion elsewhere.
#9
Changing status/tags to reflect that this is still an issue with D6.
Attached is a port of the patch to Drupal 6.x
#10
Correcting status.
This was committed to D8 and D7 in #8, just need to get into D6 now.
#11
#9: 183435.patch queued for re-testing.
#12
The D6 patch in #9 works great, tested against a service that sends malformed responses (Canada Post SellOnline).
#13
Thanks, committed to Drupal 6 too.
#14
Automatically closed -- issue fixed for 2 weeks with no activity.