The file.inc in Drupal core utilizes the PHP tempnam() function in a couple of places to obtain a temporary filename.

With the format of tempnam($dir, $prefix), it should create a temporary file in directory $dir. However, in Windows, this MUST be an absolute path. If a relative path is used, then it defaults to the system temp (i.e. $_ENV['TMP']).
(See http://www.php.net/manual/en/function.tempnam.php#67260)

This confuses file_check_upload() in file.inc since it expects the uploaded file to be in this location.

This has manifested itself in bizarre ways, such as in uploads using the image module.
(See http://drupal.org/node/178013)

The solution is to force an absolute path when calling tempnam():

tempnam(realpath(file_directory_temp()), 'tmp_');

The attached patch does this.

CommentFileSizeAuthor
#14 file_tempnam_180126.patch1.3 KBdrewish
#6 file_180126_3.patch788 bytesscor
#5 file_180126_0.patch973 bytesdrewish
#3 file_180126_2.patch674 bytesscor
#2 file_180126.patch675 bytesdrewish
file.inc__0.patch833 bytesCheckeredFlag
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drewish’s picture

drewish CreditAttribution: drewish commented

subscribing. i'll try to have a look at this later on.

drewish’s picture

drewish CreditAttribution: drewish commented
Title: Buggy use of tempnam() on Windows causes failed uploads » Buggy use of tempnam() on Windows
Version: 5.2 » 6.x-dev
Priority: Normal » Critical
Status: Active » Needs review
FileSize
file_180126.patch675 bytes

Short version of the bug report is that we're writing to the wrong temp directory.

Bumping this to D6 since it affects file_save_data() there. Once we get it committed we can back port to D5.

scor’s picture

scor CreditAttribution: scor commented
FileSize
file_180126_2.patch674 bytes

rerolled the patch from drupal root.
tested with css and js aggregator.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Needs work

This is an obscure requirement from Windows versions, so document it above the line with some comment:

// On Windows, temnap() requires an absolute path, so we use realpath().

Or something along these lines.

drewish’s picture

drewish CreditAttribution: drewish commented
Status: Needs work » Reviewed & tested by the community
FileSize
file_180126_0.patch973 bytes

Added the comment that Gabor requested.

scor’s picture

scor CreditAttribution: scor commented
FileSize
file_180126_3.patch788 bytes

Added and corrected the comment that Gabor requested ;)

drewish’s picture

drewish CreditAttribution: drewish commented

scor, did we cross-post or did you just not like my more info link?

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Reviewed & tested by the community » Fixed

Thanks, committed.

scor’s picture

scor CreditAttribution: scor commented
Status: Fixed » Reviewed & tested by the community

Both :)
the right spelling of the function is tempnam() not temnap()...

scor’s picture

scor CreditAttribution: scor commented
Status: Reviewed & tested by the community » Fixed

cross posting...

Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)
CheckeredFlag’s picture

CheckeredFlag CreditAttribution: CheckeredFlag commented

drewish, just following up. Any word on getting this fix back ported to drupal 5 as per your comment #2?

drewish’s picture

drewish CreditAttribution: drewish commented
Version: 6.x-dev » 5.x-dev
Status: Closed (fixed) » Patch (to be ported)

yeah we need to flag it for back port... i'll try to check on the patch later today.

drewish’s picture

drewish CreditAttribution: drewish commented
Status: Patch (to be ported) » Needs review
FileSize
file_tempnam_180126.patch1.3 KB

mmoreno, give this a look/try.

CheckeredFlag’s picture

CheckeredFlag CreditAttribution: CheckeredFlag commented

Worked well on Drupal 5.5. Thanks!

CheckeredFlag’s picture

CheckeredFlag CreditAttribution: CheckeredFlag commented

drewish, just following up again after testing your patch as I'll be out after today until after the new year....

drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm commented
Status: Needs review » Fixed

Committed to 5.x.

Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)

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