Work around move_uploaded_file issues with safe_mode and open_basedir

Does the file upload work for other files (e.g. a user image)?

The path (temporary://spring-360.tar.gz) is the right one as far as i can tell.

Its seems to be a permission problem, not with the temp directory, but with your Drupal installation. The file cant be moved from the temp folder to its destination.

Actually the module that is responsible for this is update.module.

Since it also affects user images it's not only the update.module

Maybe it's a chown issue? Please check owners and permissions again and check if the directory is there sites/default/files/pictures

Welcome to core patching!

1. remove the == 1 from the if, it does nothing
2. add a ton of comments of what are you doing here :)
3. instead of directly storing $wrapper->realpath(); in $file->uri put it in a variable and check whether it's not FALSE. $new_uri = $wrapper->realpath(); if ($new_uri !== FALSE) like that
4. You need to roll the patch from the Drupal root. Also, we roll patches with diff -up. it helps -- it shows us which function you are patching
5. we like old_uri better than olduri

Hi y'all,

was this issue resolved in the final release of Drupal 7?
I just installed it on an OpenSUSE box and have the exact same problem as soon as I try to upload any file (image, translation etc). Changing permissions and paths have not improved the situation. The upload works but Drupal always wants to move files from the temp to somewhere else and that fails.
I really would appreciate your help with that.
Thx
Michi

Hi Joost,

thanks for following up.
I got it to work now but I can't tell what exactly fixed it. I was starting from scratch and still getting the issue but this time I was spending way more time in setting the permissions. I switched off the PHP SAFE_MODE and finally configured Plesk to also switch off its overriding settings.
So bottom line I would say it is all about permissions and I miss a complete doc that runs through all the possible scenario's or a tool that helps you to make the correct settings.
Thx
Michi

I saw your note regarding open_basedir, etc, however I just wanted to remind you that the Drupal System Requirements contains:

PHP needs the following configuration directives for Drupal to work (only directives that differ from the default php.ini-dist / php.ini-recommended):

• safe_mode: off. Safe mode may interfere with file and image uploads.

#14: patch-file-uploading.patch queued for re-testing.

the patch did not work for me and so i cant upload images with cck.

i have tried changing tht tmp fiel but no luck.

I guess i have to go back to d 6..

#14: patch-file-uploading.patch queued for re-testing.

#9: file.inc_.patch queued for re-testing.

#19 solved my problem. my rackspace host defaults to "safe_mode: on". Turning safe_mode off fixed me problems. Am I not safe now? ;-)

i re installed d7 and everything worked. Very funny?

wolfmarter

The problem still persist: Drupal 7.0 final cannot upload any files if running in safe_mode (and open_basedir), because of some sanity and/or security considerations buried somewhere deep in PHP (running 5.2.17 here, if that counts). I must admit that I don't exactly understand all this mess with stream wrappers, though.

Patch in #14 applied only with some warnings, so rerolled it (with only changes in comments and variable names), and this solves the problem (at least for me).

Doh, forgot the patch itself.

There is a duplicate issue of this somewhere in the queue.

I think the root of the issue is that PHP upload dir (upload_tmp_dir) inside the open_basedir paths. It seems that on some versions of PHP, even move_uploaded_file() fails to get the file from the upload dir if it is not in the list of allowed paths.

@Damien: sorry, but that's not the case here: open_basedir is something like "/home/foo/bar", and upload_tmp_dir is "/home/foo/bar/tmp". D7 is in "/home/fioo/bar/www" (which is the docroot in this case), and drupal is set to use "/home/foo/bar/tmp" (with this full path) for temporary files as well. OTOH: are you suggesting that PHP has some problems with these if the upload_tmp_dir/temporary files' dir _is_ inside open_basedir?

I haven't checked, though, whether the upload works if the temporary files' dir is inside the docroot, but wouldn't think that has anything to do with the issue (and would have other implications as well, wouldn't it).

I have the same problem when I try to upload something with drupal 7 (not only for images but for any file type)

Warning: move_uploaded_file() [function.move-uploaded-file]: Unable to access public://myfile.jpg in file_save_upload() (linea 1528 di /home/mysite/public_html/includes/file.inc).

I tried to change the permissions setting for temp dir and for imagefield storage dir and also to set a different dir for temp files (in and out of the site root directory).

I read about open_basedir and safe_mode, but I absolutely don't understand what it means (and so I suppose I haven't rights to change them).

My question is: only people with full access to a server can install and use drupal 7?

thanks, upload works for me with the patch

I vote for including it to default install in next D7 update. There's still lot of hosting services where safe_mode is on

open_basedir is active on my site and having the same problem, I successfully used the patch in #14.

bethod2: what about the patch in #28?

I used patch in #28. It solved this problem for me.

thanks for the help :)

How to patch?
Do I have to copy the code inside the patch file and paste inside the settings.php substituting the corresponding code lines?

In case of drupal core update
Do I have to re-patch?

Thanks

Do this patch (#28) will be included in next Drupal issue?

How to patch?
Do I have to copy the code inside the patch file and paste inside the settings.php substituting the corresponding code lines?

In case of drupal core update
Do I have to re-patch?

Thanks

marc: Please see http://drupal.org/patch/apply. If there is a new version of Drupal core, it may contain the patch (if the patch is thoroughly tested, marked as "reviewed & tested by the community," and merged into the Drupal core CVS repository by dries or webchick).

This is not critical putting it back to original status

Ok. But the problem persist.

It is not possible to upload files and images with Drupal 7.0 without changing some server settings reserved to server administrators.

This is a strong limitation to diffusion of Drupal 7 against other popular CMS.

This patch did not solve the problem for me.

Two patches:

1. The first implements a slightly cleaned up version of Boobaa's patch in #28.
2. The second creates a new function drupal_move_uploaded_file() which moves the work-around code to a separate function. Although move_uploaded_file() is only called once in all of core, it may be worth creating this separate function for code readability, potential contrib usage, etc. It might be, however, a little late in the game for API additions.

Both were tested on a MAMP PHP 5.3 environment with safe_mode both on and off. (I didn't do any testing, however, with open_basedir configured).

Since these are functionally equivalent to Boobaa's patch in #28, more investigation will need to be done to address samtaylor's issue in #41 --- however that may prove to be a separate issue.

I tried the conservative patch, but all it did was move the same error into a javascript Ajax dialog.

The underlying error I'm getting is:
Warning: filesize(): stat failed for public://image.jpg in file_save() (line 575 of /home/patcms/public_html/includes/file.inc)

#41 is not a bug report, it's noise, #43 is another issue. I can't decide between the two patches but certainly both are good and core worthy.

For the underlying error in #43 I created http://drupal.org/node/1051736

That said, my point was that when I applied the conservative patch, the error stopped appearing in the page in the usual box and began appearing in the a JavaScript dialog box (at least in Safari).

+  if (!ini_get('safe_mode') || !($destination = drupal_realpath($file->uri))) {
+    $destination = $file->uri;
+  }

This is perfectly unreadable.

Also, do we have any idea where the actual issue is? Do we have a reference to a PHP bug report? A list of affected PHP versions? We are shooting blind here, people. Let's do the homework first.

Just mudding the waters a little bit more, but PHP manual clearly states:

move_uploaded_file() is both safe mode and open_basedir aware. However, restrictions are placed only on the destination path as to allow the moving of uploaded files in which filename may conflict with such restrictions. move_uploaded_file() ensures the safety of this operation by allowing only those files uploaded through PHP to be moved.

Which makes me wonder if we are not chasing another issue here (for example, our streamwrappers not working at all in safe mode).

Also, is this a duplicate of #357469: file_save_upload() fails to handle open_basedir restriction?

@Damien Tournod in #49: My testing would indicate that this is not connected with open_basedir, as I can trigger the bug safe mode enabled and open basedir disabled.

I think you are correct in your assumption that the underlying bug is 'safe mode + stream wrappers = broken.'

For what it's worth....
I came across this error on a client's share-hosted Drupal 7.0 (5-Jan-11) site every time they tried to upload a file (to a custom content type).
The site runs PHP 5.2.14 with a configure command that includes '--enable-safe-mode'. I can't find any reference in the site's phpinfo() output to open basedir.

Warning: move_uploaded_file() [function.move-uploaded-file]: Unable to access public://myfile.png in file_save_upload() (line 1528 of/home/flinkco/public_html/content/includes/file.inc).

What fixed it for them, which may not be a comprehensive fix, is to change includes/file.inc, line 1528 from:

  if (!move_uploaded_file($_FILES['files']['tmp_name'][$source], $file->uri)) {

to

  if (!move_uploaded_file($_FILES['files']['tmp_name'][$source], drupal_realpath($file->uri))) {

This seems to be along the line of what was proposed above...

Greetings to all,

I had the same problem. I always encountered similar warning
Warning: move_uploaded_file() [function.move-uploaded-file]: Unable to access public:// ........... in file_save_upload() (line 1528 of/ ........... /includes/file.inc).
when I tried to upload any file. This included:

• Uploading of image to Article - pre-defined content type in Drupal 7.
• Uploading of image using IMCE.
• Uploading of archive when trying to install a new module.

I use Drupal 7.0 from 2011-Jan-05, there is PHP Version 5.2.14 on the webhosting I use. The safe_mode is set to On by the hosting provider.

I made the small change to the the includes/file.inc, as described by RdeBoer in his post from February 16, 2011. This solved the upload-related problem in my case.

I can confirm the fix in http://drupal.org/node/1002048#comment-4097842 works, but this is a core file fix and not ideal. Need to explore all options.

For the record also using a shared host

#53 code worked for me too.

Yep.. #53 worked for me also.. now able to upload.. with safemode On. Thanks and big catch.

#53 code worked for me, fantastic!

In the File manager - where it states /tmp simply remove the "/" works here!

We have this issue but don't have safe mode on, just open_basedir set (IIS7.5, PHP 5.2.14).
Found this user contributed note:
http://php.net/manual/en/book.stream.php#101981

Perhaps the bug is '(safe mode OR open_basedir) AND stream wrappers = broken'.

#51 code (#53 code) worked - thanks RdeBoer!!

RdeBoer's solution works for me too, thanks!

#51 code thanks RdeBoer!!

RdeBoer's solution works for me too, thanks!

And I'm whit marc_antonio in is comment #40 http://drupal.org/node/1002048#comment-4031260

Hi everyone,

I´m having the same problem at line 1528 of file.inc.
I read carefully the whole thread and understood that RdeBoer´s patch worked for many users.

My site is hosted on Bluehost and it´s a clean new Drupal 7 installation with Marinelli theme. The copy of the site on my local instalation is working OK, so I conclude that the problem is something related with Bluehost environment.

I followed Bluehost instructions for setting safe mode Off and open_basedir to none, but I could not check that it´s working because I don´t find it reflected on any system´s variable list.

I applied the patch proposed by RdeBoer on file.inc, and after trying to upload an image I get the following error.

•Warning: move_uploaded_file() [function.move-uploaded-file]: Filename cannot be empty in file_save_upload() (line 1528 of /home2/user/public_html/LaMennais/includes/file.inc).
•Warning: move_uploaded_file() [function.move-uploaded-file]: Unable to move '/var/tmp/php1chNVN' to '' in file_save_upload() (line 1528 of /home2/user/public_html/LaMennais/includes/file.inc).
•File upload error. Could not move uploaded file.

I will appreciate your help!! :)

I want to add that I had the same problem when uploading a shortcut icon for a theme, and I fixed it with the solution mentioned by RdeBoer at #51.

#9: file.inc_.patch queued for re-testing.

#53 code worked for me too.

Attempting my first patch...
This is a patch for the solution at #51 (reiterated at #53).

Wahoo!! Is it appropriate to say ↑↑↑?

drupal_realpath in the code comment should be drupal_realpath(). Also moving to 8. and adding backport tag.

Good call - updated.

Any ideas why this patch was ignored?

Hyphens instead of underscores?
Duplicate comment number?
The patch teser doesn't like my face?

Ah.
Sorry for the patch spam. : /

Pity that due to my patch noob-ness this one missed Drupal 7.2.

@Akaoni: patches only get tested at "needs review"

@longwave: Thanks for clarifying. Only figured that out after subing 2 extra dup patches. :(
Won't make this mistake in future.

#53 did work but...
Got a HTTP AJAX error with GalleryFormatter module.
Resolved this by manually creating the tmp directory.

"The patch tester doesn't like my face?" LOL :)

A couple of things:

1. It's customary to not mark your own patches "reviewed & tested by the community". "needs review" is a signal to your peers that they need to take a look, and we like to have independent sign-off of every fix before it makes it to committers, just to provide a different POV. It sounds like from #74 this might actually still need a little bit of work. So marking back to "needs review."

2. We also want to make sure we have automated tests for bugs like this (the more broken, the more we need tests) so that we don't introduce these problems again in future releases. I don't think this is possible in this case, because of all this open_basedir() nonsense, but just something to keep in mind for future patches.

Sorry. Removing that.

also #1083982: Fix support for remote streamwrappers removes a lot of realpath() calls, this is adding one, please ensure they don't conflict.

I'm not in favor of adding the drupal_realpath() call either. As far as I can tell, nobody really knows what the true cause here is -- some sort of interaction between open_basedir, safe_mode, drupal stream wrappers, and some versions of PHP.

@ablheza: It looks like your having a different issue from this one. Seems the code is trying to move the uploaded file to nowhere (Unable to move '/var/tmp/php1chNVN' to ''). Perhaps ensure that your Public/Private file paths are correct (Admin > Config > Media > File system).

@webchick:
1. Fair call. The patch only uses RdeBoer's code which I felt had been reviewed and tested by many users (including myself). Your right though, in that the patch itself should still go through the proper process. I'll adhere to this in future. ;)
#74 seems to have been a seperate issue.

2. I'm keen to learn more about automated testing in Drupal but haven't found anything on it. Care to point me towards some doco?

@catch: Good point. Perhaps the best way forward is try using stream wrappers and then fallback to drupal_realpath() if that fails (due to safemode, open_basedir, or anything else that the move_uploaded_file() function fails to map properly). Given that this is no longer a simple one-liner, I think bfroehle had the right idea in creating the new wrapper function drupal_move_uploaded_file().

@bfroehle: As outlined above, would you be more in favour of adding drupal_realpath() as a fallback for when stream wrappers fail?

Based on all this, new patch attached (expanding on bfroehle's previous patch).

Please adhere to http://drupal.org/coding-standards as well (missing curly braces of if, missing periods, whatnot).

+++ b/includes/file.inc
@@ -1550,6 +1550,37 @@ function file_save_upload($source, $validators = array(), $destination = FALSE,
+  if (!$result) $result = move_uploaded_file($filename, drupal_realpath($uri));

Improper coding style here. Please read the Drupal coding standards handbook page.

Powered by Dreditor.

#79 patch rerolled with proper coding style.

Thanks for that Boobaa - old habits die hard!!

Finally got creating patches sorted on my work system. I should now be able to turn things around faster than 3 days. ;)

+++ b/includes/file.incundefined
@@ -1550,6 +1550,39 @@ function file_save_upload($source, $validators = array(), $destination = FALSE,
+  $result = @move_uploaded_file($filename, $uri);
+  // If the move failed, attempt to move the file using the real path
+  if (!$result) {
+    $result = move_uploaded_file($filename, drupal_realpath($uri));
+  }

I'm still worried this might negatively impact remote stream wrappers, since their errors will their errors ever be visible?

Also does this code handle the possibility of drupal_realpath() returning FALSE? I suspect that will lead to some bizarre errors.

Powered by Dreditor.

Both fair points.
Fixed.

Patch had a TortoiseGIT warning message in it.
Retrying.

*sigh* Missing LF at end of patch file.

(Server CentOS 5.6, PHP 5.3, MySQL 5.0.77)
The hack in post #53 did help and now works for me without problems.

Thank you #53!

OK, let's sum things up, as this is the most I can add here.

The relevant comments are: #30, #42, #46, #48, #51/#66, #71*, #77, #78, #84. Everything else may be skipped as noise.

Anyways, I wanted to kick this issue a bit further, but I really got lost here. I don't know if it's a PHP or a Drupal bug either; don't know if it's triggered only by safe_mode and open_basedir, or some specific PHP versions, or stream wrappers, or anything else, or even some combinations of these. I don't know if the issue gets solved somehow, it could be simpletested or not. I don't know if it even works with remote stream wrappers, as I haven't tested it yet.

All I know is the patch in #71* works, satisfies the testbot, adheres to the coding standards, and doesn't seem to have reported implications. Finally I think it should be stressed that though I do understand the version changes throughout the life of this issue so far, it does affect all 7.x instances badly (on some hosts, which should be supported, I think).

ppl with ftp access can install drupal. but for using it, that is set at the permissions module. at what level is given to admin/webmaster. user, along with access

Maybe this helps...

I've installed 2 sites with drupal 7 both on shared hosting.
The first works fine(php 5.3 - http://www.reisspecialist.net/phpinfo.php)
the second I've experienced the same problems as described above (php 5.2 - http://www.bedrijfevents.nl/phpinfo.php)

Both have the same drupal version etc...

Hope this helps...

The patch in #82 does not work. Though images seem to be appearing, I am getting tons of errors and warnings during upload:

- The file permissions could not be set on public://donkey.jpg.
- Warning: filesize() [function.filesize]: stat failed for public://donkey.jpg in file_save() (line 573 of /home/boobaa/drupal/d7site/includes/file.inc).

Let's stick with the patch in #71 as it's the last version that both actually works and passes the tests. Anyway, it would be great if we'd know what's going on here.

@pyranha: Thanks for the added info.

I'm at a bit of a loss as to why patches #82 and later aren't working. On the face of it they should be. We are using patch #90 in our TEST environment without issue.

I was hoping to chase the problem down but haven't found the time. :(

The concern with patch #71 is that while we may be fixing this issue, we might be creating another. This may break uploading to remote streamwrappers as they might not be resolvable by drupal_realpath().

#90: core-fix_file_upload-1002048-90.patch queued for re-testing.

#14: patch-file-uploading.patch queued for re-testing.

#90 definitely needs work. Test failures look real and it has typos.

Retitling as well.

Allow me to post some additional information regarding this issue.

Based on the informations posted in #95, I moved to PHP 5.3 with expectation not to encounter this problem anymore. However, the problem still persists. I took the second look to phpinfo() of PHP 5.3 hosting from #95 post. There is safe_mode set to Off and I believe this parameter makes the difference, not the PHP version.

I made additional tests on PHP 5.3 and also with the updated version of the Drupal. Results:
PHP: 5.2.17 ---- safe_mode: On ---- Drupal: 7.2 ====> error
PHP: 5.3.6 ---- safe_mode: On ---- Drupal: 7.2 ====> error
PHP: 5.3.6 ---- safe_mode: On ---- Drupal: 7.4 ====> error

As already mentioned above, I believe this issue is not PHP-version related (5.2 vs. 5.3). The safe_mode seems to make the difference. I hope this post might be helpful to someone.

Same issue on a shared hosting at Koumbit.org, fixed by applying the changes in #90.

I have edited the patch in #90 so it will apply cleanly to Drupal 7.7, using patch -p0

queued for re-testing

Lets try this again.

@catch: Thanks for that. Fixed my typos and yours. ;P

@Barney Gumble: Cheers for the additional info!!

@sfyn: Thanks, mate. ;)
There seems to be a bug in the patch which only surfaces if you don't have the above mentioned problem.

Managed to look at this **finally** and I'd say it's a basic logic issue (whoops!!).
Hopefully this tests OK.

woOtz!!!

If everyone's happy with this patch please test it.
Hopefully we can get it tested, marked "reviewed & tested by the community" and included in the next Drupal release.
Would be nice for those of us with safe_mode or open_basedir to not have to patch the core to upload. ;)

+    // Otherwise, retry the failed move so that a useful error message is produced.
+    else {
+      $result = move_uploaded_file($filename, $uri);
+    }

Rather than retrying the move (which will fail), can we just generate our own warning? Or can we set a temporary error handler to catch the warning omitted in a previous call?

(This whole thing makes me wish PHP warnings could be caught like exceptions).

It appears this is possible -- see the ErrorException class.

Could we then use code like:

function drupal_move_uploaded_file($filename, $uri) {
  set_error_handler("exception_error_handler");
  try {
    $result = move_uploaded_file($filename, $uri);
  }
  catch (ErrorException $e) {
    // Try again using drupal_realpath()
    // ...
  }
  restore_error_handler();
  return $result;
}

Drupal already creates it's own error if the file move returns FALSE:

if (!drupal_move_uploaded_file($_FILES['files']['tmp_name'][$source], $file->uri)) {
    form_set_error($source, t('File upload error. Could not move uploaded file.'));
    watchdog('file', 'Upload error. Could not move uploaded file %file to destination %destination.', array('%file' => $file->filename, '%destination' => $file->uri));
    return FALSE;
  }

I just thought it might be useful to keep the warning(s) move_uploaded_file outputs for extra info. Happy to remove this, happy to leave it in. ;)

This isn't a show-stopper however, and, as such, maybe we discuss improvements like this in a new issue?
I'm keen to see this fix get into the next Drupal release as I'll have had to patch the core 8 times all up to fix this (7.0, 7.2, 7.4, and 7.7 over two sites).
Serves me right for being vigilant with upgrades!!

Tagging.

I just rerolled with the patch in #109. It works as advertised

+++ b/includes/file.inc
@@ -1565,7 +1565,48 @@ function file_save_upload($source, $validators = array(), $destination = FALSE,
+ ¶

Trailing white-space.

+++ b/includes/file.inc
@@ -1565,7 +1565,48 @@ function file_save_upload($source, $validators = array(), $destination = FALSE,
+ * @see http://php.net/move_uploaded_file
...
+ * @see move_uploaded_file()

The second/last is sufficient. (both effectively point to the same resource)

+++ b/includes/file.inc
@@ -1565,7 +1565,48 @@ function file_save_upload($source, $validators = array(), $destination = FALSE,
+    if (drupal_realpath($uri)) {
+      $result = move_uploaded_file($filename, drupal_realpath($uri));

Let's invoke drupal_realpath() only once.

+++ b/includes/file.inc
@@ -1565,7 +1565,48 @@ function file_save_upload($source, $validators = array(), $destination = FALSE,
+    // Otherwise, retry the failed move so that a useful error message is produced.

Exceeds 80 chars.

27 days to next Drupal core point release.

Applied Issue summary standards.

New patch created w/ fixed whitespace, documentation, and invoking drupal_realpath() once.

@sfyn: Thanks for your help testing, mate. Unfortunately this new patch now needs testing. :( Maybe give it a day or two before you do to ensure everyone's happy with the code.

Applying Issue summary standards

subscribing

Subscribing.

Greetings to everyone, I just tested the patch #116. It works fine for me. Few days ago I also tested patch #109, which also works fine.

Patch applied to Drupal 7.7, running on PHP 5.3.6 with safe_mode=On. After applying this patch I am able to upload any file or install the module/theme from file.

Thanks to all contributors & community members, who work hard to fix this issue.

However, after switching to Drupal 7.7, I am not able to install modules/themes from URL, only the installation by file upload is possible. I don't know if this is related to this issue, but I don't know any better place where to mention it, being a newbie to Drupal community.

Before an attempt to install theme/module directly from URL, I had to apply patch discussed in threads #1205948: Installing new modules fails after update from 7.2 -> 7.4 and #1206916: Notice: Use of undefined constant WATCHDOG_ERR to not encounter the "undefined constant WATCHDOG_ERR" problem, which is not related to this issue. After this, I receive following error report:

http://ftp.drupal.org/files/projects/pathauto-7.x-1.0-rc2.tar.gz could not be saved to temporary://update-cache-859ece52/pathauto-7.x-1.0-rc2.tar.gz.
Unable to retrieve Drupal project from http://ftp.drupal.org/files/projects/pathauto-7.x-1.0-rc2.tar.gz.

As mentioned above, this thread might not be the right place to mention this error. But I have no idea if this problem is/isn't related to this issue.

Greetings!
This is my first post in this forum, so please bear with me. When I try to change the file.inc file as described in #51, and then try to upload an image file, I get the error message "The website encountered an unexpected error. Please try again later." Does anyone have an explanation why this might be? Thanks in advance.

Franbk

@FrankHoldt: Instead of modifying the file, try the patch in #116. This has the most current solution.

@Barney Gumble: Thanks for taking the time to test this patch!! ;)

Having a quick look at the errors your getting, the problem seems to stem from the system_retrieve_file() function.
It's successfully getting the file from the URL but can't then save it to the temp directory.
Perhaps check the setting for the Drupal Temporary directory and your directory's permissions.

The URL retrieve and upload functions look quite seperate so I don't think these issues are related.
If it was, you wouldn't be able to install projects by upload. ;)

Will install Drupal 7.7 and double check.

@FrankHoldt: Welcome to contributing to Drupal!!

I agree with xjm that you should be using patch #116.
Here's more info if you're unsure:
http://drupal.org/node/707484
http://drupal.org/node/620014

Otherwise, send me the version of Drupal you're using and I'll send you a patched file.

@Barney Gumble: Another good way to test if this patch is related to the install by URL issue your experiencing is to revert the patch. If you still can't install by URL then it's not related to the patch. ;)

@Akaoni: Yes, you are right. I gave it another try with file.inc without #116 patch. First attempt with "clean unpatched" file.inc of Drupal 7.7 version. I tried to install new module from URL, NOT by file upload. As expected, the two errors followed. Both (undefined constant WATCHDOG_ERR and PDOException) can be clearly seen here - not my screenshot, but the same error. Links to related threads in my previous post #119.

Second attempt, this time with file.inc, only patched with this patch. As expected, following error occured:

* http://ftp.drupal.org/files/projects/webform-7.x-3.11.tar.gz could not be saved to temporary://update-cache-859ece52/webform-7.x-3.11.tar.gz.
* Unable to retrieve Drupal project from http://ftp.drupal.org/files/projects/webform-7.x-3.11.tar.gz.

Patch #116 was not used in both cases. Therefore patch #116 has probably nothing to do with the error message quoted above in code block.

PS: What to do now? Should I open another bug report thread?

@Barney Gumble: You're awesome!! Thanks heaps for further testing whether these two issues were linked. ;)
The fact that you can't install by URL even when this issue's patch is not applied is proof positive that this patch does not cause the problem.

Where to from here: Do a search in the issues to see if anyone else has this problem. If not create a new issue. Either way, let me know where it is and I'll try to help. ;)

@everyone else: Looking for people to please test this patch (#116) so we can get it committed!!
19 days to next Drupal core point release (based on sun's previous post).

Hmm looking at this I had the same thought as bfroehle in #111. Since that pattern doesn't exist in core yet, I'm fine not adding it in the patch, but we should at least open a new task linked from this one to introduce the pattern I think - ideally we want to kill the use of @function() from core since it is too often abused.

@catch See #1247666: Replace @function calls with try/catch ErrorException blocks.

(Only) installed patch #116, and it seems to work fine (on a web hosted Drupal 7.7, safe_mode on). Humble thanks :-)

I have the problem mentioned in 124.
I have tested the patch in #116 but this did not change anything for me.
I am running 7.7 and before that it was 7.2 where I didn't had any problems at all.
I made an issue here:
http://drupal.org/node/1240256

Some one closed that because of this but I do think that this bug for version 7.7 should have it's own issue instead keep adding to this very long issue that original was/is for work on version 8.

Hope it is ok to mention this. I just wish to try and keep those issues easier for others to get to :-)

@jenspeter: I just posted some additional information to the mentioned thread (http://drupal.org/node/1240256). However, nothing about how to solve this problem yet. Installing of the #116 patch should AFAIK not solve your problem with module update, but it should allow you to install the new module by file upload or to upload an attachment (image) to content nodes (article by default).

subscribing

When I was trying to upload in settings of Marinelli theme some banner, there is an error:

Warning: move_uploaded_file() [function.move-uploaded-file]: Unable to access temporary://297422_195758750487122_182044571858540_526196_6914407_n[1].jpg w file_save_upload() (linia 1541 z /var/customers//pawi.pl/includes/file.inc).

line 1541 says:

  if (!move_uploaded_file($_FILES['files']['tmp_name'][$source], $file->uri)) {
    form_set_error($source, t('File upload error. Could not move uploaded file.'));
    watchdog('file', 'Upload error. Could not move uploaded file %file to destination %destination.', array('%file' => $file->filename, '%destination' => $file->uri));
    return FALSE;
  }

in reports about my page there is info about to install:
PECL uploadprogress or APC.

anyone could help me?

#132 Did you try the patch in #116 ?

@bfroehle, @catch: Thanks for creating a new issue. I'm definitely a fan of the "bug fix comes first, code improvement second" philosophy!! ;)

@Lars Bo Jensen: Thanks for testing the patch!!!

@jenspeter, @Barney Gumble: I agree with you both that your issue is a seperate one and should have it's own thread. Will give you guys some help to fix it @ #1240256: file_unmanaged_copy() Fails with Drupal 7.7+ and safe_mode or open_basedir.

@pawi81 xjm is right. Please apply patch #116 and let us know if it solves the issue for you.

@xjm You rock!!! Thanks for the assistance!! ;)

I don't know if I do this correctly, because i changed it manually

I've attached my file.inc - could anyone check it out?

#116 worked for me. (D7.7, php5.2.17,safe_mode on, shared hosting on apache vhosts)
Thanks.

@muka how can I easy commit this patch? I must use some utility to do this?

#137: If you are on a Macintosh or Linux server, download the latest patch file to the root of your drupal installation. Then type at the command line:

patch -p1 < core-fix_file_upload-1002048-116.patch

You can also apply the patch using git, but the above is easier in many cases.

Edit: On a Windows server you can apply the patch the same way using Cygwin or similar, or download patch for Windows.

The bug in #1240256: file_unmanaged_copy() Fails with Drupal 7.7+ and safe_mode or open_basedir is the same logical issue (safe mode not handling temp:// paths properly), with the same logical fix (retry with realpath destination). I've attached a patch to that issue, but not looked for other instances of the same pattern.

@pawi81: Yep, that file looks like it should work. Did it fix the problem for you?

@muka: Thanks for testing the patch!!!

After I patched with #116, I still got error, it say "The specified file temporary://file.name could not be copied to public://file.name" in Recent log messages.

After look around I found this error come from line 908 in file.inc. And when I change from

if (!@copy($source, $destination)) {
    watchdog('file', 'The specified file %file could not be copied to %destination.', array('%file' => $source, '%destination' => $destination), WATCHDOG_ERROR);
    return FALSE;
}

to

if (!@copy($real_source, $real_destination)) {
    watchdog('file', 'The specified file %file could not be copied to %destination.', array('%file' => $source, '%destination' => $destination), WATCHDOG_ERROR);
    return FALSE;
}

Now, it work fine.

@mementototem: it looks like you are posting your comment to the wrong issue?

@mementototem: lyricnz is right. See http://drupal.org/node/1240256.

+  // Attempt to move the file using the provided URI. Errors are suppressed here
+  // so that if streams aren't properly supported, we don't see a warning
+  // message for every upload.

Reading this I would ask "if streams aren't properly supported" what does it mean streams aren't properly supported? What about merging this and the third inline comment into the second saying:"PHP's move_uploaded_file() does not properly support streams if safe_mode open_basedir are enabled so if the move failed, try finding a real path and a move operation."

I bet we can't write a test for this so if the comment is fixed this is RTBC.

@chx: Thanks for the review!!

New patch with chx's comment suggestion (functionally the same as #116).

Added patch link.

Attempted test case.
Also added a bit more info to Upload Test results.

This should fail but probably won't as I think QA is PHP 5.2.17 and you can only set open_basedir in PHP > 5.3.
Edit: turns out QA runs 5.3.3.

It did fail but not for the right reasons.
Next attempt.

+++ b/modules/simpletest/tests/file.testundefined
@@ -664,10 +664,30 @@ class FileSaveUploadTest extends FileHookTestCase {
+    $open_basedir = DRUPAL_ROOT . ':' . dirname($image4_realpath) . ':' . drupal_realpath(file_directory_temp()) . ':' . drupal_realpath('public://');

I think what you wanted here and the next few lines is implode(PATH_SEPARATOR, array(DRUPAL_ROOT, ...)). See http://php.net/manual/en/function.set-include-path.php for a fairly usable mention of this constant. I have committed some changes to the PHP manual because this constant was nearly undocumented. Edit: also see http://php.net/php_uname on this matter -- it lists the values of a few OS related constants.

Updated patch #.

@chx: Great suggestion!! Will included in my next attempt. ; )

OK. I've given this test a really good crack and I think we're going to have to call it un-testable for now. :(
Can we please mark this RTBC and commit patch #145 to core?

My Findings

I can step-up the open_basedir directive but due to an outstanding PHP bug I cannot step it down again.
Unfortunately, this has a few rammifications for the rest of the file upload tests.

The NULL cookie jar issue causing the errors in patch #148 I managed to solve by specifying a file within open_basedir.
This file appears to either never get created or properly cleaned up after tests.

There is also a problem with the file munge test that stems from a failed file_move in setting up that test.
This could probably be solved...
Problem is however, this issue doesn't seem to be a problem with open_basedir in 5.3. Only open_basedir in <5.3 and safe_mode in all PHP versions.
As such, any environment were I can perform the test (by stepping-up open_basedir) will never have this issue in the first place.

Only way I can think of getting a meaningful test for this is by setting open_basedir in the PHP environment.
Coincidentally, in chats on IRC, this issue may have renewed vigor for setting up multiple PHP environments for qa.drupal.org.
Only time will tell...

Updated remaining tasks.

Remove create test from remaining tasks.

Rerolled #145 for both 7.x and 8.x, so now they are applying without offsets (the only difference between the attached two is the offsets, for webchick's ease). Tested it on 7.7, seems to work fine for me, so requesting RTBC.

Sure. There is only so much PHP brokenness we can fight. This whole issue is about that. If we can't test due to a PHP bug, we can't test. Big deal.

#116 worked for me on PHP Version 5.3.2-1ubuntu4.7

Should I undo and og to #145 ? What is the process for patches being accepted into new versions of the main drupal package? Sorry if I'm displaying total ignorance here! Thanks

@deepsoulstarfish: #116 and #154 have the same functional code, so you are fine to keep using #116 until this is committed to core. It is marked RTBC and tagged for backport, so the next step will be the core committers reviewing the patch and determining whether or not to commit it. If they okay it, the patch would be included in the next point release of Drupal 7.

Added commit patch to remaining tasks.

Wow, great job with all of the epic work in this issue! Looks like this has positive testing results from various folks, and we've ruled out being able to run automated tests on it.

Ergo, committed and pushed to 8.x and 7.x. Thanks!

It is still one problem (at least, in 7.9) when safe_mode is on:
if (!@copy($source, $destination)) {
in file.inc. This function don't working with stream wrappers in Safe mode too.

Patch, that fixing this issue is attached.

I've discovered, that 8.x needs this patch too:
(fixed patches attached)

@mva.name - please open a new issue for this attaching both patches, linking from here. This issue is already at 160 comments which makes it hard to follow.

@mva.name: Thanks for your work on this, however there is already an issue for the bug you mention:
http://drupal.org/node/1240256

Perhaps you would be interested in helping out there?

Just in the hope this might be helpful to somebody else out here, I found that, even with the patch applied, there is one special circumstance (apparently due to PHP) in which writing into files/ is not allowed:

drwxrwxrwx+ 13 root www-data 12288 Nov 22 19:04 files

Apparently the fact that it's 0777 doesn't matter if the owner is root... or so it seems. I'm currently looking deeper into this but, just so you know, making it

drwxrwxrwx+ 13 www-data www-data 12288 Nov 22 19:05 files

fixes it (through chown www-data files/, of course). This *might* be linked to ACLs under Linux, but it's definitely some PHP problem.

Hello dear Buddies many many thanks for the hint and the work!!

I have the same issues here - but workin on a drupal commons 6x 24 What do you say!? Do you think that i can apply the patch too!?
see the description and all the things i have tried:

my thread
http://drupal.stackexchange.com/questions/21640/commons-6x24-the-selecte...

and another one that has reported the same issues:
http://drupal.stackexchange.com/questions/13791/get-file-upload-error-wh...

love to hear from you

greetings
unleash

Updated issue summary.

#154 not working for me. I tried the drupal 7 version.