With the image derivative token in Drupal 7.20, the query string in the image style URL is now URL-encoded and no longer works.

The problem seems to be in the hook_preprocess_image() approach. Changing the derivative URL scheme from 'http' to 'public' there seems too late, since now the 'http' URL has a query string and it gets encoded when theme_image() converts it into an absolute URL.

Files: 
CommentFileSizeAuthor
#6 broken-image-style-7.20-1926434-0.patch1.39 KBDarren Oh
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch broken-image-style-7.20-1926434-0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#4 1926434-4-broken-image-style-7.20.patch902 byteszhangtaihao
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1926434-4-broken-image-style-7.20.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#1 1926434-broken-image-style-7.20.patch588 byteszhangtaihao
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1926434-broken-image-style-7.20.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

zhangtaihao’s picture

Status:Active» Needs review
StatusFileSize
new588 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1926434-broken-image-style-7.20.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Here's a patch that gets around the problem by simply stripping the query string (since this module does not add the token anyway).

I presume if we want to follow Drupal core 7.20 we will have to do it at some point.

MiroslavBanov’s picture

The bug is reproduced. My setup is: two drupal 7.20 sites, one site adding external image from the other site to its media field. When I choose an image style for format settings of the media field, the styled image does not display, and the token part of the image path is URL-encoded, resulting in image path that ends with %3Fitok%3Dxxxxxxxx.
Successfully applied the patch in #1, which fixed my issues.

samalone’s picture

Patch #1 fixes the problem with remote images, but breaks local ones. That is, it fixes URLs like:

    public://styles/lead_photo/http/www.cdc.gov/flu/images/influenza-virus-fulltext.jpg?itok=hyHOSHnr

but breaks ones like:

    http://lbn.local/sites/default/files/styles/large_lead_photo/public/What%20I%20have%202.jpg?itok=QjirsHjP

I adapted patch #1 to only modify public URLs, but have no idea if that's right or just happens to work in my case.

zhangtaihao’s picture

StatusFileSize
new902 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1926434-4-broken-image-style-7.20.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Silly me. Okay the patch has been updated to just match the regular expression and strip token only if remote.

pstewart’s picture

Another reporduction of this bug confirmed. I was able to use patch from #4 to fix the query string problem, which allowed images which were previously working to work again. However, it also seemed to trigger further issues with 7.20 stopping image derivatives from working on all new images (it is unclear whether it was the patch itself or whether it was something else in the module, as I applied the patch against a git clone of the current tree). Using the 'image_allow_insecure_derivatives' variable has mitigated the problem for now.

I plan to roll out 7.21 over the weekend, and will confirm whether or not this works with 'image_allow_insecure_derivatives' switched off.

Darren Oh’s picture

StatusFileSize
new1.39 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch broken-image-style-7.20-1926434-0.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

I have moved processing to hook_file_url_alter() so the query string will work as intended.

Darren Oh’s picture

The test bot will not test this patch because the project currently does not pass its own tests. However, all tests passed on my local machine with this patch applied.

pianomansam’s picture

Status:Needs review» Reviewed & tested by the community

I also had this issue and patch in #6 fixed it for me. I think it's time to mark this as RTBC.

Summit’s picture

Hi, Yes commit this please!
Greetings, Martijn

danielnolde’s picture

I experienced the problem as well (http://...jpg image url via remote stream wrapper in media field whose display settings are configured to output image with imagestyle, outputted url broken), but the patch in #6 doesn't seem to help.

amaisano’s picture

Issue summary:View changes

#6 worked for me and I was using the 1.x beta4 version. Thanks!

Dave Reid’s picture

Status:Reviewed & tested by the community» Fixed

Thanks everyone, committed to 7.x-1.x.

  • Commit ce3f6ee on 7.x-1.x by Dave Reid:
    Issue #1926434 by zhangtaihao, Darren Oh: Remote image style derivatives...

The last submitted patch, 1: 1926434-broken-image-style-7.20.patch, failed testing.

The last submitted patch, 4: 1926434-4-broken-image-style-7.20.patch, failed testing.

Status:Fixed» Needs work

The last submitted patch, 6: broken-image-style-7.20-1926434-0.patch, failed testing.

Dave Reid’s picture

Status:Needs work» Fixed

  • Commit 4597c5e on 7.x-1.x by Dave Reid:
    Issue #1926434: Validate image style derivative tokens.
    

Status:Fixed» Closed (fixed)

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