search

symfony / webpack-encore-bundle

Symfony integration with Webpack Encore!
https://symfony.com/webpack-encore
MIT License
933 stars 83 forks source link

PreloadAssetsEventListener does not use integrity hashes #225

Open ToshY opened 9 months ago

ToshY commented 9 months ago

Problem

I've been scratching my head for a while on why I'm getting warnings in my Chrome console denoting the following:

A preload for 'https://example.com/build/850.b58e0264.css' is found, but is not used due to an integrity mismatch.

This happens when I enable integrity hashes in my webpack.config.js and have preload: true in the webpack_encore.yaml.

Looking at the Link response header I see the following:

</build/850.b58e0264.css>; rel="preload"; as="style"

Initially I didn't think much of it (as I don't experience with this at all), but after a bit of researching I landed at the Asset Preloading and Resource Hints with HTTP/2 and WebLink documentation. Here I've found the following section:

According to the Preload specification, when an HTTP/2 server detects that the original (HTTP 1.x) response contains this HTTP header, it will automatically trigger a push for the related file in the same HTTP/2 connection.

After following the link I saw this:

image

So basically "integrity metadata" is part of the (so-called) "preload entry".

Reproduction

./config/webpack-encore.yaml ```yml webpack_encore: # The path where Encore is building the assets - i.e. Encore.setOutputPath() output_path: '%kernel.project_dir%/public/build' # If multiple builds are defined (as shown below), you can disable the default build: # output_path: false # Set attributes that will be rendered on all script and link tags script_attributes: defer: true # link_attributes: # If using Encore.enableIntegrityHashes() and need the crossorigin attribute (default: false, or use 'anonymous' or 'use-credentials') # crossorigin: 'anonymous' # Preload all rendered script and link tags automatically via the HTTP/2 Link header preload: true ```
./webpack.config.js Very basic (truncated) config example, as it only concerns the option `.enableIntegrityHashes()` ```js const Encore = require('@symfony/webpack-encore'); if (!Encore.isRuntimeEnvironmentConfigured()) { Encore.configureRuntimeEnvironment(process.env.NODE_ENV || 'dev'); } Encore .setOutputPath('public/build/') .setPublicPath('/build') .addEntry('js/app', './assets/js/main.js') .addStyleEntry('css/app', './assets/styles/global.scss') .addStyleEntry('css/cs', './assets/styles/custom.css') .splitEntryChunks() .enableIntegrityHashes(Encore.isProduction()) ; module.exports = Encore.getWebpackConfig(); ```

Cause

After finding out that the missing integrity in the Link response header might be the cause of the issue, I set out in trying to debug it 🕵️

I've found that the files involved are PreLoadAssetsEventListener and TagRenderer.

PreLoadAssetsEventListener

In PreLoadAssetsEventListener.php#L50-L68 this part is apparently creating the preload links.

        foreach ($this->tagRenderer->getRenderedScripts() as $href) {
            $link = $this->createLink('preload', $href)->withAttribute('as', 'script');

            if (false !== $crossOrigin) {
                $link = $link->withAttribute('crossorigin', $crossOrigin);
            }

            $linkProvider = $linkProvider->withLink($link);
        }

        foreach ($this->tagRenderer->getRenderedStyles() as $href) {
            $link = $this->createLink('preload', $href)->withAttribute('as', 'style');

            if (false !== $crossOrigin) {
                $link = $link->withAttribute('crossorigin', $crossOrigin);
            }

            $linkProvider = $linkProvider->withLink($link);
        }

There is currently nothing here that adds the integrity to the link, regardless where is should get this info from (we shall see later it can be retrieved from the TagRenderer).

TagRenderer

The TagRenderer has 2 methods thare are directly called from inside the PreLoadAssetsEventListener, which are getRenderedScripts() and getRenderedStyles().

  1. https://github.com/symfony/webpack-encore-bundle/blob/75cb918df3f65e28cf0d4bc03042bc45ccb19dd0/src/Asset/TagRenderer.php#L124
  2. https://github.com/symfony/webpack-encore-bundle/blob/75cb918df3f65e28cf0d4bc03042bc45ccb19dd0/src/Asset/TagRenderer.php#L129

Looking at both methods they will return either the script or styles that are available from $this->renderedFiles. However, both the scripts and styles keys contain simple string arrays of hrefs, as they were added to these arrays in TagRenderer.php#L80 and TagRenderer.php#L118 respectively.

The integrity for both is available as can be seen in TagRenderer.php#L62 and TagRenderer.php#L100, but can/is not being used anywhere else.

Possible solution

After finding out all of the above, I've started trying to think of way on how to "fix" this without making breaking changes 🙃

TagRenderer::getAttributes()

In order to be able to get and use the integrity inside the PreLoadAssetsEventListener, we need to find a way to have the TagRenderer return the complete attributes result (which contains the integrity hash).

To do this I've thought of adding a private property $attributes with a public method getAttributes() (similar to the existing getRenderedScripts() and getRenderedStyles() methods). At the end of the loop it will append to the array of $this->attributes in a similar it way as it does for $this->renderedFiles, however now for all attributes and not just the src or href.

TagRenderer ```php * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. */ namespace Symfony\WebpackEncoreBundle\Asset; use Symfony\Component\Asset\Packages; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; use Symfony\Contracts\Service\ResetInterface; use Symfony\WebpackEncoreBundle\Event\RenderAssetTagEvent; /** * @final */ class TagRenderer implements ResetInterface { private $entrypointLookupCollection; private $packages; private $defaultAttributes; private $defaultScriptAttributes; private $defaultLinkAttributes; private $eventDispatcher; private $renderedFiles = []; private $attributes = []; public function __construct( EntrypointLookupCollectionInterface $entrypointLookupCollection, Packages $packages, array $defaultAttributes = [], array $defaultScriptAttributes = [], array $defaultLinkAttributes = [], EventDispatcherInterface $eventDispatcher = null ) { $this->entrypointLookupCollection = $entrypointLookupCollection; $this->packages = $packages; $this->defaultAttributes = $defaultAttributes; $this->defaultScriptAttributes = $defaultScriptAttributes; $this->defaultLinkAttributes = $defaultLinkAttributes; $this->eventDispatcher = $eventDispatcher; $this->reset(); } public function renderWebpackScriptTags(string $entryName, string $packageName = null, string $entrypointName = null, array $extraAttributes = []): string { $entrypointName = $entrypointName ?: '_default'; $scriptTags = []; $entryPointLookup = $this->getEntrypointLookup($entrypointName); $integrityHashes = ($entryPointLookup instanceof IntegrityDataProviderInterface) ? $entryPointLookup->getIntegrityData() : []; foreach ($entryPointLookup->getJavaScriptFiles($entryName) as $filename) { $attributes = []; $attributes['src'] = $this->getAssetPath($filename, $packageName); $attributes = array_merge($attributes, $this->defaultAttributes, $this->defaultScriptAttributes, $extraAttributes); if (isset($integrityHashes[$filename])) { $attributes['integrity'] = $integrityHashes[$filename]; } $event = new RenderAssetTagEvent( RenderAssetTagEvent::TYPE_SCRIPT, $attributes['src'], $attributes ); if (null !== $this->eventDispatcher) { $event = $this->eventDispatcher->dispatch($event); } $attributes = $event->getAttributes(); $scriptTags[] = sprintf( '', $this->convertArrayToAttributes($attributes) ); $this->renderedFiles['scripts'][] = $attributes['src']; $this->attributes['scripts'][$attributes['src']] = $attributes; } return implode('', $scriptTags); } public function renderWebpackLinkTags(string $entryName, string $packageName = null, string $entrypointName = null, array $extraAttributes = []): string { $entrypointName = $entrypointName ?: '_default'; $scriptTags = []; $entryPointLookup = $this->getEntrypointLookup($entrypointName); $integrityHashes = ($entryPointLookup instanceof IntegrityDataProviderInterface) ? $entryPointLookup->getIntegrityData() : []; foreach ($entryPointLookup->getCssFiles($entryName) as $filename) { $attributes = []; $attributes['rel'] = 'stylesheet'; $attributes['href'] = $this->getAssetPath($filename, $packageName); $attributes = array_merge($attributes, $this->defaultAttributes, $this->defaultLinkAttributes, $extraAttributes); if (isset($integrityHashes[$filename])) { $attributes['integrity'] = $integrityHashes[$filename]; } $event = new RenderAssetTagEvent( RenderAssetTagEvent::TYPE_LINK, $attributes['href'], $attributes ); if (null !== $this->eventDispatcher) { $this->eventDispatcher->dispatch($event); } $attributes = $event->getAttributes(); $scriptTags[] = sprintf( '', $this->convertArrayToAttributes($attributes) ); $this->renderedFiles['styles'][] = $attributes['href']; $this->attributes['styles'][$attributes['href']] = $attributes; } return implode('', $scriptTags); } public function getRenderedScripts(): array { return $this->renderedFiles['scripts']; } public function getRenderedStyles(): array { return $this->renderedFiles['styles']; } public function getDefaultAttributes(): array { return $this->defaultAttributes; } public function getAttributes(): array { return $this->attributes; } /** * @return void */ public function reset() { $this->renderedFiles = [ 'scripts' => [], 'styles' => [], ]; } private function getAssetPath(string $assetPath, string $packageName = null): string { if (null === $this->packages) { throw new \Exception('To render the script or link tags, run "composer require symfony/asset".'); } return $this->packages->getUrl( $assetPath, $packageName ); } private function getEntrypointLookup(string $buildName): EntrypointLookupInterface { return $this->entrypointLookupCollection->getEntrypointLookup($buildName); } private function convertArrayToAttributes(array $attributesMap): string { // remove attributes set specifically to false $attributesMap = array_filter($attributesMap, static function ($value) { return false !== $value; }); return implode(' ', array_map( static function ($key, $value) { // allows for things like defer: true to only render "defer" if (true === $value || null === $value) { return $key; } return sprintf('%s="%s"', $key, htmlentities($value)); }, array_keys($attributesMap), $attributesMap )); } } ```

PreLoadAssetsEventListener

After adding TagRenderer::getAttributes() it can now be used in the PreLoadAssetsEventListener.

It boils down to adding the following snippet in both foreach statements:

$linkAttribute = $this->tagRenderer->getAttributes()['scripts'][$href]; // or 'styles'
if (!empty($linkAttribute['integrity'])) {
    $link = $link->withAttribute('integrity', $linkAttribute['integrity']);
}
PreLoadAssetsEventListener ```php * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. */ namespace Symfony\WebpackEncoreBundle\EventListener; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpKernel\Event\ResponseEvent; use Symfony\Component\WebLink\GenericLinkProvider; use Symfony\Component\WebLink\Link; use Symfony\WebpackEncoreBundle\Asset\EntrypointLookupCollectionInterface; use Symfony\WebpackEncoreBundle\Asset\TagRenderer; /** * @author Ryan Weaver */ class PreLoadAssetsEventListener implements EventSubscriberInterface { private $tagRenderer; public function __construct( TagRenderer $tagRenderer ) { $this->tagRenderer = $tagRenderer; } public function onKernelResponse(ResponseEvent $event): void { if (!$event->isMainRequest()) { return; } $request = $event->getRequest(); if (null === $linkProvider = $request->attributes->get('_links')) { $request->attributes->set( '_links', new GenericLinkProvider() ); } /** @var GenericLinkProvider $linkProvider */ $linkProvider = $request->attributes->get('_links'); $defaultAttributes = $this->tagRenderer->getDefaultAttributes(); $crossOrigin = $defaultAttributes['crossorigin'] ?? false; foreach ($this->tagRenderer->getRenderedScripts() as $href) { $link = $this->createLink('preload', $href)->withAttribute('as', 'script'); if (false !== $crossOrigin) { $link = $link->withAttribute('crossorigin', $crossOrigin); } $linkAttribute = $this->tagRenderer->getAttributes()['scripts'][$href]; if (!empty($linkAttribute['integrity'])) { $link = $link->withAttribute('integrity', $linkAttribute['integrity']); } $linkProvider = $linkProvider->withLink($link); } foreach ($this->tagRenderer->getRenderedStyles() as $href) { $link = $this->createLink('preload', $href)->withAttribute('as', 'style'); if (false !== $crossOrigin) { $link = $link->withAttribute('crossorigin', $crossOrigin); } $linkAttribute = $this->tagRenderer->getAttributes()['styles'][$href]; if (!empty($linkAttribute['integrity'])) { $link = $link->withAttribute('integrity', $linkAttribute['integrity']); } $linkProvider = $linkProvider->withLink($link); } $request->attributes->set('_links', $linkProvider); } public static function getSubscribedEvents(): array { return [ // must run before AddLinkHeaderListener 'kernel.response' => ['onKernelResponse', 50], ]; } private function createLink(string $rel, string $href): Link { return new Link($rel, $href); } } ```

Result

After applying the above solution I see the following Link header in my response.

</build/850.b58e0264.css>; rel="preload"; as="style"; integrity="sha384-SdP6Mg9yHiqLMGKNirMlwZrwGiIehWOOlM2vAuttaEmvOpkofKK/TpKNwksMIKlF"

There are no longer warnings in the Chrome console regarding preload integrity mismatch.

TLDR

As I really would like this to be fixed, I can create a (draft) PR for this if that's okay. I'm not sure if the above solution is a proper way to do this in a technical sense, but that's what I came up with for now. 🙂

ToshY commented 9 months ago

Hey @weaverryan 👋

Could you possibly spare some time to take a look at this? I've just found the related PR #161 which has been open for well over a year, signifying the same issue with a proposed fix, but is has never been merged.

ToshY commented 6 months ago

@weaverryan @jrushlow please take a look at this.

ToshY commented 6 months ago

@weaverryan @jrushlow please take a look at this.

ToshY commented 6 months ago

@weaverryan @jrushlow please take a look at this.

ping-localhost commented 3 months ago

Thank you for doing all this research, I just ran into this issue as well @ToshY.

ToshY commented 3 months ago

@ping-localhost No problem. I'm just slightly disappointed that PR #161 was created over 2 years ago and is still just sitting there collection dust. 🤷‍♂️