Description of the bug
The docblock of that function in file.entity.inc states:
* The "view" operation is not supported on the File Entity itself because it
* has no page callback (yet), but there is separately file_download_access()
* to check if a file is downloadable.
And accordingly the code only checks for create, update and delete operations.
However, that's not correct anymore. A few months after that code and comment was added, several additional permissions have been added, also "view" permissions.
So the comment is outdated and the access check is incomplete. Instead of trying to get File:access in sync with file_access, we decided to consolidate them to let one do the job and the other one is just a wrapper.
Expected behavior
Function access() should be up to date with actual permissions. Otherwise entity_access() would lead to unexpected results.
Additional information
- Backdrop CMS version: latest dev
Some communication happened in Zulip, so I'm posting initial concerns and findings re file_access() here. Yes, we had some trouble to understand the current code. :wink: This matters here (File::access()), because the logic gets consolidated.
- This function does not actually handle $file = NULL, but claims to. After all it returns FALSE, anyway, if $file is neither an object nor a string. Why is it optional then?
if (!$file && !in_array($op, array('view', 'download', 'update', 'delete', 'create'), TRUE)what does this check try to achieve? Shouldn't that be two different checks, or, honestly be an OR instead of an AND?// Fall back to default behaviors on view.why is "view" the only operation not handled in file_file_access? Means: no modules can hook in, no? Aren't they supposed to?@param $file... file object or file type. But that makes no sense, as actually the 'type' isn't considered when creating files: user_access('create files', $account). There's no create permission per file type.
Minor release update: - [ ] Add a conclusion suitable for blog post + release notes - [x] N/A Add Documentation label if the feature needs to be documented - [x] Add Needs change record label if the the issue contains an API change. - [x] N/A Add Needs translation label if the the issue contains string translations.
Recent comments
Check out this post re: tempstore: https://forum.backdropcms.org/forum/tempstore-table
Which database tables can I saftely empty before DB backup
Hi. I use the "[node:title] | [site:name]" tokens for the main image of news articles, blog posts, and similar publications. It's a quick and practical automatic ALT.
How to Improve SEO Performance in Backdrop CMS
I understand how tokens work, but not so much their practical use in image alt/text (maybe caption?) text. Could you give an example or two?
How to Improve SEO Performance in Backdrop CMS