File uploads silently failing on Windows when magic_quotes_gpc = On: fix_gpc_magic() strips legitimate backslashes from $_FILES

Okay, so this applies to all Windows installations, but with Apache you most likely have .htaccess turning off magic_quotes_gpc for you. WIth IIS it is a much bigger problem. It took me hours to finally track this down to fix_gpc_magic().

Bdragon confirmed my suspicions and pointed out a comment at http://us2.php.net/manual/en/features.file-upload.php#42280

So this patch checks if we're on Windows, and will stripslashes on the $_FILES array while avoiding tmp_name. There are quite a few posts on drupal.org with failed uploads for Windows with errors like this:

warning: copy(c:\inetpub\wwwroot\files) [function.copy]: failed to open stream: Permission denied in c:\inetpub\wwwroot\includes\file.inc on line 374.

Some related forum posts:
http://drupal.org/node/81613
http://drupal.org/node/80986
http://drupal.org/node/82360
http://drupal.org/node/63536

I'll roll one for HEAD after some reviews.

I can confirm that this patch works for me. I had problems uploading images and files with 4.7 on windows 2003 server IIS 6. This did the trick. Thanks!

but with Apache you most likely have .htaccess turning off magic_quotes_gpc for you

My server is Apache 1.3.37 and I'm experiencing this problem with uploading files. I can't upload anything to the files directory without getting a fatal error message on the server. The files/images shows 775, but when I click the images to change it to 777, it shows as 777. If I try to save the setting, it returns fatal error.

I tried to change the magic_quotes_gpc in the site/default/settings.php and it didn't change the .htaccess file. I tried to change the .htaccess file, and it returned fatal error.

I looked through all the directory folders, and I can't find the php.ini file - where can I find it? Or will the above patch fix my problem on Apache?

Please don't change the subject.

php.ini is usually in /etc on Linux and C:\PHP or C:\WINDOWS on Windows. This would fix your problem if magic_quotes_gpc = On.

I tested the latest release as well as 5.x. The upload module just didn't work.

I had to change the line in php.ini :

magic_quotes_gpc = On

to

magic_quotes_gpc = Off

Otherwise, the upload module WILL NOT WORK.

That modification in the php.ini file might create other problems...

I did NOT install the patch.

I think that should be in the documentation somewhere... don't you?

It seems that the patch given here has not been reviewed ?

So it's not included in 4.7, neither in 5.x

That's a real shame considering many people will have a testing server installed on Windows.

My testing server runs on Windows XP SP 2.

I have Apache 2.0.59 and PHP 5.1.4.

Moving to 5-dev to increase the review chance.

"I did NOT install the patch."

@Chill35: Can you turn magic_quotes_gpc = On and actually test the patch out. This patch is designed to fix the totally borked uploads when magic_quotes_gpc = On on Windows. You testing the patch and reviewing it will help get this in to Drupal.

Thanks!

I tested the patch on a Windows XP SP 2 Apache 2.0.59 php 5.1.4 server.

Before the patch, uploading files didn't work.
After the patch, uploading works.

I modified the php.ini file to the way it was originally, i.e. magic_quotes_gpc = On

I tested both with 4.7.4 and 5.x. Both had the problem.

Do I change the status to : 'patch (ready to be committed)' ? I am a newbie, I don't know how these things work.

Is it possible to include the patch in the current release as well ? Right now it is set to Version 5.x-dev.

I am afraid to make a mistake.

Can you turn magic_quotes_gpc = On and actually test the patch out.

Sorry I had not seen that.

Yes, I put the magic_quotes_gpc back to On in php.ini, I restarted my server, tested on both 4.7 and 5 to see if the problem was back. The problem was back. Then I installed the patch on both 4.7 and 5, tested both 4.7 and 7 and BINGO, it works very well now.

Thanks!

Do I change the status to : 'patch (ready to be committed)' ?

In http://drupal.org/contribute/testing

It says :

If there's a patch, test it. See below for further details.

Testing patches

// TODO

LOL... so I don't know how to review a patch officially. Sorry.

I'll have to go.

Hehe, that's pretty funny. Your review was excellent, that's exactly what a review is and helps the big boys see that this actually fixes something that was broken. I'll get somebody to mark this RTBC. Thanks for the review!

Marking this critical. Although this only applies to Windows users with magic_quotes_gpc On, this totally borks file uploads and doesn't even provide a descriptive message. This patch fixes that behavior.

dopry let me know that the second key of array_walk() is key, so I've simplified this a bit. Sure did seem like there should have been a PHP function for this. Please retest.

I retested. Here is what I did :

For both installations, 4.7.4 and 5.0, I overwrote the includes/common.inc file with its original file. I tested to see if I could experience the problem again. And I did. So that was good.

Then I made sure to use the right patch, the second one :fix_gpc_win_0.patch (and not fix_gpc_win.patch) and patched both includes/common.inc files, in my 4.7.4 and 5.0 installations. With that patch.

Upload is fixed with that new patch (fix_gpc_win_0.patch) in both 4.7 and 5.

Thank you much!

I noticed something interesting, RobRoy, that probably is not a bug. More like a feature LOL!

When I upload files that contain in their names funny characters such as an apostrophe, the file IS uploaded successfully, thank God, but the file uploaded has a new name that has, from its original name, the part up to, and including, the apostrophe, amputated.

Actual (real) example : Legros'-team.jpg

in 4.7 > the file uploaded has that name > -team.jpg

in 5 > the same file has the name > -team.jpg

Now that's not only the name as it is shown in the post or story (as attachment), it is also the actual name of the file uploaded (in /files/...).

There only seems to be a "problem" with the apostrophe.
Whitespace and accentuated characters are ok.

Other examples :

Original names :

le 3.jpg
l'eau dans l'bain.jpg
l'eau.jpg
beau trésor.jpg

New names (after upload) :

le 3.jpg (OK)
bain.jpg (truncated)
eau.jpg (truncated)
beau trésor.jpg (OK)

All the word up to and including the last apostrophe is truncated.

I've noticed that behavior both with and without this patch and think it may be how PHP deals with funny names like that. Thanks for the new review. Someone please mark this RTBC. :)

:) done

This smells like a bogus issue. If this would be a real issue, upload would have never worked, and almost surely someone would have complained for a PHP bug, too because we are not alone to use stripslashes to offset magic_quotes_gpc. bugs.php.net search for magic_quotes_gpc upload on Windows comes up almost empty handed.

At least two issues linked are not definitely about GPC. http://drupal.org/node/63536 says "he selected file C:\PHP\uploadtemp\tmp3.tmp could not be copied." so the file is there and http://drupal.org/node/81613#comment-150195 found that a mere change to his temporary directory is enough.

Also the patch is a hack.

Tested on Windows 2003 Server, Apache 2.0
PHP Version 4.3.10

Drupal 5-head (updated on 13nov06)

I turned magic quotes ON. Restarted Apache and was able to upload files and images.

forgot something. I did not apply any patch or change any files for this.

Note the directives in .htaccess:

Setting magic_quotes_gpc to 1, will cause upload to fail on PHP 5.1.4.

This smells like a bogus issue. If this would be a real issue, upload would have never worked

Not correct. Most installations have (a) magic_quotes_gpc turned Off in php.ini or (b) are on Apache so the "php_value magic_quotes_gpc 0" in .htaccess is getting run even if for some reason they have them turned On in php.ini.

To easily test on Apache/Windows, change "php_value magic_quotes_gpc 0" to "php_value magic_quotes_gpc 1" in the default .htaccess and try to upload something.

Also the patch is a hack.

Oh, how gracious and constructive of you to say. Thank you! :P Well, I've set it back to code needs review until you tell me what to fix.

Also, I just quickly included those related forum posts and did not fully look into each one. They are just possibly related to this issue, but this issue is definitely real.

Rob made me test with htaccess file set
php_value magic_quotes_gpc 1

files do not upload with this setting in htaccess on Windows 2003 Server with Apache 2

Still need to test patch later.

chx said to take out the windows specific stuff. "FILES needs a different strip routine because per that comment tmp_name does not have backslashes added. call your routine _fix_gpc_magic_files."

Even though this is a windows specific behavior, while dopry and I discussed earlier that we save one conditional over if we just did a _fix_gpc_magic_files(), it is cleaner to do it this way as chx recommended.

Re-added code comment that got lost from original patch "tmp_name does not have backslashes added see http://us2.php.net/manual/en/features.file-upload.php#42280 ".

I also said that the real problem here is that (per that php.net comment) slashes are not added to tmp_name so then we shall not remove then. If we do this, a very useful byproduct will be that Windows does not break. Other effects could be, do not know, if some other OS changes the function that generates tmp file names to have a backslash, that also won't break. But anyways, if something is not added, we shall not remove.

Rerolled the patch, it was slightly malformed. Otherwise good.

I will test that new patch.

RobRoy, which is the patch to test, yours or chx ? They came in within 4 minutes of each other.

Oups, sorry. The patch is now RTBC.

Committed to HEAD.

Here is the same patch rolled for 4.7.

This is the same patch that was RTBC for HEAD so maybe this isn't getting attention because I put it up for review needlessly.

@killes, can we get a backport of this to 4.7?

applied

Thanks!