The property used for the file ID in file_entity_access is $file->id, but that's not the standard property name, it should be $file->fid.
I noticed this when debugging #2874781: allowed types, video files should include web files like vimeo and youtube - existing files always had a hash as the cache ID, but they should instead use the file ID as the cache ID. If anything changes about the file object between invocations of this function, access control will be checked again when it doesn't need to be. Patch forthcoming.
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | file_entity-wrong-property-2916585-3.patch | 834 bytes | cboyden |
Comments
Comment #2
cboyden commentedAlso the checks are in the wrong order. If it's OK for the $file parameter to be a string, that should be checked first. Otherwise you'll get a PHP notice about "property of non-object" on the string. Is there anyplace where a string is passed into this function? It seems like the API allows it, but all the places where this hook is invoked seem to be passing in a file object.
Comment #3
cboyden commentedHere's a patch that fixes the property name and handles the check regardless of whether a string or an object is passed to the function. I added a check for a file object that didn't have a file ID but did have a type - which is the condition when you have a newly-uploaded file object that hasn't been saved as an entity yet - and it also seems to fix #2874781: allowed types, video files should include web files like vimeo and youtube.
Comment #4
cboyden commentedComment #5
joseph.olstadnice work. Patch looks good..
Comment #8
joseph.olstadComment #9
joseph.olstadmight want to add additional validation , check to make sure $cache_id is not empty before using it.