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.
(I guess this is rather an issue for Entity module, but I'll let you folks do the proper routing)
The JSON and XML outputs of a node show empty entries for file fields, and only the 'tag' entry for image fields.
Comment | File | Size | Author |
---|---|---|---|
#10 | 1136356-entity-file-access.patch | 1.62 KB | klausi |
#8 | 1136356-entity-file-access.patch | 1.47 KB | klausi |
#6 | 1136356-entity-file-access.patch | 1.44 KB | klausi |
#4 | 1136356-entity-file-access.patch | 1.4 KB | klausi |
Comments
Comment #1
Loocor CreditAttribution: Loocor commentedMark!
Comment #2
fagohm, in my test it did output something. E.g. this is the xml output I get of a node with a file and an image attached:
It looks like the "http://mysite.com/file/3" path doesn't work properly though.
Comment #3
fagoComment #4
klausiWe need to fix the file access handling in the entity API module, thanks fago for the idea. Patch attached.
This should fix the issue with file fields in RESTWS, too.
Comment #5
fago1. Shouldn't we check $op and only give green lights in case $op == 'view'? I don't think the hook does deal with editing. Indeed, I think files are never edited but replaced by new ones. So returning FALSE should be fine then.
2. Let's make use of existing helpers, i.e. module_invoke().
Comment #6
klausiNew patch fixes that issues.
Comment #7
fagopatch fails tests, as $entity may be unset.
Trying to get property of non-object Notice callbacks.inc 615 entity_metadata_file_access() Exception
Trying to get property of non-object Notice callbacks.inc 615 entity_metadata_file_access() Exception
Trying to get property of non-object Notice callbacks.inc 615 entity_metadata_file_access()
Comment #8
klausiFixed that.
Comment #9
fagohm, spotted another problem: If $account is set to another user, the function will still check access for the global user. Thus, I guess we need to return FALSE in that cases too - as we do not know access permissions for other users.
This was confusing me, as there is no $entity->uri in general, but a $file->uri. So if $entity is $file, let's call it that way.
Comment #10
klausiok, patch attached.
Comment #11
Dave ReidFYI this hook only applies if the file is private or temporary, not any other schemes.
Comment #12
fagoindeed, so probably we should bypass the hook and just return TRUE by default for public files. You can determine the scheme with file_uri_scheme().
I'm not sure how we should handle files of an arbitrary scheme though. We really cannot know whether the user has access to them, so maybe we should just return FALSE until we figure out something better.
Comment #13
fagoComment #14
Dave ReidMy solution in file_entity.module has been to alter in 'uses file_download' into hook_stream_wrappers_alter() and then check the file's uri's stream wrapper for that property, if so, run the file_download() checks.
Comment #15
klausiClosed #1273334: Image and file fields are omitted from the response as duplicate.
Comment #16
larjohn CreditAttribution: larjohn commentedDoes this issue also include the fact that the image fields have no access callback?
Comment #17
sepgil CreditAttribution: sepgil commentedToday I tested the patch (using restws) and I noticed that it also worked public files.
Why?
Well, because the foreach loop returns true if the access is restricted. Since public files aren't affected by hook_file_download, the function will skip the loop and return true.
Comment #18
klausiI agree, we can just ignore the file scheme as our callback returns TRUE if nobody returned -1 in hook_file_download().
Comment #19
klausiStatus change accident, sorry.
Comment #20
fagoIndeed, it shouldn't harm to run it for public files as well. At worst, it will return FALSE - similar to the situation now. Thus, this can be only an improvement, so I've committed #10. Thanks!
Comment #22
btopro CreditAttribution: btopro commentedNot sure where to post this so I'll leave a note that based on the way this is setup it's not possible to use this call to answer the question "can a user view files" in a generic sense. The user / node access callbacks have this capability in the form of a permission that overrides all other access checks, for consistency I think this one should as well.
File Entity thread - #1227706: Add a file entity access API
RestWS thread - #1858338: file entity can't be queried because of an inconsistency in the file entity access callback