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);
Files: 
CommentFileSizeAuthor
#9 183435.patch782 bytesTR
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#7 183435.patch706 bytesTR
PASSED: [[SimpleTest]]: [MySQL] 29,406 pass(es).
[ View ]
#2 common.inc__8.patch450 bytesgregmac
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch common.inc__8.patch.
[ View ]

Comments

Status:Needs review» Active

Please attach a patch, see http://drupal.org/patch/create.

Status:Active» Needs review
StatusFileSize
new450 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch common.inc__8.patch.
[ View ]

Patch attached (same as code in first post..)

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.

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)

Status:Needs review» Needs work

Needs code commenting, as mentioned in #3.

Version:5.2» 6.x-dev

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

Version:6.x-dev» 8.x-dev
Status:Needs work» Needs review
Issue tags:+needs backport to D6, +needs backport to D7
StatusFileSize
new706 bytes
PASSED: [[SimpleTest]]: [MySQL] 29,406 pass(es).
[ View ]

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.

Status:Needs review» Fixed

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.

Version:8.x-dev» 6.x-dev
Assigned:Unassigned» TR
Status:Fixed» Needs review
Issue tags:-needs backport to D7
StatusFileSize
new782 bytes
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

Changing status/tags to reflect that this is still an issue with D6.

Attached is a port of the patch to Drupal 6.x

Status:Needs review» Patch (to be ported)

Correcting status.
This was committed to D8 and D7 in #8, just need to get into D6 now.

Status:Patch (to be ported)» Needs review

#9: 183435.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

The D6 patch in #9 works great, tested against a service that sends malformed responses (Canada Post SellOnline).

Status:Reviewed & tested by the community» Fixed

Thanks, committed to Drupal 6 too.

Status:Fixed» Closed (fixed)
Issue tags:-needs backport to D6

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