Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dave Reid’s picture

Issue tags: +user pictures

Adding tag

c960657’s picture

Status: Active » Needs review
FileSize
2.64 KB
chrishaslam’s picture

My 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)

Dries’s picture

Status: Needs review » Needs work

Personally, 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!

c960657’s picture

Status: Needs work » Needs review
FileSize
7.43 KB

Reroll following the introduction of the image module with some tests added.

c960657’s picture

FileSize
9.06 KB

Updated tests.

Status: Needs review » Needs work

The last submitted patch failed testing.

c960657’s picture

Status: Needs work » Needs review
FileSize
7.4 KB

Reroll.

Status: Needs review » Needs work

The last submitted patch failed testing.

c960657’s picture

Status: Needs work » Needs review
FileSize
7.92 KB

Reroll.

Status: Needs review » Needs work

The last submitted patch failed testing.

c960657’s picture

Status: Needs work » Needs review
FileSize
7.94 KB

Status: Needs review » Needs work

The last submitted patch failed testing.

c960657’s picture

Status: Needs work » Needs review
FileSize
17.45 KB

The 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.

drewish’s picture

In image_file_download() why are you rebuilding $original_path rather than just using $uri?

c960657’s picture

$original_path is the URI of the original (non-resized) image, and $uri is the generated (resized).

I renamed the variable to $original_uri.

c960657’s picture

FileSize
17.52 KB

... forgot the updated patch.

Status: Needs review » Needs work

The last submitted patch failed testing.

c960657’s picture

Status: Needs work » Needs review
FileSize
17.57 KB

Reroll.

Status: Needs review » Needs work

The last submitted patch failed testing.

c960657’s picture

Status: Needs work » Needs review
FileSize
18.1 KB

Reroll.

Status: Needs review » Needs work

The last submitted patch failed testing.

c960657’s picture

Status: Needs work » Needs review
FileSize
18.17 KB

Reroll.

Status: Needs review » Needs work

The last submitted patch failed testing.

c960657’s picture

Status: Needs work » Needs review
FileSize
18.17 KB

Reroll.

andypost’s picture

Tested 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

+++ modules/image/image.module	29 Sep 2009 19:42:39 -0000
@@ -181,42 +181,43 @@ function image_permission() {
-          'Expires' => gmdate(DATE_RFC1123, time() + 1209600),
+          'Expires' => gmdate(DATE_RFC1123, REQUEST_TIME + 1209600),

another old fix!

I'm on crack. Are you, too?

Status: Needs review » Needs work

The last submitted patch failed testing.

andypost’s picture

need re-roll

c960657’s picture

Status: Needs work » Needs review
FileSize
17.85 KB

Reroll (some conflicts with #560780: Add Image Field to image.module were resolved).

andypost’s picture

Status: Needs review » Reviewed & tested by the community

Without 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!

andypost’s picture

it's good if we add "?[timestamp]" suffix to user picture so problem with browser cache could be solved

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Reviewed, and committed.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

nzcodarnoc’s picture

Subscribe