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

[5.x] Support glide urls with URL params #11003

Open
wants to merge 6 commits into
base: 5.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/Imaging/GlideManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,13 @@ private function getCachePathCallable()
$hashCallable = $this->getHashCallable();

return function ($path, $params) use ($hashCallable) {
$qs = Str::contains($path, '?') ? Str::after($path, '?') : null;
$path = Str::before($path, '?');

if ($qs) {
$path = Str::replaceLast('.', '-'.md5($qs).'.', $path);
}

$sourcePath = $this->getSourcePath($path);

if ($this->sourcePathPrefix) {
Expand Down
3 changes: 2 additions & 1 deletion src/Imaging/ImageGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,14 @@ private function doGenerateByUrl($url, array $params)
$this->skip_validation = true;
$this->setParams($params);

$qs = Str::contains($url, '?') ? Str::after($url, '?') : null;
$parsed = $this->parseUrl($url);

$this->server->setSource($this->guzzleSourceFilesystem($parsed['base']));
$this->server->setSourcePathPrefix('/');
$this->server->setCachePathPrefix('http');

return $this->generate($parsed['path']);
return $this->generate($parsed['path'].($qs ? '?'.$qs : ''));
}

/**
Expand Down
52 changes: 51 additions & 1 deletion tests/Imaging/ImageGeneratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,56 @@ public function it_generates_an_image_by_external_url()
Event::assertDispatchedTimes(GlideImageGenerated::class, 1);
}

#[Test]
public function it_generates_an_image_by_external_url_with_query_string()
{
Event::fake();

$cacheKey = 'url::https://example.com/foo/hoff.jpg?query=david::4dbc41d8e3ba1ccd302641e509b48768';
$this->assertNull(Glide::cacheStore()->get($cacheKey));

$this->assertCount(0, $this->generatedImagePaths());

$this->app->bind('statamic.imaging.guzzle', function () {
$file = UploadedFile::fake()->image('', 30, 60);
$contents = file_get_contents($file->getPathname());

$response = new Response(200, [], $contents);

// Glide, Flysystem, or the Guzzle adapter will try to perform the requests
// at different points to check if the file exists or to get the content
// of it. Here we'll just mock the same response multiple times.
return new Client(['handler' => new MockHandler([
$response, $response, $response,
])]);
});

// Generate the image twice to make sure it's cached.
foreach (range(1, 2) as $i) {
$path = $this->makeGenerator()->generateByUrl(
'https://example.com/foo/hoff.jpg?query=david',
$userParams = ['w' => 100, 'h' => 100]
);
}

$qsHash = md5('query=david');

// Since we can't really mock the server, we'll generate the md5 hash the same
// way it does. It will not include the fit parameter since it's not an asset.
$md5 = $this->getGlideMd5("foo/hoff-{$qsHash}.jpg", $userParams);

// While writing this test I noticed that we don't include the domain in the
// cache path, so the same file path on two different domains will conflict.
// TODO: Fix this.
$expectedPath = "http/foo/hoff-{$qsHash}.jpg/{$md5}/hoff-{$qsHash}.jpg";

$this->assertEquals($expectedPath, $path);
$this->assertCount(1, $paths = $this->generatedImagePaths());
$this->assertContains($expectedPath, $paths);
$this->assertEquals($expectedPath, Glide::cacheStore()->get($cacheKey));
Event::assertDispatchedTimes(GlideImageGenerated::class, 1);
}

#[Test]
public function the_watermark_disk_is_the_public_directory_by_default()
{
Expand Down Expand Up @@ -331,7 +381,7 @@ private function getGlideMd5($basename, $params)
{
ksort($params);

return md5($basename.'?'.http_build_query($params));
return md5($basename.(str_contains($basename, '?') ? '&' : '?').http_build_query($params));
}

private function assertLocalAdapter($adapter)
Expand Down
Loading