The File API in 4.4.0 won't work with PHP < 4.3.3. See the PHP manual: the fgets function used in file_transfer is not binary safe. This leads to problems with things like Avatars.

See the attached patch for a sure (but slow) fix.

A faster fix needs to be written, or we need to set PHP 4.3.3 or newer as a Drupal requirement.

CommentFileSizeAuthor
#7 common.inc_0.patch743 bytesdrumm
#3 common.patch547 byteskilles@www.drop.org
#1 drupal_18505 byteskilles@www.drop.org
file.patch246 bytesFrodo Looijaard
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

killes@www.drop.org’s picture

killes@www.drop.org CreditAttribution: killes@www.drop.org commented
Assigned: Unassigned » killes@www.drop.org
FileSize
drupal_18505 bytes

After some discussion with Kjartan on #drupal I tested the proposed solution. I found it to be working fine on PHP-4.2.1.

Dries’s picture

Dries CreditAttribution: Dries commented

We use fgets($fd, 1024) in includes/common.inc too.

killes@www.drop.org’s picture

killes@www.drop.org CreditAttribution: killes@www.drop.org commented
FileSize
common.patch547 bytes

here is the patch for common.inc

Dries’s picture

Dries CreditAttribution: Dries commented

Committed to both HEAD and DRUPAL-4-4. Thanks.

killes@www.drop.org’s picture

killes@www.drop.org CreditAttribution: killes@www.drop.org commented

Please revert that second patch. According to drumm it causes drupal_http_header to break and treat everything as header.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm commented
Priority: Normal » Critical

The patch to common.inc breaks the processing which happens later in the drupal_http_request function. That processing depends on the $response array to be broken by newlines. Switching functions removed that property of the array. This causes everyhting to be dumpted into the header array because it never encounters a blank array element representing a blank line. This makes me question what might happen with an unpatched common.inc getting a file with http headers longer than 1024 characters.

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm commented
FileSize
common.inc_0.patch743 bytes

Here is a patch to fix it while using the new function. Its better than the old behavior because it handles header lines longer than 1024 characters.

Its equally bad as the old (prepatch) behavior if we look at RFC 822, section 3.1.1 (referenced by RFC 2616 (http 1.1), section 4.2). To sum up that section, http headers can be multiple lines and we don't handle that case now.

Dries’s picture

Dries CreditAttribution: Dries commented

Committed to HEAD and DRUPAL-4-4. Thanks drumm.

Anonymous’s picture

(not verified) CreditAttribution: commented