Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
After writing changes in existing config, Drupal try to modify it's chmod. Not sure this is right, because, if Drupal successfully write changes to config, nothing else it should do.
Problem source
I install Drupal with 'drush si' in command line with my user: 'andyceo'. After that, all active configs have the chmod: 'rw r r' and owner=andyceo and group=andyceo.
I use ACL to allow both 'www-data' (apache2 user in ubuntu) and 'andyceo' change Drupal's 'files' folder:
$ getfacl files/ # file: files/ # owner: andyceo # group: andyceo user::rwx user:www-data:rwx user:andyceo:rwx group::rwx mask::rwx other::r-x default:user::rwx default:user:www-data:rwx default:user:andyceo:rwx default:group::rwx default:mask::rwx default:other::rwx
Comment | File | Size | Author |
---|---|---|---|
#23 | 2232051-23.patch | 709 bytes | andypost |
Comments
Comment #1
andypostComment #2
moshe weitzman CreditAttribution: moshe weitzman commentedThe File API always does a chmod after updating any Drupal file. It has always been that way. I don't know if it is a good idea or not. I personally would be fine with dropping it ... Note that you can control what the chmod does using new(ish) settings in settings.php
Comment #3
andyceo CreditAttribution: andyceo commentedIn Linux, you can not execute chmod on files you don't own, even files has 0777 permissions. And even you add the user that try to execute chmod to the group of the file.
Further info is available here: http://unix.stackexchange.com/questions/75893/can-i-allow-users-to-chmod...
So, if we want a simple solution for ability to work fine both with drush in console and with Drupal web interface without any annoying warnings in dblog, just let get this in :)
Comment #4
andypostunneccessary_chmod.patch queued for re-testing.
Comment #6
tayzlor CreditAttribution: tayzlor commentedRe-rolling last failing patch
Comment #21
quietone CreditAttribution: quietone at PreviousNext commentedThere has been no activity here for 8 years.
Is this issue still a problem?
Since we need more information to move forward with this issue, I am setting the status at Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #22
andypostI still think this
chmod()
is useless otherwise it needs comment explaining reason behind itTechnically setting 644 permission on newly written file could have security consideration but
ensureStorage()
already used to care about permissions on folder and writing.htaccess
(which may not work if config stored in web-root)Comment #23
andypostAlso no reason to call
getFilePath()
in exception because it already calledComment #24
andypostMoreover there's https://www.php.net/manual/en/function.umask.php which allows to set default permissions for every file to be written within request. FleSystem's chmod is very resource intensive operation and pointless in that case
Thinking a bit more - since PHP 8.1 acl works without issues so core could start to care about it
It needs follow-up to explore other places of core according to comment #2
Comment #25
smustgrave CreditAttribution: smustgrave at Mobomo commentedShould this go back to needs work since it needs a follow up or can it be RTBC
Comment #26
andypostFollow-up filed #3298497: Use umask() instead of always making chmod() after write
Comment #27
smustgrave CreditAttribution: smustgrave at Mobomo commentedThanks I'll mark RTBC then
Comment #29
andypostre-queued
Comment #30
alexpottThe essential point of #2 has not been answered... I'm pretty sure we added the chmod to duplicate what's being done \Drupal\Core\File\FileSystem::move() in.
Here are some thoughts that try to answer that...
Therefore I think making this change is fine.
Committed and pushed 581f751582 to 10.1.x and ed275b385e to 10.0.x and 3959be8d83 to 9.5.x. Thanks!
I'm going to treat this as a task and therefore only backport to 9.5.x.