Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
User pictures are always served as public files and does not respect the file_downloads
setting. This is a regression from #357403: Move user picture to managed files.
Comment | File | Size | Author |
---|---|---|---|
#29 | user-picture-13.patch | 17.85 KB | c960657 |
#25 | user-picture-12.patch | 18.17 KB | c960657 |
#23 | user-picture-11.patch | 18.17 KB | c960657 |
#21 | user-picture-10.patch | 18.1 KB | c960657 |
#19 | user-picture-9.patch | 17.57 KB | c960657 |
Comments
Comment #1
Dave ReidAdding tag
Comment #2
c960657 CreditAttribution: c960657 commentedComment #3
chrishaslam CreditAttribution: chrishaslam commentedMy filesystem is set to private at /home/chris/d7files
Without the patch my user pictures are not working at all with a path of:
http://dev/chris/7x//home/chris/d7files/pictures/picture-1.png
After applying the above patch the file_downloads settings is honoured and the images are correctly served privately from the home directory outside of the webroot ( /system/files/pictures)
Comment #4
Dries CreditAttribution: Dries commentedPersonally, I think we should redo the private downloads stuff.
Either way, it sounds like this patch would require some tests. Please create a couple of tests. Thanks!
Comment #5
c960657 CreditAttribution: c960657 commentedReroll following the introduction of the image module with some tests added.
Comment #6
c960657 CreditAttribution: c960657 commentedUpdated tests.
Comment #8
c960657 CreditAttribution: c960657 commentedReroll.
Comment #10
c960657 CreditAttribution: c960657 commentedReroll.
Comment #12
c960657 CreditAttribution: c960657 commentedComment #14
c960657 CreditAttribution: c960657 commentedThe patch for #517814: File API Stream Wrapper Conversion added a file_create_url() call to theme_image() as suggested in the previous patches for this issue, and that fixes the original bug reported in this issue. However, it introduced a bug in the Image module, so when that is enabled, user pictures (and AFAICT also other pictures generated by the image module) still don't work with private files.
The problem is that hook_file_download() is now passed a URI rather than a relative filepath, but image_file_download() does not reflect that. Also, image_style_path() always assumes that generated files have the public scheme.
Comment #15
drewish CreditAttribution: drewish commentedIn image_file_download() why are you rebuilding $original_path rather than just using $uri?
Comment #16
c960657 CreditAttribution: c960657 commented$original_path is the URI of the original (non-resized) image, and $uri is the generated (resized).
I renamed the variable to $original_uri.
Comment #17
c960657 CreditAttribution: c960657 commented... forgot the updated patch.
Comment #19
c960657 CreditAttribution: c960657 commentedReroll.
Comment #21
c960657 CreditAttribution: c960657 commentedReroll.
Comment #23
c960657 CreditAttribution: c960657 commentedReroll.
Comment #25
c960657 CreditAttribution: c960657 commentedReroll.
Comment #26
andypostTested with cvs-HEAD, enabled private files at /admin/config/media/file-system (Private local files served by Drupal.)
Before patch user Images are broken
after patch
Files upload and displayed fine
after rollback pach images broken again but if I change download method to Public local files served by the webserver.
Image are displayed again
so RTBC +1
another old fix!
I'm on crack. Are you, too?
Comment #28
andypostneed re-roll
Comment #29
c960657 CreditAttribution: c960657 commentedReroll (some conflicts with #560780: Add Image Field to image.module were resolved).
Comment #30
andypostWithout a patch when I try to change user picture it works only if I mark 'Delete picture' and upload new one but sometimes and old image is displayed. Switching public/private download method show different images.
With this patch everything works fine!
Comment #31
andypostit's good if we add "?[timestamp]" suffix to user picture so problem with browser cache could be solved
Comment #32
Dries CreditAttribution: Dries commentedReviewed, and committed.
Comment #34
nzcodarnoc CreditAttribution: nzcodarnoc commentedSubscribe