Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ImageResizer clobbers query string on URLs to resized images #812

Open
lackneets opened this issue Dec 22, 2022 · 5 comments
Open

ImageResizer clobbers query string on URLs to resized images #812

lackneets opened this issue Dec 22, 2022 · 5 comments

Comments

@lackneets
Copy link

lackneets commented Dec 22, 2022

Winter CMS Build

1.2

PHP Version

8.1

Database engine

SQLite

Plugins installed

No response

Issue description

My project is configured to store file on s3. And I set a image type column in backend list, but it cannot display correctly.

CleanShot 2022-12-22 at 18 09 41@2x

Steps to replicate

At this line, I figure out the access token (query string) which is after filename has been url-encoded as below:

example.jpg%3FX-Amz-Content-Sha256%3DUNSIGNED-PAYLOAD%26X-Amz-Algorithm%3DAWS4-HMAC-SHA256%26X-Amz-Credential%3Dxxxxxxxxxx%2F20221222%2Fap-northeast-1%2Fs3%2Faws4_request%26X-Amz-Date%3D20221222T093208Z%26X-Amz-SignedHeaders%3Dhost%26X-Amz-Expires%3D3600%26X-Amz-Signature%3Dxxxxxxxxxx

It should be:

example.jpg?X-Amz-Content-Sha256=UNSIGNED-PAYLOAD&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=xxxxxxxxxx/20221222/ap-northeast-1/s3/aws4_request&X-Amz-Date=20221222T093208Z&X-Amz-SignedHeaders=host&X-Amz-Expires=3600&X-Amz-Signature=xxxxxxxxxx

I understand a filename without url-encode is unsafe, however it should be optimized for AWS s3 file access. Perhaps to split out query string before encode it.

Workaround

No response

@LukeTowers
Copy link
Member

What does your filesystem disk configuration look like along with the configuration for your CMS file folders?

@lackneets
Copy link
Author

lackneets commented Dec 23, 2022

What does your filesystem disk configuration look like along with the configuration for your CMS file folders?

Did you checked the source code I mentioned ?
I don't think there has any relevant to my config. Anything besides this list column is correct.

Here is my plugin code:

<?php namespace My\Plugin\Models;

class Comment extends Model
{

    public $attachOne = [
        'image' => [
            'System\Models\File',
            'public' => false,
            'disk' => 's3',
        ],
    ];
}
// Tested, all got correct URL!
url($comment->image->path); // ✅
url($comment->image->getThumb(400, 400)); // ✅

But this is bad: (source)

use System\Classes\ImageResizer;

// Got wrong encoded URL
ImageResizer::filterGetUrl($comment->image, 40, 40); // 👎🏼

Furthermore, I also tested upload files to media manager. Everything is OK.

And here is how my config looks like:

<?php
// config/cms.php
return [
    'storage' => [
        'uploads' => [
            'disk' => 's3',
            'folder' => 'dev/uploads',
            'path' => 'https://my-bucket.s3.amazonaws.com/dev/uploads',
            'temporaryUrlTTL' => 3600,
        ],
        'media' => [
            'disk' => 's3',
            'folder' => 'dev/media',
            'path' => 'https://my-bucket.s3.amazonaws.com/dev/media',
        ],
        'resized' => [
            'disk' => 's3',
            'folder' => 'dev/resized',
            'path' => 'https://my-bucket.s3.amazonaws.com/dev/resized',
        ],
    ],
];
<?php
// config/filesystems.php
return [
    'default' => 's3',
    'disks' => [
        's3' => [
            'bucket' => 'my-bucket',
            'driver' => 's3',
            'endpoint' => null,
            'key' => 'AKI..................2A',
            'region' => 'ap-northeast-1',
            'secret' => 'Fxt49.............8TE',
            'stream_uploads' => false,
            'url' => 'https://my-bucket.s3.amazonaws.com/',
            'use_path_style_endpoint' => false,
        ],
    ],
];

@lackneets
Copy link
Author

Workaround

Currently I will solve this problem by adding custom list column to replace the image resize function.

CleanShot 2022-12-23 at 15 10 28@2x

class Plugin extends PluginBase
{
    public function registerListColumnTypes()
    {
        return [
            'thumbnail' => function ($value, $column, $record) {
                $image = null;
                $config = $column->config;

                // Get config options with defaults
                $width = isset($config['width']) ? $config['width'] : 50;
                $height = isset($config['height']) ? $config['height'] : 50;
                $options = isset($config['options']) ? $config['options'] : [];
                $fallback = isset($config['default']) ? $config['default'] : null;

                // Handle attachMany relationships
                if (isset($record->attachMany[$column->columnName])) {
                    $image = $value->first();

                    // Handle attachOne relationships
                } elseif (isset($record->attachOne[$column->columnName])) {
                    $image = $value;

                    // Handle absolute URLs
                } elseif (str_contains($value, '://')) {
                    $image = $value;

                    // Handle embedded data URLs
                } elseif (starts_with($value, 'data:image')) {
                    $image = $value;

                    // Assume all other values to be from the media library
                } elseif (!empty($value)) {
                    $image = MediaLibrary::url($value);
                }

                if (!$image && $fallback) {
                    $image = $fallback;
                }

                if ($image) {
                    // $imageUrl = ImageResizer::filterGetUrl($image, $width, $height, $options);
                    // 🔥 KEY POINT: replace to this:
                    $imageUrl = $value->getThumb($width, $height, $options);
                    return "<img src='$imageUrl' width='$width' height='$height' />";
                }
            },
        ];
    }
}
columns:
    # ...
    image:
        label: image
        type: thumbnail
        options:
            mode: crop

@LukeTowers
Copy link
Member

@lackneets can you try replacing the problem lines in ImageResizer.php with the following:

// Ensure that a properly encoded URL is returned
$url = \Winter\Storm\Router\UrlGenerator::buildUrl($url);

@LukeTowers LukeTowers changed the title AWS S3 file access token incorrectly encoded at List image column ImageResizer clobbers query string on URLs to resized images Feb 26, 2023
@LukeTowers LukeTowers added this to the v1.2.2 milestone Feb 26, 2023
@LukeTowers LukeTowers modified the milestones: v1.2.2, v1.2.3 May 3, 2023
@LukeTowers LukeTowers modified the milestones: v1.2.3, v1.2.4 Jul 7, 2023
@LukeTowers LukeTowers modified the milestones: v1.2.4, 1.2.5 Dec 27, 2023
@mjauvin mjauvin modified the milestones: 1.2.5, 1.2.6 Feb 18, 2024
@LukeTowers LukeTowers modified the milestones: 1.2.6, 1.2.7 Apr 25, 2024
@LukeTowers
Copy link
Member

@lackneets are you able to test my proposed fix? If it works I can merge it. Otherwise it'll have to wait until I or someone else can reproduce the original issue and test the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants