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

drumm’s picture

Status:Needs review» Active

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

gregmac’s picture

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..)

Dries’s picture

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.

marie70’s picture

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)

drumm’s picture

Status:Needs review» Needs work

Needs code commenting, as mentioned in #3.

anarchivist’s picture

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

TR’s picture

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.

Dries’s picture

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.

TR’s picture

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

TR’s picture

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.

TR’s picture

Status:Patch (to be ported)» Needs review

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

iswilson’s picture

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).

Gábor Hojtsy’s picture

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.