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

Images fail to load if the base64-encoded path contains multiple slashes #11176

Open
morhi opened this issue Nov 26, 2024 · 4 comments · May be fixed by #11299
Open

Images fail to load if the base64-encoded path contains multiple slashes #11176

morhi opened this issue Nov 26, 2024 · 4 comments · May be fixed by #11299
Labels

Comments

@morhi
Copy link
Contributor

morhi commented Nov 26, 2024

Bug description

I encountered a special case, where images won't load, if the base64-encoded path of the generated Glide image URL contains multiple strings, like in /img/asset/YXNzZXRzLzgwMF/DvGJlci11bnMvbWEtZm90by12b24tb2Jlbl8yMDE3LmpwZw==/ma-foto-von-oben_2017.jpg.

This URL is technically correct, but Laravel somehow doesn't use the correct portion of the URL to decode the base64 part. Laravel ends up only decoding YXNzZXRzLzgwMF, but not the whole YXNzZXRzLzgwMF/DvGJlci11bnMvbWEtZm90by12b24tb2Jlbl8yMDE3LmpwZw== string. The whole string would result in the decoded string 800_über-uns/ma-foto-von-oben_2017.jpg.

I know, that Statamic converts german Umlaute to usual characters, but this came from an older project. It is only an example to demonstrate the issue with the special base64 string. The problematic base64 string can be generated from other values as well.

I made a fix in the GlideController which solves this issue (although it doesn't seem like the best solution to me):

    /**
     * Generate a manipulated image by an asset reference.
     *
     * @param  string  $ref
     *
     * @return mixed
     *
     * @throws \Exception
     */
    public function generateByAsset($encoded)
    {
        $this->validateSignature();

        $prefix = Str::ensureRight(Str::ensureLeft(request()->route()->getCompiled()->getStaticPrefix(), '/'), '/');
        $routePath = Str::ensureLeft(request()->path(), '/');
        $routeParts = explode('/', str_replace($prefix, '', $routePath));
        array_pop($routeParts);
        $encoded = implode('/', $routeParts);
        $decoded = base64_decode($encoded);

        // The string before the first slash is the container
        [$container, $path] = explode('/', $decoded, 2);

        throw_unless($container = AssetContainer::find($container), new NotFoundHttpException);

        throw_unless($asset = $container->asset($path), new NotFoundHttpException);

        return $this->createResponse($this->generateBy('asset', $asset));
    }

There is also a Laravel issue (laravel/framework#22125) which makes me think, that this is not possible to get fixed from Laravels side.

How to reproduce

  1. Clone https://github.com/statamic/statamic
  2. Create CP user
  3. Edit blueprint for Pages
  4. Add an asset field with 1 file and name it image
  5. Create a new folder from a CLI under public/assets named 800_über-uns (need to use CLI because in newer versions Statamic would convert an ü to u
  6. Upload an image to that folder called ma-foto-von-oben_2017.jpg (the naming here is only important to make sure, that the generated base64 string contains multiple /)
  7. Edit resources/views/home.antlers.html and insert this line: <img src="{{ glide:image }}" /
  8. The image cannot be loaded, because the servers returns a 404 error.

Another way is to clone https://github.com/morhi/statamic-glide-base64-path-bug

Logs

No response

Environment

Environment
Application Name: Statamic
Laravel Version: 11.31.0
PHP Version: 8.2.20
Composer Version: 2.5.8
Environment: local
Debug Mode: ENABLED
URL: localhost
Maintenance Mode: OFF
Timezone: UTC
Locale: en

Cache
Config: NOT CACHED
Events: NOT CACHED
Routes: NOT CACHED
Views: CACHED

Drivers
Broadcasting: log
Cache: file
Database: sqlite
Logs: stack / single
Mail: log
Queue: sync
Session: file

Statamic
Addons: 0
Sites: 1
Stache Watcher: Enabled (auto)
Static Caching: Disabled
Version: 5.37.0 Solo

Installation

Other (please explain)

Additional details

No response

@jasonvarga
Copy link
Member

I had ran into this issue recently and forgot to report it, so, thank you.

My gut reaction was to urlencode the base64 chunk, but looking at the Laravel thread you referenced, it seems like that probably won't work either.

There are "base64url" variants, which avoid using slashes. This example is from php.net. Maybe we can use this or something similar.

function base64url_encode($data) {
  return rtrim(strtr(base64_encode($data), '+/', '-_'), '=');
}
function base64url_decode($data) {
  return base64_decode(str_pad(strtr($data, '-_', '+/'), strlen($data) % 4, '=', STR_PAD_RIGHT));
}

@morhi
Copy link
Contributor Author

morhi commented Nov 27, 2024

Sure, no problem :)

Yea, I tried to use the encoded version %2F for the slash, but it didn't help.

I also thought about this and it seems like a doable workaround to me, although it might break some images at first, as long as the static cache is not cleared.

Would you implement these functions as a global helper (in src/Support/Str.php) or only use its logic within the necessary places (GlideUrlBuilder and GlideController)?

It also seems to me, that this code is fine as well (without the padding):

function base64url_encode($data)
{
    return rtrim(strtr(base64_encode($data), '+/', '-_'), '=');
}

function base64url_decode($data)
{
    return base64_decode(strtr($data, '-_', '+/'));
}

@jasonvarga
Copy link
Member

Could probably do Str::fromBase64Url (decode) and Str::toBase64Url (encode). This matches fromBase64/toBase64 that already exist in there.

@marcorieser
Copy link
Contributor

I had similar issues where the base64_encode produces too many slashes for an external image.

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

Successfully merging a pull request may close this issue.

4 participants