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.howdy.
this is an edge case caused by human error.
private_upload creates the private directory, .htaccess, etc. when the user visits admin/settings/private_upload the first time after enabling the module.
however, it's possible to attempt to upload a private file even if the settings page has never been accessed and the module is not yet configured
in this case, we end up with a file named private - attempting to move a file into a non-existent directory results in a file rename. now it's impossible to configure private_upload correctly without manual intervention.
private_upload could be smarter about this. for example it could:
- create the directory, htaccess, etc. on install
- set a variable when it is successfully configured, and disable the private checkbox until that variable is set
there are probably other good fixes. i'd be happy to take a stab at providing a patch if the maintainers would provide some feedback on what solution they'd prefer.
thanks!
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | file_move-error-if-no-private-directory-1104304-5.patch | 1.24 KB | firebus |
| #5 | disable-private_upload-unless-configured-1104304-5.patch | 2.39 KB | firebus |
Comments
Comment #1
John Carbone CreditAttribution: John Carbone commentedRan into the same errors when installing this to use with Webform Protected Downloads. Definitely throws some nasty errors and requires manual db and filesystem changes if the settings page is not configured first. I agree with firebus, that should probably all happen in a hook_install and have hook_requirements checks or something. :)
Comment #2
firebus CreditAttribution: firebus commentedthanks for the confirmation! i was worried that my diagnosis was incorrect, and someone had manually created this bizarre situation on my server instead of private_upload. will work on a patch.
Comment #3
firebus CreditAttribution: firebus commentedComment #4
John Carbone CreditAttribution: John Carbone commentedNo problem! Let me know if I can help.
Comment #5
firebus CreditAttribution: firebus commentedokay, here are two patches against 6.x-1.x-dev that address this issue
the first patch fixes the bug in question - if the private directory does not exist, then file_move will fail with a meaningful error instead of renaming the file to 'private' and fudging things up.
the second patch is more general - it disables the private check box on the file upload form if private_upload is not configured. a new variable 'private_upload_configured' is set when the admin visits the settings page. if any of the status checks fail, this variable is set false. the patch also adds a message on module enable asking the admin to access the settings page in order to enable the module.
the second patch is kind of overkill - there are some status checks that, if they fail, you might not want to totally disable the checkbox. and maybe it makes more sense to just fix the file_move bug instead of conditionally disabling the module.