Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

[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 1 commit
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
Next Next commit
[AssetMapper] Add Integrity Hashes to ImportMap 
This is a basic implementation to support integrity hashes within import maps:

- Computes a base64-encoded SHA-384 digest in the factory.
- Renders the integrity attribute for JavaScript files in the import map.

**TODO**
- [ ] Make the integrity hash optional (e.g., through a constructor argument in the factory)
- [ ] Compute hashes only for certain assets / types / paths ?
- [ ] Expose configuration settings
- [ ] Adapt the FrameworkBundle / DI
- [ ] Determine handling approach for CSS files

**Sources**
- [Subresource Integrity (SRI) Goals - W3C](https://www.w3.org/TR/SRI/#goals)
- [JSPM: JS Integrity with Import Maps](https://jspm.org/js-integrity-with-import-maps)

_PS: I'm a bit short on time lately... so if anyone wants to help or take over, please feel free!_
  • Loading branch information
@smnandre
smnandre committed Oct 30, 2024
commit d0c6ab6a6ceec091f3af7722c60ef769b7314b7f
13 changes: 13 additions & 0 deletions 13 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 7 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
7 changes: 6 additions & 1 deletion 7 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,8 +71,12 @@ 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;
}
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
6 changes: 6 additions & 0 deletions 6 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
18 changes: 16 additions & 2 deletions 18 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-ZDljYTViYzY0NTgyZjA4ZTBmMjgwODY1NDNlMmRhMTY2NTVlOTNhYTlkZjMwZGY5YzU0NjdlNDExMThjY2RjNGFmNWZkNDhmZDg0ODIzMmVmMjkyNmIwNGE2NGJkMjdi', $asset->integrity);
}

public function testCreateMappedAssetRespectsPreDigestedPaths()
Expand All @@ -46,11 +47,12 @@ 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-YThlMTY4MzI3MGY3ZjFlNTk0M2VhMDQ0MzMyYjEwYjRkNGQ2NjU4YzZlMDZjYjA3YTgwNDUzNjUwOTQyOGI4NjQ1YmFiMmIyMzg4ZWZhOGRiMGQ5MjU4MjJjNThlOTkz', $asset->integrity);
}

public function testCreateMappedAssetWithContentThatChanged()
{
$file1Compiler = new class implements AssetCompilerInterface {
$file1Compiler = new class () implements AssetCompilerInterface {
public function supports(MappedAsset $asset): bool
{
return true;
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 All @@ -92,7 +105,7 @@ public function testCreateMappedAssetWithContentErrorsOnCircularReferences()

public function testCreateMappedAssetWithDigest()
{
$file6Compiler = new class implements AssetCompilerInterface {
$file6Compiler = new class () implements AssetCompilerInterface {
public function supports(MappedAsset $asset): bool
{
return true;
Expand All @@ -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 7 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
58 changes: 58 additions & 0 deletions 58 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,62 @@ 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(): void
{
$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);

}
}
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.