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);
Comment | File | Size | Author |
---|---|---|---|
#9 | 183435.patch | 782 bytes | TR |
#7 | 183435.patch | 706 bytes | TR |
#2 | common.inc__8.patch | 450 bytes | gregmac |
Comments
Comment #1
drummPlease attach a patch, see http://drupal.org/patch/create.
Comment #2
gregmac CreditAttribution: gregmac commentedPatch attached (same as code in first post..)
Comment #3
Dries CreditAttribution: Dries commentedIf 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.
Comment #4
marie70 CreditAttribution: marie70 commentedSo 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)
Comment #5
drummNeeds code commenting, as mentioned in #3.
Comment #6
anarchivist CreditAttribution: anarchivist commentedThis 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
Comment #7
TR CreditAttribution: TR commentedBug 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.
Comment #8
Dries CreditAttribution: Dries commentedCommitted 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.
Comment #9
TR CreditAttribution: TR commentedChanging status/tags to reflect that this is still an issue with D6.
Attached is a port of the patch to Drupal 6.x
Comment #10
TR CreditAttribution: TR commentedCorrecting status.
This was committed to D8 and D7 in #8, just need to get into D6 now.
Comment #11
TR CreditAttribution: TR commented#9: 183435.patch queued for re-testing.
Comment #12
iswilson CreditAttribution: iswilson commentedThe D6 patch in #9 works great, tested against a service that sends malformed responses (Canada Post SellOnline).
Comment #13
Gábor HojtsyThanks, committed to Drupal 6 too.