Skip to content

[AssetMapper] Add Integrity Hashes to ImportMap (wip) #58722

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

[AssetMapper] Add Integrity Hashes to ImportMap (wip) #58722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d0c6ab6
[AssetMapper] Add Integrity Hashes to ImportMap
smnandre Oct 30, 2024
83294e2
fabbot
smnandre Oct 30, 2024
e8c0c50
fix tests
smnandre Oct 30, 2024
82b44da
[AssetMapper] Add integrity hash to modulepreload
pierreboissinot Oct 30, 2024
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
13 changes: 13 additions & 0 deletions src/Symfony/Component/AssetMapper/Factory/MappedAssetFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public function createMappedAsset(string $logicalPath, string $sourcePath): ?Map
$asset->getDependencies(),
$asset->getFileDependencies(),
$asset->getJavaScriptImports(),
$this->getIntegrity($asset, $content),
);

$this->assetsCache[$logicalPath] = $asset;
Expand All @@ -73,6 +74,18 @@ public function createMappedAsset(string $logicalPath, string $sourcePath): ?Map
return $this->assetsCache[$logicalPath];
}

/**
* Returns an SRI integrity hash for the given asset.
*/
private function getIntegrity(MappedAsset $asset, ?string $content): string
{
if (null !== $content) {
return 'sha384-'.base64_encode(hash('sha384', $content, true));
}

return 'sha384-'.base64_encode(hash_file('sha384', $asset->sourcePath, true));
Comment on lines +82 to +86
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be simplified to:

Suggested change
if (null !== $content) {
return 'sha384-'.base64_encode(hash('sha384', $content, true));
}
return 'sha384-'.base64_encode(hash_file('sha384', $asset->sourcePath, true));
$hash = $content !== null ? hash('sha384', $content, true) : hash_file('sha384', $asset->sourcePath, true);
return 'sha384-'.base64_encode($hash);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if adding a var is worth it here. This code is not in the hotpath, so i'd maybe vote for code readability.

wdyt ?

}

/**
* Returns an array of "string digest" and "bool predigested".
*
Expand Down
7 changes: 5 additions & 2 deletions src/Symfony/Component/AssetMapper/ImportMap/ImportMapGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function getEntrypointNames(): array
/**
* @param string[] $entrypointNames
*
* @return array<string, array{path: string, type: string, preload?: bool}>
* @return array<string, array{path: string, type: string, preload?: bool, integrity?: string}>
*
* @internal
*/
Expand Down Expand Up @@ -83,7 +83,7 @@ public function getImportMapData(array $entrypointNames): array
/**
* @internal
*
* @return array<string, array{path: string, type: string}>
* @return array<string, array{path: string, type: string, integrity?: string}>
*/
public function getRawImportMapData(): array
{
Expand All @@ -106,6 +106,9 @@ public function getRawImportMapData(): array

$path = $asset->publicPath;
$data = ['path' => $path, 'type' => $entry->type->value];
if ($asset->integrity) {
$data['integrity'] = $asset->integrity;
}
$rawImportMapData[$entry->importName] = $data;
}

Expand Down
18 changes: 13 additions & 5 deletions src/Symfony/Component/AssetMapper/ImportMap/ImportMapRenderer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public function render(string|array $entryPoint, array $attributes = []): string

$importMapData = $this->importMapGenerator->getImportMapData($entryPoint);
$importMap = [];
$integrityMap = [];
$modulePreloads = [];
$cssLinks = [];
$polyfillPath = null;
Expand All @@ -70,10 +71,14 @@ public function render(string|array $entryPoint, array $attributes = []): string
}

$preload = $data['preload'] ?? false;
$integrity = $data['integrity'] ?? null;
if ('css' !== $data['type']) {
$importMap[$importName] = $path;
if ($integrity) {
$integrityMap[$path] = $integrity;
}
if ($preload) {
$modulePreloads[] = $path;
$modulePreloads[] = ['path' => $path, 'integrity' => $integrity];
}
} elseif ($preload) {
$cssLinks[] = $path;
Expand All @@ -96,7 +101,7 @@ public function render(string|array $entryPoint, array $attributes = []): string
}

$scriptAttributes = $attributes || $this->scriptAttributes ? ' '.$this->createAttributesString($attributes) : '';
$importMapJson = json_encode(['imports' => $importMap], \JSON_THROW_ON_ERROR | \JSON_PRETTY_PRINT | \JSON_UNESCAPED_SLASHES | \JSON_HEX_TAG);
$importMapJson = json_encode(['imports' => $importMap, 'integrity' => $integrityMap], \JSON_THROW_ON_ERROR | \JSON_PRETTY_PRINT | \JSON_UNESCAPED_SLASHES | \JSON_HEX_TAG);
$output .= <<<HTML

<script type="importmap"$scriptAttributes>
Expand Down Expand Up @@ -137,10 +142,13 @@ public function render(string|array $entryPoint, array $attributes = []): string
HTML;
}

foreach ($modulePreloads as $url) {
$url = $this->escapeAttributeValue($url);
foreach ($modulePreloads as $modulePreload) {
$url = $this->escapeAttributeValue($modulePreload['path']);
if ($integrity = $modulePreload['integrity']) {
$integrity = " integrity=\"$integrity\"";
}

$output .= "\n<link rel=\"modulepreload\" href=\"$url\">";
$output .= "\n<link rel=\"modulepreload\" href=\"$url\"$integrity>";
}

if (\count($entryPoint) > 0) {
Expand Down
6 changes: 6 additions & 0 deletions src/Symfony/Component/AssetMapper/MappedAsset.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public function __construct(
private array $dependencies = [],
private array $fileDependencies = [],
private array $javaScriptImports = [],
public readonly ?string $integrity = null,
) {
if (null !== $sourcePath) {
$this->sourcePath = $sourcePath;
Expand All @@ -72,6 +73,11 @@ public function __construct(
}
}

public function getIntegrity(): ?string
{
return $this->integrity;
}

/**
* Assets that the content of this asset depends on - for internal caching.
*
Expand Down
14 changes: 14 additions & 0 deletions src/Symfony/Component/AssetMapper/Tests/Factory/MappedAssetFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public function testCreateMappedAsset()
$this->assertSame('file2.js', $asset->logicalPath);
$this->assertMatchesRegularExpression('/^\/final-assets\/file2-[a-zA-Z0-9]{7,128}\.js$/', $asset->publicPath);
$this->assertSame('/final-assets/file2.js', $asset->publicPathWithoutDigest);
$this->assertSame('sha384-2cpbxkWC8I4PKAhlQ+LaFmVek6qd8w35xUZ+QRGMzcSvX9SP2EgjLvKSawSmS9J7', $asset->integrity);
}

public function testCreateMappedAssetRespectsPreDigestedPaths()
Expand All @@ -46,6 +47,7 @@ public function testCreateMappedAssetRespectsPreDigestedPaths()
$this->assertSame('/final-assets/already-abcdefVWXYZ0123456789.digested.css', $asset->publicPath);
// for pre-digested files, the digest *is* part of the public path
$this->assertSame('/final-assets/already-abcdefVWXYZ0123456789.digested.css', $asset->publicPathWithoutDigest);
$this->assertSame('sha384-qOFoMnD38eWUPqBEMysQtNTWZYxuBssHqARTZQlCi4ZFurKyOI76jbDZJYIsWOmT', $asset->integrity);
}

public function testCreateMappedAssetWithContentThatChanged()
Expand Down Expand Up @@ -81,6 +83,17 @@ public function testCreateMappedAssetWithContentThatDoesNotChange()
$this->assertNull($asset->content);
}

public function testCreateMappedAssetComputeIntegrity()
{
$assetMapper = $this->createFactory();

$asset = $assetMapper->createMappedAsset('file1.css', __DIR__.'/../Fixtures/dir1/file1.css');
$this->assertSame('sha384-XgVLYsLqVK+VujG5zkQyFuRtBc98ql0YRAQYP8CT8paQgxTtAUAdgcvTO9TxlUXp', $asset->integrity);

$asset = $assetMapper->createMappedAsset('file2.js', __DIR__.'/../Fixtures/dir1/file2.js');
$this->assertSame('sha384-2cpbxkWC8I4PKAhlQ+LaFmVek6qd8w35xUZ+QRGMzcSvX9SP2EgjLvKSawSmS9J7', $asset->integrity);
}

public function testCreateMappedAssetWithContentErrorsOnCircularReferences()
{
$factory = $this->createFactory();
Expand Down Expand Up @@ -119,6 +132,7 @@ public function compile(string $content, MappedAsset $asset, AssetMapperInterfac
$factory = $this->createFactory($file6Compiler);
$asset = $factory->createMappedAsset('subdir/file6.js', __DIR__.'/../Fixtures/dir2/subdir/file6.js');
$this->assertSame('7e4f24ebddd4ab2a3bcf0d89270b9f30', $asset->digest);
$this->assertSame('sha384-1GNXqZ7KhQjqjTLnfTOAln3lJn8wex5coUWjjZ7DT40GKkCb5XK+P6kNTHjdnXs/', $asset->integrity);
}

public function testCreateMappedAssetWithPredigested()
Expand Down
7 changes: 6 additions & 1 deletion src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapGeneratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,7 @@ public static function getRawImportMapDataTests(): iterable
'/path/to/simple.js',
publicPathWithoutDigest: '/assets/simple.js',
publicPath: '/assets/simple-d1g3st.js',
integrity: 'simple-integrity',
);
yield 'it adds dependency to the importmap' => [
[
Expand All @@ -360,7 +361,7 @@ public static function getRawImportMapDataTests(): iterable
new MappedAsset(
'app.js',
publicPath: '/assets/app-d1g3st.js',
javaScriptImports: [new JavaScriptImport('/assets/simple.js', assetLogicalPath: $simpleAsset->logicalPath, assetSourcePath: $simpleAsset->sourcePath, isLazy: false, addImplicitlyToImportMap: true)]
javaScriptImports: [new JavaScriptImport('/assets/simple.js', assetLogicalPath: $simpleAsset->logicalPath, assetSourcePath: $simpleAsset->sourcePath, isLazy: false, addImplicitlyToImportMap: true)],
),
$simpleAsset,
],
Expand All @@ -372,6 +373,7 @@ public static function getRawImportMapDataTests(): iterable
'/assets/simple.js' => [
'path' => '/assets/simple-d1g3st.js',
'type' => 'js',
'integrity' => 'simple-integrity',
],
],
];
Expand Down Expand Up @@ -401,6 +403,7 @@ public static function getRawImportMapDataTests(): iterable
'/assets/simple.js' => [
'path' => '/assets/simple-d1g3st.js',
'type' => 'js',
'integrity' => 'simple-integrity',
],
],
];
Expand Down Expand Up @@ -440,6 +443,7 @@ public static function getRawImportMapDataTests(): iterable
'/assets/simple.js' => [
'path' => '/assets/simple-d1g3st.js',
'type' => 'js',
'integrity' => 'simple-integrity',
],
],
];
Expand Down Expand Up @@ -472,6 +476,7 @@ public static function getRawImportMapDataTests(): iterable
'/assets/simple.js' => [
'path' => '/assets/simple-d1g3st.js',
'type' => 'js',
'integrity' => 'simple-integrity',
],
'imports_simple' => [
'path' => '/assets/imports_simple-d1g3st.js',
Expand Down
57 changes: 57 additions & 0 deletions src/Symfony/Component/AssetMapper/Tests/ImportMap/ImportMapRendererTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,4 +211,61 @@ public function testItAddsPreloadLinks()
$this->assertSame(['as' => 'style'], $linkProvider->getLinks()[0]->getAttributes());
$this->assertSame('/assets/styles/app-preload-d1g35t.css', $linkProvider->getLinks()[0]->getHref());
}

public function testIntegrityMap()
{
$importMapGenerator = $this->createMock(ImportMapGenerator::class);
$importMapGenerator->expects($this->once())
->method('getImportMapData')
->with(['app'])
->willReturn([
'app_js_preload' => [
'path' => '/assets/app-preload-d1g35t.js',
'type' => 'js',
'integrity' => 'sha384-abc123-js',
],
'app_js_no_preload' => [
'path' => '/assets/app-no-preload-d1g35t.js',
'type' => 'js',
'integrity' => 'sha384-abc123-js-no',
],
'app_css_preload' => [
'path' => '/assets/styles/app-preload-d1g35t.css',
'type' => 'css',
'integrity' => 'sha384-abc123-css',
],
'app_css_no_preload' => [
'path' => '/assets/styles/app-nopreload-d1g35t.css',
'type' => 'css',
'integrity' => 'sha384-abc123-css-no',
],
]);

$assetPackages = $this->createMock(Packages::class);
$assetPackages->expects($this->any())
->method('getUrl')
->willReturnCallback(function ($path) {
// try to imitate the behavior of the real service
if (str_starts_with($path, 'http') || str_starts_with($path, '/')) {
return $path;
}

return '/subdirectory/'.$path;
});

$renderer = new ImportMapRenderer($importMapGenerator, $assetPackages, polyfillImportName: false);
$html = $renderer->render(['app']);

$this->assertStringContainsString(<<<EOTXT
"app_js_preload": "/subdirectory/assets/app-preload-d1g35t.js",
"app_js_no_preload": "/subdirectory/assets/app-no-preload-d1g35t.js",
EOTXT, $html);

$this->assertStringContainsString(<<<EOTXT
"integrity": {
"/subdirectory/assets/app-preload-d1g35t.js": "sha384-abc123-js",
"/subdirectory/assets/app-no-preload-d1g35t.js": "sha384-abc123-js-no"
}
EOTXT, $html);
}
}