Private file image styles are not accessible after entering password.

Hello,

Here is a patch that should solve the problem, but I think it's should be improved.

The problem was from the $uri which is wrong when it's an image generated from an image style so $files was void and so the hook returned -1 to prevent access.

Please test because if the return -1 is not returned when the user shouldnot have access. the user will have access to your files.

what do you mean by uri is wrong when the image is a derivative of private one ? doesn't it have the private streamwrapper in the uri ?

also i think the patch is making all the image derivatives bypass the protection which may not be really what all users need. i think the image derivatives should be accessible only when the original image is accessible too

also 'styles' is not a directory name we can relay, in case of privatefiles/tshirts/styles/pdf we can bypass the protection by error

Ok,

About the $uri, in the case of the file, it matches the uri stored in the table file_managed so file_load_multiple returns the file. In the case of a derivative, $uri doesn't match the uri stored in the table so it returns nothing so you don't go in the loops and the hook return -1.

About the patch, I would prefer to have the fid to load the file, but it seems the hook_file_download only have uri in parameter. And I don't know if I could get the fid starting from a derivative.

Hello,

I think I found the clean solution in the image.module file line 280 where its implements the hook_file_download.

Thanks for the review.

Hi ishworthapaliya,
Did the patch in #5 work well for you ?
Thanks

I have remade a patch because the last one was not applyable anymore.

Thanks ishworthapaliya for testing.

line 702 : we can even check if it starts with private://styles i guess ?
lines 702 to 712, i think we need a helper function that gets an image derivative uri and returns the original file uri, it would be easier to read :)

Hum, I wrote the patch two months ago, so line 702 I need to check.

I remember I took exactly what the module image in core used to deal with the private image. As said in #5, so if you want I put it in an helper function, ok as you want.

line 702 : no, $path starts with style, ex : styles/medium/private/field/image/toto.jpg

Now, you have the choice, a patch with an helper function or not :).

we're close :) and as we'e close we find non issued bugs yet !

1. +++ b/protected_node.module
   @@ -693,11 +693,29 @@
   +  if (strpos($path, 'styles/') === 0) {
   

   There is something we miss here, we should only do this for private files (and their derivatives if the file is of image type) if they belong only to protected nodes. i think a way to do this is checking is $uri starts with private:// (this should wrapp all the code of this hook i think) and for image derivatives that should start with private://styles

2. +++ b/protected_node.module
   @@ -736,10 +754,28 @@
   + * Helper function used in hook_file_download to return the original uri from
   

   you should do it in two paragraphs with a blank line (https://drupal.org/coding-standards/docs#general) and the @see tag (https://drupal.org/coding-standards/docs#see) for documentation here refering the function name protected_node_file_download

1) This hook is only triggered by private files. https://api.drupal.org/api/drupal/modules!system!system.api.php/function...

I test putting a dpm at the very beginning of the function. with a node with two image field, one private, one public, only the private field triggers the hook.

And if I unprotect the node, I don't pass in the hook for both public or private image.

2) Is it better now ?

this is now merged.
Thank you