private files enabled, hook_file_download useless.

file attachments do not work when submitting an issue.

Here's my situation is slightly complex. I have a node type called a reel. It stores a video file, but only administrators are allowed to view the node directly, no users are.

I have another node type called a 'pitch'. Some users are allowed to view some pitches. And pitches refer to reels. If the user is allowed to view a pitch, the she is allowed to view the videos referred to by that pitch. And she is allowed to view the video files, but not the reel node that contains them.

The logic is not very complex. I have a hook_file_download that takes care of it. However the recent change breaks it.

One question here is: is hook_file_download a function that one and only one module gets to implement? If you have upload.module installed, no other module can implement hook_file_download. Well to be accurate: they can implement, but the most they can do is deny files that upload_file_download would have allowed.

It's pretty clear to me that either upload_file_download has to change, or file_download (in file.inc). The logic there is that if any module returns -1, the file cannot be downloaded, even if another module return valid headers. I hope you agree that this should change.

I suppose I could redo my hook_file_download logic so that it denies access instead of allowing it.

I'd like to point out one thing before someone closes this issue... The other drupal access systems, namely hook_access and the node_access table, function by allowing access to things and not by denying it. That is, if they can't give you permission, they leave room for some other module to do so. It works nicely because if hook_access does not return true, the node_access table might provide the grant, and vice-versa. Meanwhile, upload_file_download is the opposite. It says no and the answer is final.

You may be thinking that hook_access also can return false, thereby explicitly denying access to a node. True it can, but now that node_access exists, returning false from hook_access is bad form. It prevents a grant that might be in the node_access table from actually working. If I submitted a bug "module X should not return false in hook_access, because it prevents my node_access grant from working", I suspect the bug would be fixed.

I don't understand the resistance to simply not returning -1. In my mind the permission "view uploaded files" should mean "view files uploaded by the upload module" but it really means "view any files stored on the system regardless of how they got there". I don't think it should mean that.

Finally, I apologize if the tone of any of my earlier comments was out of line (notably the name of my patch file). It took me a while to figure out what was causing my problem and that put me in a bad mood.

Rant over.

I was wrong in my first statement above. I can't re-jigger my logic in hook_file_download to deny access. The problem is the files I want to show the user are in a 'reel' node. I never want users to see the node itself, just the video files stored there.

uplaod_file_download includes a check that the user can see the node to which the file belongs. So even if I give all users "view uploaded files" permission, upload_file_download will return -1 because they can't view the node.

I'll use my patched upload.module.