Fix missing integrity hash on preload #161

arnaud-ritti · 2022-01-12T16:28:37Z

Has mentioned in issue #101 , integrity hash is missing into the link headers.

Update test case to be cohérent with the case

weaverryan

Sorry for the slow review! Thanks for this - there is just one minor backwards-compatibility concern we need to address!

weaverryan · 2022-02-10T14:19:45Z

src/Asset/TagRenderer.php

@@ -91,7 +91,7 @@ public function renderWebpackScriptTags(string $entryName, string $packageName =
                $this->convertArrayToAttributes($attributes)
            );

-            $this->renderedFiles['scripts'][] = $attributes['src'];
+            $this->renderedFiles['scripts'][] = $attributes;


Unfortunately, this is a BC break, as it changes the structure of what's returned from getRenderedScripts() and getRenderedStyles().

Probably we need to add and maintain a second property - e.g. $this->renderedFilesWithAttributes(). Then, in getRenderedScripts() (and also for styles), we could add a new bool $includeAttributes = false argument. If that's true, then we return the renderedFilesWithAttributes.

Hi @weaverryan , Sorry for the delay. Thanks for your comment.
I fixed the BC break according your recommendations, if you want to check again.

@weaverryan @jrushlow This PR has been open for almost 2 years now, could either one spare some time to review it further and merge it? Thanks in advance.

mustanggb · 2024-02-06T11:20:54Z

Needs a reroll.

ToshY · 2024-02-16T17:47:47Z

@weaverryan @jrushlow please take a look at this.

ToshY · 2024-02-23T18:50:34Z

@weaverryan @jrushlow please take a look at this.

ToshY · 2024-03-03T16:59:02Z

@weaverryan @jrushlow please take a look at this.

jrushlow · 2024-03-12T12:59:17Z

src/Asset/TagRenderer.php

@@ -91,13 +92,14 @@ public function renderWebpackScriptTags(string $entryName, string $packageName =
                $this->convertArrayToAttributes($attributes)
            );

-            $this->renderedFiles['scripts'][] = $attributes['src'];
+            $this->renderedFiles['scripts'][] = $attributes["src"];


Suggested change

$this->renderedFiles['scripts'][] = $attributes["src"];

$this->renderedFiles['scripts'][] = $attributes['src'];

jrushlow · 2024-03-12T12:59:49Z

src/Asset/TagRenderer.php

@@ -129,7 +131,8 @@ public function renderWebpackLinkTags(string $entryName, string $packageName = n
                $this->convertArrayToAttributes($attributes)
            );

-            $this->renderedFiles['styles'][] = $attributes['href'];
+            $this->renderedFiles['styles'][] = $attributes["href"];


Suggested change

$this->renderedFiles['styles'][] = $attributes["href"];

$this->renderedFiles['styles'][] = $attributes['href'];

ping-localhost · 2024-05-21T13:18:39Z

Can you process the feedback @arnaud-ritti or would you like someone else to pick this up?

ToshY · 2024-09-30T20:02:24Z

Multiple other users have already reported this bug (in #101 , #210 , #220, #225) and others also offered to fix it, but the lack of communication and the delayed response times from the core maintainers is shockingly bad for Symfony standards. And while I fully understand that this is not something Ryan should have to worry about (regarding his health), it does not take away that the original issue #101 was created on December 10, 2020 (and this PR roughly a year later on January 12, 2022).

We are now nearing the end of 2024 and I have a feeling that this will not get merged for another 4 years at this rate.

Is there not a(nother) core maintainer that can take responsibility for this bundle and make sure that atleast bugfixes still have a chance of getting merged?

Kocal · 2024-09-30T20:41:02Z

Hi @ToshY, thanks for pointing this out.

Yes, things have slowed down a lot if not almost stopped, for the reasons you mentioned, but also for the time invested on the Symfony AssetMapper component. Note, this has also affected Webpack Encore.

but the lack of communication and the delayed response times from the core maintainers is shockingly bad for Symfony standards.

I know it can be frustrating, but maybe that Webpack isn't their priority, and above all, whether it's Symfony or not, at the end of the day it's still open-source and nobody owes you anything.

That said, I've been able to merge and create releases for a few weeks now. I'll do my best to handle the existing pull-requests, including this one!

Cheers!

ToshY · 2024-09-30T21:49:47Z

Thank you for your response @Kocal .

Yes, things have slowed down a lot if not almost stopped, for the reasons you mentioned, but also for the time invested on the Symfony AssetMapper component. Note, this has also affected Webpack Encore.

I'm somewhat (positively) surprised that Webpack Encore (in this case you) has made some updates in the last couple of days. That's encouraging to see.

I know it can be frustrating, but maybe that Webpack isn't their priority, and above all, whether it's Symfony or not, at the end of the day it's still open-source and nobody owes you anything.

I am aware of that fact. It's just disappointing that this issue could have been resolved years ago if there was better communication and a more pro-active approach in merging open PRs.

Kocal · 2024-10-01T15:32:10Z

Unfortunately the checkbox "Allow edits and access to secrets by maintainers" is not checked, so I won't be able to push on this PR.
I'm gonna open a new PR (with @arnaud-ritti's commits), fix some things, and merge ASAP.

…cal) This PR was merged into the 2.x branch. Discussion ---------- Fix missing integrity hash on preload tags I wasn't able to edit #161, so I've opened a new PR and made changes here. Fix #101, fix #225 On an existing app, integrity hashes and other attributes are nicely injected into `Link` header: <img width="1101" alt="image" src="https://github.com/user-attachments/assets/3733d80f-1927-49d9-8f0e-c8e340f7ed69"> Commits ------- b8219a8 Pass all attributes to `Link` header 707cd49 style: php-cs-fixer 2025c41 Fix missing integrity hash on preload

Kocal · 2024-10-02T07:34:34Z

Released in https://github.com/symfony/webpack-encore-bundle/releases/tag/v2.2.0

arnaud-ritti added 2 commits January 12, 2022 17:24

Fix missing integrity hash on preload

47d22ee

Update tests

d642586

Update test case to be cohérent with the case

weaverryan reviewed Feb 10, 2022

View reviewed changes

Fix BC break

8411712

jrushlow added the Bug Bug Fix label Jul 6, 2022

ToshY mentioned this pull request Dec 5, 2023

PreloadAssetsEventListener does not use integrity hashes #225

Closed

jrushlow reviewed Mar 12, 2024

View reviewed changes

jrushlow added the Status: Needs Work Additional work is needed label Mar 12, 2024

Kocal changed the base branch from main to 2.x October 1, 2024 15:34

Kocal mentioned this pull request Oct 1, 2024

Fix missing integrity hash on preload tags #237

Merged

Kocal closed this Oct 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix missing integrity hash on preload #161

Fix missing integrity hash on preload #161

arnaud-ritti commented Jan 12, 2022

weaverryan left a comment

weaverryan Feb 10, 2022

arnaud-ritti Mar 29, 2022

ToshY Jan 7, 2024

mustanggb commented Feb 6, 2024

ToshY commented Feb 16, 2024

ToshY commented Feb 23, 2024

ToshY commented Mar 3, 2024

jrushlow Mar 12, 2024

jrushlow Mar 12, 2024

ping-localhost commented May 21, 2024

ToshY commented Sep 30, 2024

Kocal commented Sep 30, 2024

ToshY commented Sep 30, 2024

Kocal commented Oct 1, 2024

Kocal commented Oct 2, 2024

	$this->renderedFiles['scripts'][] = $attributes["src"];
	$this->renderedFiles['scripts'][] = $attributes['src'];

	$this->renderedFiles['styles'][] = $attributes["href"];
	$this->renderedFiles['styles'][] = $attributes['href'];

Fix missing integrity hash on preload #161

Fix missing integrity hash on preload #161

Conversation

arnaud-ritti commented Jan 12, 2022

weaverryan left a comment

Choose a reason for hiding this comment

weaverryan Feb 10, 2022

Choose a reason for hiding this comment

arnaud-ritti Mar 29, 2022

Choose a reason for hiding this comment

ToshY Jan 7, 2024

Choose a reason for hiding this comment

mustanggb commented Feb 6, 2024

ToshY commented Feb 16, 2024

ToshY commented Feb 23, 2024

ToshY commented Mar 3, 2024

jrushlow Mar 12, 2024

Choose a reason for hiding this comment

jrushlow Mar 12, 2024

Choose a reason for hiding this comment

ping-localhost commented May 21, 2024

ToshY commented Sep 30, 2024

Kocal commented Sep 30, 2024

ToshY commented Sep 30, 2024

Kocal commented Oct 1, 2024

Kocal commented Oct 2, 2024