Posted by yched on April 23, 2011 at 1:00pm
10 followers
| Project: | Entity API |
| Version: | 7.x-1.x-dev |
| Component: | Core integration |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
(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.
Comments
#1
Mark!
#2
hm, 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:
<?xml version="1.0" encoding="utf-8"?>
<node><nid>3</nid><vid>3</vid><is_new></is_new><type>article</type><title>asdfdsf</title><language>und</language><url>http://mysite.com/node/3</url><edit_url>http://mysite.com/node/3/edit</edit_url><status>1</status><promote>1</promote><sticky>0</sticky><created>1294913241</created><changed>1303982320</changed><author resource="user" id="1">http://mysite.com/user/1</author><log></log><revision></revision><comment>2</comment><comment_count>1</comment_count><comment_count_new>0</comment_count_new><body><value><p>test2</p>
</value><summary><p>ha</p>
</summary><format>filtered_html</format></body><field_tags/><field_image><file resource="file" id="2">http://mysite.com/file/2</file><alt></alt></field_image><field_test>1</field_test><field_file><item><file resource="file" id="3">http://mysite.com/file/3</file></item></field_file></node>
It looks like the "http://mysite.com/file/3" path doesn't work properly though.
#3
#4
We 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.
#5
1. 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().
#6
New patch fixes that issues.
#7
patch 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()
#8
Fixed that.
#9
hm, 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.
+ $result = module_invoke($module, 'file_download', $entity->uri);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.
#10
ok, patch attached.
#11
FYI this hook only applies if the file is private or temporary, not any other schemes.
#12
indeed, 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.
#13
#14
My 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.
#15
Closed #1273334: Image and file fields are omitted from the response as duplicate.
#16
Does this issue also include the fact that the image fields have no access callback?
[file] => Array
(
[label] => File
[base table] => file_managed
[entity keys] => Array
(
[id] => fid
[label] => filename
[revision] =>
[bundle] =>
)
[static cache] =>
[fieldable] =>
[controller class] => DrupalDefaultEntityController
[field cache] => 1
[load hook] => file_load
[bundles] => Array
(
[file] => Array
(
[label] => File
[rdf_mapping] => Array
(
)
)
)
[view modes] => Array
(
[entityreference_view_widget] => Array
(
[label] => Entity Reference View Widget
[custom settings] => 1
)
[token] => Array
(
[label] => Tokens
[custom settings] =>
)
)
[translation] => Array
(
)
[schema_fields_sql] => Array
(
[base table] => Array
(
[0] => fid
[1] => uid
[2] => filename
[3] => uri
[4] => filemime
[5] => filesize
[6] => status
[7] => timestamp
)
)
[token type] => file
[plural label] => Files
[description] => Uploaded file.
[save callback] => file_save
[deletion callback] => entity_metadata_delete_file
[configuration] =>
)
#17
Today 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.
#18
I agree, we can just ignore the file scheme as our callback returns TRUE if nobody returned -1 in hook_file_download().
#19
Status change accident, sorry.
#20
Indeed, 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!
#21
Automatically closed -- issue fixed for 2 weeks with no activity.
#22
Not 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