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

Comments

drewish’s picture

drewish commented

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

drewish’s picture

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
StatusFileSize
new 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 commented
StatusFileSize
new 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
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 commented
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new file_180126_0.patch973 bytes

Added the comment that Gabor requested.

scor’s picture

scor commented
StatusFileSize
new file_180126_3.patch788 bytes

Added and corrected the comment that Gabor requested ;)

drewish’s picture

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
commented
Status: Reviewed & tested by the community » Fixed

Thanks, committed.

scor’s picture

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

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

scor’s picture

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

cross posting...

Anonymous’s picture

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

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 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 commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new file_tempnam_180126.patch1.3 KB

mmoreno, give this a look/try.

CheckeredFlag’s picture

CheckeredFlag commented

Worked well on Drupal 5.5. Thanks!

CheckeredFlag’s picture

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
commented
Status: Needs review » Fixed

Committed to 5.x.

Anonymous’s picture

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

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