Skip to content

Commit f376fdd

Browse files
authored
fix: weak etag causes invalid header (#103)
1 parent 5b88ea7 commit f376fdd

File tree

5 files changed

+125
-7
lines changed

5 files changed

+125
-7
lines changed

src/Cdn/Domain/CdnFileMetadata.php

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public static function fromArray(array $data): self
3838
return new self(
3939
originalUrl: $data['originalUrl'],
4040
contentType: $data['contentType'],
41-
etag: $data['etag'],
41+
etag: self::normalizeEtag($data['etag']),
4242
expiresAt: null !== $data['expiresAt'] ? new DateTimeImmutable($data['expiresAt']) : null,
4343
);
4444
}
@@ -48,7 +48,7 @@ public function withDownloadInfo(string $contentType, ?string $etag, DateTimeImm
4848
return new self(
4949
originalUrl: $this->originalUrl,
5050
contentType: $contentType,
51-
etag: $etag,
51+
etag: self::normalizeEtag($etag),
5252
expiresAt: $expiresAt,
5353
);
5454
}
@@ -74,4 +74,24 @@ public function jsonSerialize(): array
7474
'expiresAt' => $this->expiresAt?->format(\DateTimeInterface::ATOM),
7575
];
7676
}
77+
78+
/**
79+
* Normalize ETag by stripping surrounding quotes while preserving weak indicator.
80+
* Converts: "value" → value, W/"value" → W/value.
81+
*/
82+
private static function normalizeEtag(?string $etag): ?string
83+
{
84+
if (null === $etag) {
85+
return null;
86+
}
87+
88+
$weak = '';
89+
90+
if (str_starts_with($etag, 'W/')) {
91+
$weak = 'W/';
92+
$etag = substr($etag, 2);
93+
}
94+
95+
return $weak.trim($etag, '"');
96+
}
7797
}

src/Controller/CdnController.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,10 @@ public function __invoke(Request $request, string $id, string $filename, string
8181
}
8282

8383
if (true === $this->etag && null !== $metadata->etag) {
84-
$response->setEtag($metadata->etag);
84+
$etag = $metadata->etag;
85+
$weak = str_starts_with($etag, 'W/');
86+
87+
$response->setEtag($weak ? substr($etag, 2) : $etag, $weak);
8588
}
8689

8790
if (null !== $this->maxAge) {

tests/Unit/Cdn/Domain/CdnFileMetadataTest.php

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public function fromArray(): void
7070

7171
self::assertSame('https://a.storyblok.com/f/12345/image.jpg', $metadata->originalUrl);
7272
self::assertSame('image/png', $metadata->contentType);
73-
self::assertSame('"xyz789"', $metadata->etag);
73+
self::assertSame('xyz789', $metadata->etag);
7474
self::assertSame('2025-06-15T10:30:00+00:00', $metadata->expiresAt?->format(\DateTimeInterface::ATOM));
7575
}
7676

@@ -104,7 +104,7 @@ public function withDownloadInfo(): void
104104

105105
self::assertSame('https://a.storyblok.com/f/12345/image.jpg', $enriched->originalUrl);
106106
self::assertSame('image/webp', $enriched->contentType);
107-
self::assertSame('"newetag"', $enriched->etag);
107+
self::assertSame('newetag', $enriched->etag);
108108
self::assertSame($expiresAt, $enriched->expiresAt);
109109
}
110110

@@ -190,4 +190,70 @@ public function jsonSerializeWithNullValues(): void
190190
'expiresAt' => null,
191191
], $json);
192192
}
193+
194+
#[Test]
195+
public function fromArrayNormalizesStrongEtag(): void
196+
{
197+
$data = [
198+
'originalUrl' => 'https://a.storyblok.com/f/12345/image.jpg',
199+
'contentType' => 'image/jpeg',
200+
'etag' => '"f062382e65b34974ba85009cf8625737"',
201+
'expiresAt' => null,
202+
];
203+
204+
$metadata = CdnFileMetadata::fromArray($data);
205+
206+
self::assertSame('f062382e65b34974ba85009cf8625737', $metadata->etag);
207+
}
208+
209+
#[Test]
210+
public function fromArrayNormalizesWeakEtag(): void
211+
{
212+
$data = [
213+
'originalUrl' => 'https://a.storyblok.com/f/12345/image.jpg',
214+
'contentType' => 'image/jpeg',
215+
'etag' => 'W/"f062382e65b34974ba85009cf8625737"',
216+
'expiresAt' => null,
217+
];
218+
219+
$metadata = CdnFileMetadata::fromArray($data);
220+
221+
self::assertSame('W/f062382e65b34974ba85009cf8625737', $metadata->etag);
222+
}
223+
224+
#[Test]
225+
public function withDownloadInfoNormalizesStrongEtag(): void
226+
{
227+
$metadata = new CdnFileMetadata(
228+
originalUrl: 'https://a.storyblok.com/f/12345/image.jpg',
229+
);
230+
231+
$enriched = $metadata->withDownloadInfo('image/jpeg', '"abc123"', new DateTimeImmutable());
232+
233+
self::assertSame('abc123', $enriched->etag);
234+
}
235+
236+
#[Test]
237+
public function withDownloadInfoNormalizesWeakEtag(): void
238+
{
239+
$metadata = new CdnFileMetadata(
240+
originalUrl: 'https://a.storyblok.com/f/12345/image.jpg',
241+
);
242+
243+
$enriched = $metadata->withDownloadInfo('image/jpeg', 'W/"abc123"', new DateTimeImmutable());
244+
245+
self::assertSame('W/abc123', $enriched->etag);
246+
}
247+
248+
#[Test]
249+
public function withDownloadInfoHandlesNullEtag(): void
250+
{
251+
$metadata = new CdnFileMetadata(
252+
originalUrl: 'https://a.storyblok.com/f/12345/image.jpg',
253+
);
254+
255+
$enriched = $metadata->withDownloadInfo('image/jpeg', null, new DateTimeImmutable());
256+
257+
self::assertNull($enriched->etag);
258+
}
193259
}

tests/Unit/Cdn/Storage/CdnFilesystemStorageTest.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,8 @@ public function getMetadataReturnsMetadata(): void
132132

133133
self::assertSame('https://a.storyblok.com/f/12345/image.jpg', $result->originalUrl);
134134
self::assertSame('image/jpeg', $result->contentType);
135-
self::assertSame('"abc123"', $result->etag);
135+
// ETag is normalized when loading from storage (quotes stripped)
136+
self::assertSame('abc123', $result->etag);
136137
}
137138

138139
#[Test]
@@ -207,7 +208,8 @@ public function setMetadataUpdatesExistingMetadata(): void
207208
$result = $this->storage->getMetadata($id, 'image.jpg');
208209

209210
self::assertSame('image/jpeg', $result->contentType);
210-
self::assertSame('"updated"', $result->etag);
211+
// ETag is normalized when loading from storage (quotes stripped)
212+
self::assertSame('updated', $result->etag);
211213
}
212214

213215
#[Test]

tests/Unit/Controller/CdnControllerTest.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,33 @@ public function doesNotSetEtagHeaderWhenNotConfigured(): void
274274
self::assertNull($response->getEtag());
275275
}
276276

277+
#[Test]
278+
public function setsWeakEtagHeaderWhenConfigured(): void
279+
{
280+
$tempFile = $this->createTempFile('content');
281+
$file = new File($tempFile);
282+
283+
$metadata = new CdnFileMetadata(
284+
originalUrl: 'https://a.storyblok.com/f/12345/image.jpg',
285+
contentType: 'image/jpeg',
286+
etag: 'W/"f062382e65b34974ba85009cf8625737"',
287+
expiresAt: new DateTimeImmutable('+1 day'),
288+
);
289+
290+
$storage = self::createMock(CdnStorageInterface::class);
291+
$storage->method('getMetadata')->willReturn($metadata);
292+
$storage->method('hasFile')->willReturn(true);
293+
$storage->method('getFile')->willReturn($file);
294+
295+
$downloader = self::createMock(FileDownloaderInterface::class);
296+
297+
$controller = new CdnController($storage, $downloader, null, null, null, true);
298+
299+
$response = $controller->__invoke(new Request(), 'ef7436441c4defbf', 'image', 'jpg');
300+
301+
self::assertSame('W/"f062382e65b34974ba85009cf8625737"', $response->getEtag());
302+
}
303+
277304
#[Test]
278305
public function setsMaxAgeWhenConfigured(): void
279306
{

0 commit comments

Comments
 (0)