Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Found an issue that broke images URL, example:
.../the_image.jpg&h=6ccb2c19?itok=oSXmdgZi
As you can see the "?" is after the "?".
Correct URL:
.../the_image.jpg?h=6ccb2c19&itok=oSXmdgZi
The bug is in the implementation of hook_file_url_alter:
// In case the image style uses "crop_type" effect, load the crop entity.
if ($crop_type && $crop = Crop::findCrop($file_uri, $crop_type)) {
// Found a crop for this image, append a hash of it to the URL,
// so that browsers reload the image and CDNs and proxies can be bypassed.
$shortened_hash = substr(md5(implode($crop->position()) . implode($crop->anchor())), 0, 8);
$uri .= '&h='.$shortened_hash;
}
That is adding "&h=" to the URI, if the URI do not have a "?" later another implementation of the hook can add it and broke the image.
In order to fix it we should check if there is a "?" before adding the "&".
$uri .= (strpos($uri, '?') !== FALSE ? '&' : '?').'h='.$shortened_hash;
Comment | File | Size | Author |
---|---|---|---|
#2 | crop-fix-uri-2884103-1-D8.patch | 549 bytes | juanl |
|
Comments
Comment #2
juanl CreditAttribution: juanl commentedComment #3
juanl CreditAttribution: juanl commentedComment #4
joelpittetThat seems reasonaable, the whitespace around the periods needs to be added for coding standards but that could be fixed on commit. There is probably some PHP or drupal helper methods for building a URL out but don't have them on hand so this seems reasonable solution.
Comment #5
woprrr CreditAttribution: woprrr as a volunteer and at NeoLynk commentedHi all I review it and merge it fast.
Comment #6
weri CreditAttribution: weri at Previon Plus AG commentedComment #7
weri CreditAttribution: weri at Previon Plus AG commentedComment #8
woprrr CreditAttribution: woprrr as a volunteer and at NeoLynk commentedI can't reproduce this bug in current state of Crop API module because I guess berdir patch already fixe that #2868339: Public folder check in crop_file_url_alter() is incorrect, does not work for responsive images.
Example :
Fell free to switch status of issue if your problem persist but I think all works as excepted now.