Problem/Motivation

The install.php script and the "Reports > Status" Report both check for the files directory to have read/write permissions, but they should be checking for read/write/execute permissions instead.

Steps to reproduce

  • Drupal is already installed
  • (goof up permissions) chmod 766 ./sites/default/files
  • Click Reports > Status Report
  • File System is green (should be red here, directory is not executable). No big deal yet
  • Click on Configuration > Performance
  • Click either Aggregate checkboxes under Bandwidth optimization or both
  • Clear all caches
  • You'll notice the CSS problem right away if that's what you selected to aggregate

Results

The specified file temporary://file6GnL0E could not be copied, because the destination directory is not properly configured. This may be caused by a problem with file or directory permissions. More information is available in the system log.

Proposed resolution

Install.php and Reports > Status should check for read/write/execute permissions (x77) on the directory in question

Remaining tasks

Contributor tasks needed
Task Novice task? Contributor instructions Complete?
Reroll the patch if it no longer applies. Instructions
Update the issue summary Instructions
Update the issue summary noting if allowed during the beta Instructions

User interface changes

  • None known.

API changes

Two functions will be added to the API:

  • file_directory_is_writable($uri)
  • drupal_is_executable($uri)

Original report by [mlehner616]

The install.php script checks for the files directory to have read/write permissions (x66), should be checking for read/write/execute instead (x77).
If the files directory is created with just read/write permissions, turning on caching/compression strips off CSS and displays errors regarding temporary files.

Install.php should check for this correctly, as well as the status report script.
Basically these two scripts don't care if execute permissions are set on ./sites/default/files

This may be a critical issue but I'll let someone more technical triage this one.
Tested multiple times and was able to reproduce this behavior:

Scenario 1

Drupal is already installed
(goof up permissions) chmod 766 ./sites/default/files
Click Reports > Status Report
File System is green (should be red here, directory is not executable). No big deal yet
Click on Configuration > Performance
Click either Aggregate checkboxes under Bandwidth optimization or both
Clear all caches
You'll notice the CSS problem right away if that's what you selected to aggregate

Here are the errors:
The specified file temporary://file6GnL0E could not be copied, because the destination directory is not properly configured. This may be caused by a problem with file or directory permissions. More information is available in the system log.
The specified file temporary://fileNY1O4m could not be copied, because the destination directory is not properly configured. This may be caused by a problem with file or directory permissions. More information is available in the system log.
The specified file temporary://fileSsnd94 could not be copied, because the destination directory is not properly configured. This may be caused by a problem with file or directory permissions. More information is available in the system log.
The specified file temporary://fileV3PZdN could not be copied, because the destination directory is not properly configured. This may be caused by a problem with file or directory permissions. More information is available in the system log.
The specified file temporary://fileSFTvjv could not be copied, because the destination directory is not properly configured. This may be caused by a problem with file or directory permissions. More information is available in the system log.
The specified file temporary://fileLB2jpd could not be copied, because the destination directory is not properly configured. This may be caused by a problem with file or directory permissions. More information is available in the system log.
The specified file temporary://fileSKgNvV could not be copied, because the destination directory is not properly configured. This may be caused by a problem with file or directory permissions. More information is available in the system log.
The specified file temporary://file1KkKCD could not be copied, because the destination directory is not properly configured. This may be caused by a problem with file or directory permissions. More information is available in the system log.
The specified file temporary://fileCHUVJl could not be copied, because the destination directory is not properly configured. This may be caused by a problem with file or directory permissions. More information is available in the system log.
The specified file temporary://fileNcdmR3 could not be copied, because the destination directory is not properly configured. This may be caused by a problem with file or directory permissions. More information is available in the system log.

What's expected:
Install.php and Reports > Status Report should red flag the non-executable files directory (permissions must be 777)

Install.php actually does the same thing. It doesn't care that ./sites/default/files directory is 766 when it should be 777

Files: 
CommentFileSizeAuthor
#115 sites_default_files-944582-115.patch15.34 KBjproctor
#110 sites_default_files-944582-110.patch12.1 KBmvc
#102 sites_default_files-944582-102.patch9.19 KBalvar0hurtad0
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,775 pass(es), 87 fail(s), and 15 exception(s). View
#100 944582-is_exec_writ-100-8.0_0.patch9.93 KBankitgarg
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,693 pass(es), 89 fail(s), and 15 exception(s). View
#98 is_exec_writ-944582-98.patch9.93 KBadci_contributor
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,714 pass(es), 87 fail(s), and 15 exception(s). View
#95 944582-is_exec_writ-95-8.0_0.patch10.32 KBmgifford
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,921 pass(es). View
#93 944582-is_exec_writ-92-8.0.patch10.4 KBjohnish@gmail.com
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,787 pass(es). View
#91 944582-is_exec_writ-91-8.0.patch10.43 KBsuperspring
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,007 pass(es). View
#70 issue_944582-drupal-7.18.patch8.03 KBorlitzky
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch issue_944582-drupal-7.18.patch. Unable to apply patch. See the log in the details link for more information. View
#59 d7-944582-59-do-not-test.patch8.1 KBxjm
#59 d7.12-944582-59-do-not-test.patch8.03 KBxjm
#56 directory-executable-944582-56.patch8.36 KBDavid_Rothstein
PASSED: [[SimpleTest]]: [MySQL] 36,286 pass(es). View
#40 Screen Shot 2012-02-15 at 1.22.50 PM.png53.13 KBryanissamson
#31 directory-executable-944582-31.patch8.57 KBkathyh
PASSED: [[SimpleTest]]: [MySQL] 33,798 pass(es). View
#15 file_error_update.jpg61.05 KBxaav
#12 directory-executable-944582-12.patch9.54 KBDavid_Rothstein
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch directory-executable-944582-12.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots. View
#11 directory-executable-944582-11.patch8.94 KBDavid_Rothstein
PASSED: [[SimpleTest]]: [MySQL] 30,363 pass(es). View
#9 directory-executable-944582-9.patch6.08 KBDavid_Rothstein
FAILED: [[SimpleTest]]: [MySQL] 30,080 pass(es), 127 fail(s), and 238 exception(es). View
Capture.PNG60.88 KBmlehner616

Comments

anavarre’s picture

Version: 7.0-beta1 » 7.0-beta2

I have the same problem in 7.0-beta 2 and was forced to manually delete both the js and css directories within files to make the errors go even though I unchecked CSS/JS minification.

mlehner616’s picture

I see this as a major issue. It's not a show stopper but thinking of the less tech savvy users of Drupal perhaps we should bump it up. We don't seem to be getting the attention of the developers here.

anavarre’s picture

Version: 7.0-beta2 » 7.0-beta3
Status: Active » Needs review

Now with 7.0-beta3, everything seems fine.

zerodeux’s picture

Testing files access and ownership within PHP will never work correctly. You can't emulate the filesystem logic for all OS, ACL variant and whatever NFS strangeties. It just failed on a very simple case for me 10min ago.

If you need to know if a file can be written to, you fopen($filename, "r+") and check the result. Then use the handle or close it.

Besides it's racy (and thus risky) to check file permission, then manipulate the file. Countless security issues arise from this misconception.

Fixing this is quite complicated since drupal_verify_install_file() check if files, dirs or symlinks can be read, written or executed. Callers should actually *try* to do what they're only checking (read, write, exec), and then handling exceptions as usual.

And this function heavily calls drupal_install_fix_file() which means that drupal_verify_... does modify files. It tries to fix things, and user-friendlines is part of Drupal experience, so let's be it. But it will pass a given test if drupal_install_fix_file() succeeds *without* re-running the test. On a NFS setup it's possible for a chmod() to succeed without having the intended effect on bitmask (because the NFS server logic decides and it sometimes looks funny to a client). This:

if (!is_writable($file) && !drupal_install_fix_file($file, $mask))  { return FALSE; }

Should be (ugly, yes):

if (is_writable($file)) { return TRUE; }
elsif (drupal_install_fix_file($file, $mask) && is_writable($file)) { return TRUE; }
else { return FALSE; }

I see 12 calls of drupal_verify_install_file() in Core. Worth patching ? It's maybe not a security risk, although this design is flaky.

mlehner616’s picture

What about PHP's fileperms function? I'm not a familiar yet with the Drupal API but it seems that PHP's implementation of fileperms seems to work MOST of the time and should certainly work on a no frills CentOS 5.5 server which is the OS I'm having issues with.

substr(decoct( fileperms('.') ), 2); // To get '777'
substr(decoct( fileperms('.') ), 1); // To get '0777'

right?

mlehner616’s picture

lvbeck’s picture

same issue with 7.0-rc1

David_Rothstein’s picture

Version: 7.0-beta3 » 7.x-dev
Status: Needs review » Active

This should be "active", not "needs review", since there is no patch yet. It's probably not hard to write one, though :)

Let's keep this focused on the original bug report. Some of the last few comments seem like they're about something different.

David_Rothstein’s picture

Component: install system » file system
Status: Active » Needs review
FileSize
6.08 KB
FAILED: [[SimpleTest]]: [MySQL] 30,080 pass(es), 127 fail(s), and 238 exception(es). View

Here's a patch. I've only tested it lightly, though.

It seems there are a number of places where we check these permissions on directories. The issue really doesn't have a whole lot to do with the installer itself, so I'm moving it to the "file system" component also.

Status: Needs review » Needs work

The last submitted patch, directory-executable-944582-9.patch, failed testing.

David_Rothstein’s picture

Status: Needs work » Needs review
FileSize
8.94 KB
PASSED: [[SimpleTest]]: [MySQL] 30,363 pass(es). View

Apparently is_executable() is one of the PHP functions that doesn't always work well with stream wrapper URIs - that is causing many (if not all) of the failing tests.

Let's try it with a Drupal wrapper for that function instead, similar to some of the other replacement functions in file.inc.

David_Rothstein’s picture

FileSize
9.54 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch directory-executable-944582-12.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots. View

OK, so the tests pass. I guess it also makes sense to create a helper function for the combined writable/executable check, rather than repeating it everywhere. The attached patch does that.

mlehner616’s picture

mgifford’s picture

That patch applied well to my last RC. Still not sure if that works through the permissions problem on the site I've been trying to upgrade mind you.

xaav’s picture

FileSize
61.05 KB

I'm also getting this error.
See attached screenshot.

Christopher James Francis Rodgers’s picture

Version: 8.x-dev » 7.x-dev
Issue tags: -Needs issue summary update, -needs backport to D7

2011.06.07
This solution relates strictly to the Drupal 7 error...

===
Error:
The specified file temporary://file****** could not be copied, because the destination directory is not properly configured. This may be caused by a problem with file or directory permissions. More information is available in the system log.
===

The Solution for me was simply..
.
Go to 'Configuration' > 'Media' > 'File System'
[*root*]/admin/config/media/file-system
and change the 'Temporary directory' from...
/tmp
...to...
tmp

(Do Not use a "/" to begin the directory specification)

See also http://drupal.org/node/1106492#comment-4562780
for all of my known details of that error.

hbblogger’s picture

I have the same problem. I thought that it had to do with my server as drupal 7 works fine on WAMP, but I see that I am not alone! If I copy the file using Filezilla, all is well but if I have a file that needs to write something to a directory (such as an image) the problem returns. I get the same error as geoffreyfishing. I have tried changing group permissions to 'write' in my site and include directories but it has had no effect. I get an error when I try to change the name of the tmp file (basically saying that it is unable to create the file). I see that David produced some code presumably to fix the problem, but I am not sure what to do with this. This is a serious problem as it makes drupal very difficult to use.

sun’s picture

Version: 7.x-dev » 8.x-dev
Issue tags: +needs backport to D7
Christopher James Francis Rodgers’s picture

Version: 7.x-dev » 8.x-dev
Issue tags: +needs backport to D7

Moved: Consolidated into comment #16 above.

http://drupal.org/node/944582#comment-4231504

NotNotCow’s picture

#16 My god,

All afternoon trying to figure it out and that's the flippin solution!

[smacks forehead]

Thanks for posting!

NNC

rickmanelius’s picture

When trying to use development version of devel to generate content in Drupal 7, I get this same error. #16 doesn't solve. So hopefully the backported patch corrects this!

rickmanelius’s picture

Solved my problem.
So when you specify a file or image field in a content type, the directory isn't necessarily available already. If you try to run devel generate to make a batch of content, the lack of this destination folder throws the same type of error as original post above.

If you manually create a single node and enter the information for these fields, then devel can add additional items without a problem. So there is some disconnect with the field folders. Perhaps they should be created on the content type configuration save. But that still may lead to issues if something tries to access it programmatically before that.

a_webb’s picture

I get the #19 bolded error code, and cannot seem to fix. i am upgrading from 7.2 to 7.4. i tried #16 solution to no avail. my file.inc specifies the /sites/default/files/tmp folder, which is there and has 777 permissions. There is no upload_tmp_directory set in my php.ini file (as that setting is commented out.) I am at a total loss as everything else with my upgrade went fine. This is a big site for a college, so stuff like this *kills* productivity. Kudos to anyone who can help, I'll return the favor sometime!

gaurav26570’s picture

For me Solution #16 worked..Thanks.

xjm’s picture

Tagging issues not yet using summary template.

maxchock’s picture

I'm using Panels and this error cause all my page layout with Panel doesn't work.. error message from log:

TYPE	file
DATE	Saturday, July 30, 2011 - 16:14
USER	max
LOCATION	http://www.trekker.my/node/2
REFERRER	http://www.trekker.my/node/2
MESSAGE	The specified file temporary://filecAw4AM could not be copied to public://ctools/css/4314fe7864ddd4cd7ddcc2cd7b74323f.css.
SEVERITY	error
HOSTNAME	175.138.122.218
OPERATIONS	

my tmp folder work fine, but the "ctools/css/" just empty...

Fannon’s picture

Got this error with the latest 7.7 Version after an upgrade from 6.x

Solved this with giving 777 Permissions on several folders. But which ones need it really?

Greets,
Simon

drupleg’s picture

I had this problem too after moving my local development site to be live on a shared hosting environment - #16 fix worked partially - I was still getting errors on several other pages such as admin/appearance. Looking at the recent log entries, admin/reports/dblog, I found that this was due to Drupal wanting to recreate it's own cache directories. For example - the log entry showed the error:

File temporary://filepFKhQY could not be copied, because the destination directory public://css is not configured correctly.

I deleted the css directory at sites/default/files/css, then Drupal would happily recreate the css directory with the proper owner and permissions. This was also true for the directory sites/default/files/js. Once Drupal recreated those all of the errors were gone and all was right with the world.

nikkubhai’s picture

I am facing same problem. Changing /tmp to tmp does not work!

xjm’s picture

Status: Needs review » Needs work
Issue tags: -drupal installation issue, -file permissions +Novice

This needs to be rerolled for both #22336: Move all core Drupal files under a /core folder to improve usability and upgrades and the git migration. Tagging as novice for the task of rerolling the patch.

If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.

kathyh’s picture

Status: Needs work » Needs review
FileSize
8.57 KB
PASSED: [[SimpleTest]]: [MySQL] 33,798 pass(es). View

update for #30 - d8 /core and git. change this to needs work after testbot for other issues/comments. carry on.

andypost’s picture

I think this related and should be included #484554-7: Stop relying on Apache for determining the current path

mlehner616’s picture

mesr01’s picture

@nikkubhai (#29), you may try to manually create a subfolder inside /tmp, let's say /tmp/d7/subdomain.example.com (replacing last part with actual site folder name for recognition purpose, although any subfolder name would be equivalent). Set permissions to 777 for all three folders and change the setting in admin/config/media/file-system accordingly.

I noticed that writing directly to /tmp, even with permissions set to 777, won't work with some D7 installs. I don't know at this point if this is a problem with D7 or a hosting account feature.

TransitionKojo’s picture

Issue summary: View changes

Updated summary to comply with Issue Summary Standards

TransitionKojo’s picture

Left out my username: Transition (with help from XJM)

TransitionKojo’s picture

Issue summary: View changes

added "code" tags around error message in results section; User: Transition

TransitionKojo’s picture

Issue summary: View changes

Updated "API changes" section to reflect 2 API Additions; user: Transition

TransitionKojo’s picture

Issue summary: View changes

added "related issues" sub-section in "Remaining Tasks" section; user=Transition

TransitionKojo’s picture

Issue summary: View changes

Updated "Remaining Tasks" section; user=Transition

TransitionKojo’s picture

Issue summary: View changes

Corrected link to Item #31; User=Transition

TransitionKojo’s picture

Issue summary: View changes

fixed typo in "

  • Patch in #944582-31: ./sites/default/files directory permission check is incorrect during install AND status report implements the proposed solution and passes automated tests.
  • "; user=Transition

    xjm’s picture

    Summary added by Transition.

    jagalbraith’s picture

    Version: 8.x-dev » 7.10

    Guys this is an easy fix. It is only a Unix files permission problem. Here is the fix:

    1. Change directory to the root of your Drupal site.

    cd /var/www

    2. Set your permissions to 777 and try it out. You can back off permissions to lock it down later, but get it working for now. Ensure to do it RECURSIVE. the -R is what is killing people in this post because there are already files in the directory that need to be writable for Drupal to work properly.

    chmod 777 -R sites/default/files

    3. Now create a directory called tmp at the root of your site.
    mkdir tmp

    4. Set the permissions there too.
    chmod 777 -R tmp

    Enjoy.

    John Galbraith
    www.binarybb.info

    Tor Arne Thune’s picture

    Version: 7.10 » 8.x-dev
    xjm’s picture

    Issue tags: +Needs manual testing

    Tagging for manual testing.

    xjm’s picture

    Issue summary: View changes

    Updated issue summary.

    ryanissamson’s picture

    Assigned: Unassigned » ryanissamson

    I will test it.

    ryanissamson’s picture

    I went through the steps and was able to reproduce the issue.

    I then tested the patch and upon clearing the cache received a warning message that there was something wrong with my Drupal installation.

    After navigating the to startus report page, I had the appropriate error. (See screenshot)

    message

    So, the patch looks good :D

    Let me know if I missed something or need to do something else.

    Thanks all! And thanks xjm for your continued help and support.

    ryanissamson’s picture

    Issue summary: View changes

    Updated issue summary.

    xjm’s picture

    Status: Needs review » Reviewed & tested by the community
    Issue tags: -Novice, -Needs manual testing, -status report

    Alright, based on @ryanissamson's testing, I think this is RTBC. I also updated the issue summary. (It's not possibly to test this reliably with an automated test, as indicated in the summary.) Thanks everyone!

    xjm’s picture

    Assigned: ryanissamson » Unassigned
    catch’s picture

    Status: Reviewed & tested by the community » Needs work
    +++ b/core/includes/file.incundefined
    @@ -2108,6 +2108,23 @@ function file_get_mimetype($uri, $mapping = NULL) {
     /**
    + * Determines if a directory is writable by the web server.
    + *
    + * In order to be able to write files within the directory, the directory
    + * itself must be writable, and it must also have the executable bit set. This
    + * helper function checks both at the same time.
    + *
    + * @param $uri
    + *   A URI or pathname pointing to the directory that will be checked.
    + *
    + * @return
    + *   TRUE if the directory is writable and executable; FALSE otherwise.
    + */
    +function file_directory_is_writable($uri) {
    +  return is_writable($uri) && drupal_is_executable($uri);
    +}
    

    drupal_realpath() doesn't work for remote stream wrappers. That's now documented for drupal_realpath() and it's even marked as deprecated.

    So if we have no choice other than to use it, we need a similar comment here, so no-one ever tries to use this for remote stream wrappers. Same for the other helper functions added. To an extent I'd consider actually dropping the helpers here and just inlining these checks, since we actively want to discourage people from using these in 99% of cases.

    12 days to next Drupal core point release.

    xjm’s picture

    Okay I was totally confused by #43 until I realized it actually refers to this function (not the one quoted above):

    +++ b/core/includes/file.incundefined
    @@ -2261,6 +2278,27 @@ function drupal_dirname($uri) {
    + * Determines if a file or directory is executable.
    + *
    + * PHP's is_executable() does not fully support stream wrappers, so this
    + * function fills that gap.
    + *
    + * @param $uri
    + *   A URI or pathname pointing to the file or directory that will be checked.
    + *
    + * @return
    + *   TRUE if the file or directory is executable; FALSE otherwise.
    + *
    + * @see is_executable()
    + * @ingroup php_wrappers
    + */
    +function drupal_is_executable($uri) {
    +  // By converting the URI to a normal path using drupal_realpath(), we can
    +  // correctly handle both stream wrappers and normal paths.
    +  return is_executable(drupal_realpath($uri));
    +}
    

    I read the doc for drupal_realpath(); unfortunately, it doesn't seem to indicate what one is supposed to use instead:

    This function was originally written to ease the conversion of 6.x code to use 7.x stream wrappers. However, it assumes that every URI may be resolved to an absolute local filesystem path, and this assumption fails when stream wrappers are used to support remote file storage. Remote stream wrappers may implement the realpath method by always returning FALSE. The use of drupal_realpath() is discouraged, and is slowly being removed from core functions where possible.

    Only use this function if you know that the stream wrapper in the URI uses the local file system, and you need to pass an absolute path to a function that is incompatible with stream URIs.

    wooody’s picture

    Hi , i copy my site to new domain , and i made database and import the sql file.

    everything work fine , but i can't install any new module.

    this message

        The specified file temporary://file3x433g could not be copied, because the destination directory is not properly configured. This may be caused by a problem with file or directory permissions. More information is available in the system log.
        http://ftp.drupal.org/files/projects/xmlsitemap-7.x-2.0-rc1.zip could not be saved to temporary://update-cache-a3b7b25e/xmlsitemap-7.x-2.0-rc1.zip.
        Unable to retrieve Drupal project from http://ftp.drupal.org/files/projects/xmlsitemap-7.x-2.0-rc1.zip.
    

    And in Status report i saw file system green
    File system Writable (public download method)

    Any help.

    catch’s picture

    OnkelTem’s picture

    Drupal doesn't support ACLs.
    One solution was provided here 5 months ago and didn't get enough feedback. It is marked 'duplicate' by @catch for now. No other solution I seen, and it seems that nobody actually using ACLs. Ok, I don't care anymore about this.

    orlitzky’s picture

    The patch here doesn't fix #1333390: file_prepare_directory() / is_writable() on Linux don't support ACLs. PHP's is_writable() returns false when the directory is writable, so the file_directory_is_writable() function introduced here still fails since it calls is_writable().

    The patch that OnkelTem posted in #1333390 does fix it; and as a side effect, fixes this, because it actually writes the file. You can't do that if the directory isn't executable. (See the comments in #4)

    This is rather important to me, as I'm attempting to migrate a large number of Drupal sites to a server that uses ACLs. I'll have to manually patch every install after every update until this is fixed.

    paularmand’s picture

    Agreeing on #48: I also have ACLS set on the files directory, and this is not a marginal phenomenon. I agree with mjorlitzky: testing by writing to file (and handling errors in the test) is at this moment our best possible option.

    xjm’s picture

    Issue tags: +Needs manual testing

    I re-closed #1333390: file_prepare_directory() / is_writable() on Linux don't support ACLs as a duplicate. Let's broaden the scope of this issue to include that bug, and update the patch here to use the technique from that issue:

    The patch at #944582 fixes the executability test by introducing a wrapper around is_executable() called drupal_is_executable(). However, it also adds a wrapper, file_directory_is_writable(), around is_writable(), which calls both is_writable() and the new drupal_is_executable().

    The latter is incorrect; the only writability test that has any chance of working is the one in this issue: you write a file. If we replace the file_directory_is_writable() in that issue with OnkelTem's implementation of drupal_is_writable() from this issue, it should fix both.

    This is the patch we should incorporate into #31:
    http://drupal.org/files/linux-acl-drupal-is-writable-d7.9.patch

    We also still need to address catch's feedback from #43.

    orlitzky’s picture

    Thanks, this seems like the most sensible solution to me.

    OnkelTem’s picture

    agreed

    David_Rothstein’s picture

    I don't think I understand what's going on here but maybe my brain is fuzzy.

    The issue summary in #1333390: file_prepare_directory() / is_writable() on Linux don't support ACLs says that the only problem with ACLs comes when is_writable() is called on a stream. My patch above dealt with a similar issue for is_executable(); the solution I tried was to convert to a non-stream using drupal_realpath().

    @catch pointed out in #43 that this function is deprecated (because it doesn't work on remote stream wrappers), but (as he also pointed out) that doesn't mean we can't use it or something like it here... we just have to be careful to make sure we're only using it on local files (which I believe is the case), and document what we're doing rather than trying to make generically reusable functions.

    So if we do what @catch suggested, and additionally do it for is_writable() - rather than just for is_executable() - won't that solve all the problems at once, in a relatively simple way?

    I really am not excited about any solution that resorts to writing "test" files to determine writability. I believe we do that in one place in Drupal currently (when writing settings.php in the installer) but that's a very special case, and the resulting code to deal with all the edge cases involved with that is a bit of a mess. I would not like to start doing that everwhere in Drupal except as an absolute last resort.

    OnkelTem’s picture

    @David_Rothstein

    Would you clarify your proposition via providing alternative patch which we can test on ACL-enabled setups?

    orlitzky’s picture

    The currently writability test just looks at some filesystem bits. This does not take into account,

    • Filesystem ACLs
    • SELinux
    • Mount options

    Therefore it does not actually test writability. I agree that writing a test file is ugly, but there's no better way to do it (if someone can think of one, by all means, implement it).

    David_Rothstein’s picture

    Status: Needs work » Needs review
    FileSize
    8.36 KB
    PASSED: [[SimpleTest]]: [MySQL] 36,286 pass(es). View

    Sure, here's a patch along the lines of what I had in mind.

    It would need more work to be a real patch, but I think this is probably good enough to test. The essential change is from this:

    +  return is_writable($uri) && drupal_is_executable($uri);
    

    to this:

    +  return is_writable(drupal_realpath($uri)) && drupal_is_executable($uri);
    

    ***

    @mjorlitzky: So is the issue summary at #1333390: file_prepare_directory() / is_writable() on Linux don't support ACLs incorrect, then? In any case, seems like this is worth testing.

    orlitzky’s picture

    I'm not even sure anymore...

    All of our sites where this is a real issue are running 7.12 and the patch won't apply that far back. For what it's worth, I set up an 8.x-dev installation on my local machine and IMCE (which calls file_prepare_directory) seems to work with the patch.

    xjm’s picture

    @mjorlitzky, if the patch is accepted, it will also be backported to Drupal 7. We can probably create a "draft" D7 version for testing as well.

    xjm’s picture

    Like these. The first applies to 7.x HEAD, the second to 7.12.

    orlitzky’s picture

    Thank you, I'll try to move a few sites to our new server later tonight and they'll need to be patched. I'll let you know how it goes.

    orlitzky’s picture

    Well, I've moved a 7.12 site, patched, and the issue with IMCE is fixed. Thanks!

    OnkelTem’s picture

    I confirm that patch works on clean Drupal 7.12 on ACL-enabled setup.
    I haven't tested on remote file-systems though, which is actually not very important in most setups.

    Both approaches - test-writing and with using drupal_realpath() have some drawbacks, but I personally like this solution more, despite this warning in docs:

    The use of drupal_realpath() is discouraged, and is slowly being removed from core functions where possible.

    I hope we will find another solution until drupal_realpath() is removed from core.

    xjm, David_Rothstein - thank you!

    maxterner23’s picture

    I got the same problem and resolved by making small modification as #36 and #28 suggested in the comment.

    chmod 777 -f sites/default/files/js

    chmod 777 -f sites/default/files/css

    and it resolves the issue

    anarcat’s picture

    Status: Needs review » Reviewed & tested by the community
    Issue tags: +DrupalWTF

    I was able to apply http://drupal.org/files/d7-944582-59-do-not-test.patch to Drupal 7.15 and it works.

    I think this is a major WTF for users on shared hosting that use ACLs. Quite hard to diagnose too!

    I wonder if D6 is affected... probably not, because it doesn't use stream wrappers.

    webchick’s picture

    Status: Reviewed & tested by the community » Needs work

    Hm. I tried to duplicate this error using the (well-written, thanks!) reproduction instructions from the OP in 8.x and was unable to do so. :( Upon submitting the performance form, I got just a simple "The configuration options have been saved." message.

    Also, the patch in #56 no longer applies, so needs work for that.

    Could someone please provide instructions on how to cause this error on D8? Or is it a D7 only issue?

    OnkelTem’s picture

    @anarcat

    Yes, D6 is affected. At least I remember for sure that I have been creating ugly 'write-test' patch for it.

    wrg20’s picture

    I recently installed upgrades/security fixes for feeds, workbench moderation, pathologic and fontyourface. I got the same error "The specified file temporary://******** could not be copied...". My status report looked fine.

    I fixed the problem by implementing #36 and #63.

    orlitzky’s picture

    I tried installing D8 but I get a symfony error, so... unknown.

    The patch in #59 still works for D7, can we have that reviewed/tested? The issue that we ran into was that if an ACL allowed read/write access to the (otherwise unreadable) sites/default/files directory, IMCE would claim that it couldn't write there and fail.

    orlitzky’s picture

    PIng. Another release I'm patching manually to get Drupal to work at all with ACLs.

    orlitzky’s picture

    FileSize
    8.03 KB
    FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch issue_944582-drupal-7.18.patch. Unable to apply patch. See the log in the details link for more information. View

    And again. Here's a patch rebased for 7.18.

    orlitzky’s picture

    And again. The patch from comment #70 still applies cleanly to 7.19.

    YesCT’s picture

    Version: 8.x-dev » 7.x-dev
    Status: Needs work » Needs review
    Gábor Hojtsy’s picture

    Yes, in fact #1885510: Can't install on some systems (like windows) due to folder permissions (executable check not needed) in fact deal with *removing* the directory executability check because it was bugos on Windows. Looks like these two issues contradict each other?

    YesCT’s picture

    Yes. In those issues, we used the rational that the files dir was not tested for being executable for a reason to take out the executable checking. Another thought there was that trouble with a non-executable directory was so rare, checking was not needed.

    So, I'm wondering if people who are familiar with this issue, can provide some guidance and an update:

    1) How common is the problem with D7 (or D6) that someone installing or running a site will have incorrect permissions that need to be fixed? Here and in those other issues, to test, the execute permission was deliberately removed. Aside from that, does the situation happen?

    2) checkout the approach in #1886164: Install in different language requirement warns translations dir not executable, when the dir is executable on some systems. That tries to keep the checks for executable in, but make it work for windows by checking if whatever/. is readable since the is_executable() function does not work for all systems. How is that different from the approach here?

    orlitzky’s picture

    Another ticket, #1333390: file_prepare_directory() / is_writable() on Linux don't support ACLs, was merged into this one and the patch here was made to solve both problems. Half of that solution somewhat conflicts with #1885510: Can't install on some systems (like windows) due to folder permissions (executable check not needed), it seems, and the other half (the ACL fixes) should not.

    One half of the fix, from #1333390, just makes the writable/executable check work in the presence of ACLs. It does this by adding new functions file_directory_is_writable and drupal_is_executable. This has nothing to do with #1885510 and should be fine to apply anyway.

    The other half from this ticket adds the executable check to the installer, and may conflict with #1885510. This is visible in the patch by the change,

    diff --git a/includes/install.core.inc b/includes/install.core.inc
    index 9805e1c..854c597 100644
    --- a/includes/install.core.inc
    +++ b/includes/install.core.inc
    @@ -1592,7 +1592,7 @@ function install_check_requirements($install_state) {
         // Otherwise, if settings.php does not exist yet, we can try to copy
         // default.settings.php to create it.
         elseif (!$exists) {
    -      $copied = drupal_verify_install_file($conf_path, FILE_EXIST|FILE_WRITABLE, 'dir') && @copy($default_settings_file, $settings_file);
    +      $copied = drupal_verify_install_file($conf_path, FILE_EXIST|FILE_WRITABLE|FILE_EXECUTABLE, 'dir') && @copy($default_settings_file, $settings_file);
           if ($copied) {
             // If the new settings file has the same owner as default.settings.php,
             // this means default.settings.php is owned by the webserver user.
    

    I personally would be happy without the executable check at install time. My primary concern is that the unpatched writable/executable checks fail in the presence of ACLs. This breaks IMCE, and all of our websites.

    Gábor Hojtsy’s picture

    Status: Needs review » Needs work
    Issue tags: +D8MI

    #1885510: Can't install on some systems (like windows) due to folder permissions (executable check not needed) got committed, so that would need to be taken into account for any further patch updates here

    orlitzky’s picture

    If you want the "executable" check to work on both Windows and sane operating systems, you need to forget the is_executable() nonsense, and create e.g. drupal_is_traversable() instead.

    On Windows, that can just return true, or delete My Photos, or whatever -- I don't care. Everywhere else it should do the drupal_is_executable() check.

    In any case, it was clearly a mistake to merge these two issues. Can we please create a separate issue for fixing the existing permissions checks which make drupal unusable? This has been a huge waste of time for something so basic as "can I create a file in this directory".

    David_Rothstein’s picture

    Well, there's no reason to make the installer inconsistent with everything else, so that doesn't seem like a meaningful way to split things to me. But if you think #1333390: file_prepare_directory() / is_writable() on Linux don't support ACLs can be solved without running into the issues here you should consider reopening that one.

    As far as I can tell, though, this is all pretty crazy, and #1885510: Can't install on some systems (like windows) due to folder permissions (executable check not needed) at most just worked around the problem? If it's really true that Windows will fail if the directories are (forced to be) executable and Linux will fail if they aren't, that is a tough one to solve :) I certainly didn't realize that before.

    orlitzky’s picture

    I just created #1899126: Add wrappers to fix permission checks with a new patch that doesn't touch the installer.

    We don't really care if the directory is executable, we care that its traversable. On everything else, you just check the execute bit. On Windows, that doesn't work -- so one possible fix is to just traverse the damn thing and see if it works. There's a note about this in the PHP is_executable() documentation.

    Cottser’s picture

    Version: 7.x-dev » 8.x-dev

    This should go back to 8.x at least…

    YesCT’s picture

    adding issue summary initiative tag to encorporate what we have found out about the related issues (wrapper for permission check, can't install on windows) and to try and write out some remaining tasks and next steps here.

    kurt.iverson’s picture

    This worked for me. Thanks!

    the_phi’s picture

    Since #1987262: [Installation Error] Symfony: "The service definition 'request' does not exist." is related: any recommendations for us there? Any input how to proceed best?

    jair’s picture

    Issue tags: +Needs reroll
    pratik60’s picture

    On changing sudo chmod 766 ./sites/default/files, I get a white screen of death on 'admin/reports'.

    On checking the logs, I got this

    'PHP Fatal error: Call to undefined function user_access() in /var/www/d8/core/includes/menu.inc on line 599'

    On further checking, I found on https://drupal.org/node/2049309, that user_access() got converted to a method on the user/accountInterface. On the page, the description states that '
    The user_access() method was directly replaced by AccountInterface::hasPermission()'

    Hence the error!

    pratik60’s picture

    Issue summary: View changes

    Updated issue summary.

    mkapst’s picture

    #16 worked for me. Cheers to Christopher James Francis Rodgers!

    XTCHost’s picture

    I just fixed my ./sites/default/files directory permission with from the site domain or default folder, whichever you use

    chown -R user:user files

    user being the CPanel username on linux server

    arkestra’s picture

    Status: Needs work » Needs review

    70: issue_944582-drupal-7.18.patch queued for re-testing.

    Status: Needs review » Needs work

    The last submitted patch, 70: issue_944582-drupal-7.18.patch, failed testing.

    The last submitted patch, 70: issue_944582-drupal-7.18.patch, failed testing.

    superspring’s picture

    Status: Needs work » Needs review
    FileSize
    10.43 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 67,007 pass(es). View

    Rerolled

    johnish@gmail.com’s picture

    Assigned: Unassigned » johnish@gmail.com

    I'm going to try and reroll this.

    johnish@gmail.com’s picture

    Issue tags: -Needs reroll
    FileSize
    10.4 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,787 pass(es). View

    I rerolled this.

    johnish@gmail.com’s picture

    Assigned: johnish@gmail.com » Unassigned
    mgifford’s picture

    FileSize
    10.32 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,921 pass(es). View

    Just a reroll...

    YesCT’s picture

    Issue summary: View changes
    Issue tags: +Needs reroll
    YesCT’s picture

    Status: Needs review » Needs work
    adci_contributor’s picture

    Status: Needs work » Needs review
    FileSize
    9.93 KB
    FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,714 pass(es), 87 fail(s), and 15 exception(s). View

    Trying to reroll

    Status: Needs review » Needs work

    The last submitted patch, 98: is_exec_writ-944582-98.patch, failed testing.

    ankitgarg’s picture

    Status: Needs work » Needs review
    FileSize
    9.93 KB
    FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,693 pass(es), 89 fail(s), and 15 exception(s). View

    Rerolled #95.

    Status: Needs review » Needs work

    The last submitted patch, 100: 944582-is_exec_writ-100-8.0_0.patch, failed testing.

    alvar0hurtad0’s picture

    Issue tags: -Needs reroll +SprintWeekend2015
    FileSize
    9.19 KB
    FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,775 pass(es), 87 fail(s), and 15 exception(s). View

    rerolled

    YesCT’s picture

    Status: Needs work » Needs review

    changing status to needs review so the testbot will run on the patch.

    Status: Needs review » Needs work

    The last submitted patch, 102: sites_default_files-944582-102.patch, failed testing.

    pbuyle’s picture

    Coming from https://www.drupal.org/node/2018947

    +++ b/core/includes/file.inc
    @@ -431,6 +431,46 @@ function file_prepare_directory(&$directory, $options = FILE_MODIFY_PERMISSIONS)
    + * Determines if a directory is writable by the web server.
    + *
    + * In order to be able to write files within the directory, the directory
    + * itself must be writable, and it must also have the executable bit set. This
    + * helper function checks both at the same time.
    + *
    + * @param $uri
    + *   A URI or pathname pointing to the directory that will be checked.
    + *
    + * @return
    + *   TRUE if the directory is writable and executable; FALSE otherwise.
    + */
    +function file_directory_is_writable($uri) {
    +  // By converting the URI to a normal path using drupal_realpath(), we can
    +  // correctly handle both stream wrappers and normal paths.
    +  return is_writable(drupal_realpath($uri)) && drupal_is_executable($uri);
    +}
    +
    

    This would not properly support remote filesystem exposed as stream (eg. Amazon S3) as drupal_realpath() only support local URI.

    The proper way would be something like

    function file_directory_is_writable($uri) {
      // By converting the URI to a normal path using drupal_realpath(), we can
      // correctly handle both stream wrappers and normal paths.
      $realpath =  drupal_realpath($uri);
      return is_writable($realpath ? $realpath : $uri) && drupal_is_executable($uri);
    }
    
    pbuyle’s picture

    A bit more information on the issue. Everything was working fine when we were using NFS v3, we started to have issue once switching to NFS v4.

    When using NFS v4, all the files are shown as owned by "nobody:nogroup" with the rwx for both the user and the group, and r-x for others. But all users are actually able to write all files.

    Using Drupal 7.42, we can observe different results when using is_writable() on a local file path vs. the path to the same file using the public:// stream wrappe: is_writable(drupal_realpath('public://styles')) returns TRUE, while is_writable('public://styles') returns FALSE.

    Using the patch in #70 fixes to issue for us (but with the same remote file system issue as reported in #105.

    OnkelTem’s picture

    For those still seeking solution for Drupal 7 — patch from #70 (for 7.18) still applies fine on 7.43.
    Without it Linux ACL won't work.

    Version: 8.0.x-dev » 8.1.x-dev

    Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

    Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

    serundeputy’s picture

    Issue summary: View changes
    mvc’s picture

    Status: Needs work » Needs review
    FileSize
    12.1 KB

    Reroll of #102 for 8.2.x-dev, plus added the wrapper to a few missing instances of is_writable() and is_executable().

    @pbuyle I didn't extend this to cover Amazon S3 since I need the patch only for NFSv4 & extended ACL's but wrapping all calls with file_directory_is_writable() should at least make fixing that issue simpler.

    Status: Needs review » Needs work

    The last submitted patch, 110: sites_default_files-944582-110.patch, failed testing.

    Version: 8.1.x-dev » 8.2.x-dev

    Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

    Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

    rkcreation’s picture

    Hi

    #70 works for me (Drupal 7.51).

    I had a weird issue : drupal in a docker container (derived from official php container) doesn't let me upload files with the "cck" file field. However, simple form API' file field works well (eg. in appearance > settings > logo field).
    After investigating, I found that was php is_writable which returned a false false in file_prepare_directory (includes/file.inc), while my directory was in fact writable : if I bypass is_writable check, upload works like a charm :-) even in the case of the directory doesn't exist. I think it's related to the docker mounted volume and the way php handles uri in is_writable, but with drupal_realpath, is_writable' return is correct.

    Could we hope a patch in a future 7.x release ?

    Thanks anyway for the patch !

    FrenchBarbu’s picture

    Hi

    Got the exact same issue on D8 and Docker : is_writable and URI return false. If I comment this part everything is ok so the directory is writable. Any clues ?

    jproctor’s picture

    Status: Needs work » Needs review
    FileSize
    15.34 KB

    Reroll of #110 for current 8.2.x-dev, plus added the wrapper to a couple more occurrences of is_writable().

    This doesn't solve the similar-but-unrelated problem I was having and it fails testing for me, but my rig has other problems so I figured I'd at least get the patch applying cleanly in case there's value in moving it forward.

    Status: Needs review » Needs work

    The last submitted patch, 115: sites_default_files-944582-115.patch, failed testing.

    Version: 8.2.x-dev » 8.3.x-dev

    Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

    Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.