-
Notifications
You must be signed in to change notification settings - Fork 552
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.
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
[BUG] Integrity error when loading external js resource #4812
Comments
@mdmontesinos your Resource declaration does not include the Integrity property, however this is a requirement for many CDNs. This is an example from the Oqtane Theme:
|
@sbwalker Yes, I know. As I mentioned earlier, Swiperjs does not provide an integrity hash in its installation guide, so I assumed it wasn't required and it had something to do with Oqtane js handling. |
Are you using WebAssembly? If so, there is some documentation related to this topic: |
No, it's static render mode, with server interactivity (although I've gotten the error with all static modules and anonymous user) |
I don't think this would be related to Oqtane... Oqtane simply creates the script tag and puts it into the page output - the browser interprets the script tag and does the validation of the remote resource |
Thanks for the feedback, I'll keep investigating on my side. |
I ended up locking to a specific version of the library (i.e. 14.1.1 instead of generic 14 that could change without me noticing) so that I can manually compute the integrity hash and add it to the resource declaration. The error doesn't seem to appear anymore. |
@sbwalker Sorry for reopening the issue, but actually the error has not disappeared. I actually believe it's related to the "Reload" property of the resources and PageScript, because it generates a preload link. When first loading the page where the script is requested, it works fine and the integrity error does not show up. However, if the first load is in a different page and I then navigate to the page (with enhanced-navigation), the error appears. Also, I have several resources from cdn with the integrity value, but only some of them fail. And even sometimes one fails and the others not, and another time, they change. This is an example of a script that fails when enh-navigating: Resource declaration: new Resource {
ResourceType = ResourceType.Script,
Url = "https://unpkg.com/[email protected]/imagesloaded.pkgd.min.js",
CrossOrigin = "anonymous",
Integrity = "sha384-e3sbGkYzJZpi7OdZc2eUoj7saI8K/Qbn+kPTdWyUQloiKIc9HRH4RUWFVxTonzTg",
Location = ResourceLocation.Body,
Reload = true
}, Page output: <link rel="modulepreload" href="https://unpkg.com/[email protected]/imagesloaded.pkgd.min.js" integrity="sha384-e3sbGkYzJZpi7OdZc2eUoj7saI8K/Qbn+kPTdWyUQloiKIc9HRH4RUWFVxTonzTg" crossorigin="anonymous">
<page-script src="https://unpkg.com/[email protected]/imagesloaded.pkgd.min.js"></page-script> Error:
I'm using https://www.srihash.org/ to compute the hash. |
Oqtane is specifying the integrity and crossorigin in the exact format documented by Microsoft: MackinnonBuck/blazor-page-script@44ad683 I think this will need to be escalated to Mackinnon Buck. |
One other item to mention is that not all JavaScript libraries need to be reloaded on every page navigation. Generally they only need to be reloaded if they contain "onload" logic. If you look at the Arsha theme it only specifies Reload for the custom scripts which are part of theme (as they use "onload") - whereas all of the other references to JavaScript libraries (ie. Swiper) do not specify Reload. |
Usually for me, when I don't specifiy Reload, most libraries won't work properly, and it's also hard to determine when one does theoretically require it. I'll try to get a minimal repo of the integrity issue for Mackinnon Buck. |
page-script was actually made obsolete by the new BlazorJsComponents, which I believe were presented in the TrailBlazor conference. |
And this change was never integrated into the main branch and released in his repo, so perhaps it wasn't properly tested, and I have no way to create a minimal repo without using Oqtane. |
I wasn't able to reproduce with a regular Blazor app (no interactivity) using Mackinnon Buck's PageScript cloned from the branch that contains the integrity and crossorigin. Therefore, it seems to be specific to Oqtane in some way. To clarify the issue and reproduce:
new Resource {
ResourceType = ResourceType.Script,
Url = "https://unpkg.com/[email protected]/imagesloaded.pkgd.min.js",
CrossOrigin = "anonymous",
Integrity = "sha384-e3sbGkYzJZpi7OdZc2eUoj7saI8K/Qbn+kPTdWyUQloiKIc9HRH4RUWFVxTonzTg",
Location = ResourceLocation.Body,
Reload = true
},
This will only happen when a script is added for the first time after an enhanced-navigation. If you do a hard refresh in the page that contains the module, the error won't be thrown (i.e. theme resources never show this behaviour). As a current workaround, you must remove both CrossOrigin and Integrity from the resource. If any of them are not empty, page-script will try to add the link with modulepreload and it will fail. Of course, not having a crossorigin and integrity for external cdn resources is a security risk and vulnerability, so this should be fixed. In my original comment, the resource for swiper contained the CrossOrigin property but not the integrity, which is why it was failing. new Resource {
ResourceType = ResourceType.Script,
Url = "https://cdn.jsdelivr.net/npm/swiper@11/swiper-element-bundle.min.js",
//CrossOrigin = "anonymous",
Location = ResourceLocation.Body,
Reload = true
}
This is only true for resources defined on the theme, as they are always added on the first page load (i.e. bootstrap). However, for resources defined in modules (such as swiper for a oqtane slider module), they might get added for the first time after an enhanced-navigation, which is why they all require the Reload property. |
@zyhfish this looks strange: |
@mdmontesinos indicated that he specified both the integrity and crossorigin properties: "To clarify the issue and reproduce: Create a module (not theme) and declare a js resource with integrity and crossorigin properties, and Reload=true (i.e.):
|
there also have a content in @mdmontesinos 's comment: |
This seems to work, but external resources should always include crossOrigin and integrity due to security concerns. Anyway, I'll try to create a minimal repo for a module that reproduces the issue. |
@sbwalker @zyhfish I just created a minimal repo for a module which reproduces the issue. https://github.com/mdmontesinos/mdmontesinos.Module.Issue4812 |
Check the browser's console logs. It actually seems you are actually reproducing the error, as the masonry grid is not aligned as it should, it's behaving as a regular grid. |
Are you doing a hard-refresh and clearing cache on a different page, and then navigating to the page that contains the module? If so, then I don't know why it behaves differently for you or how to even debug it on my side. |
I followed your steps to repro:
The steps do not say anything about hard refresh or clearing the cache |
Yes, navigating in incognito should reproduce it as well. Then, I don't know what the problem is. |
@zyhfish I was not able to reproduce... however if it is a race condition based on JavaScript library dependencies then I would investigate how the JavaScript is being declared and utilized... I do not understand why the references to masonry.pkgd.min.js and imagesloaded.pkgd.min.js are declared for injection in the Body and have Reload set to True. Reload should only be set to True if the library contains 'onload' events which need to be executed on page navigations. I would try loading these 2 scripts in the Head and setting Reload = false. not sure why this is using a setTimeout of 500 milliseconds? |
@sbwalker The resources are declared for injection in the Body due to performance reasons. If a js script is placed in the Head, it would delay the first render of the page, especially when loading multiple external and "big" resources. As for the 500 milliseconds setTimeout, it was a way to wait for the external resources to load, as they are required in the custom script. I didn't bother to add more advances ways to achieve this in the minimal repo. About the Reload property, I wasn't able to use external scripts yet if they are loaded after an enhanced navigation (not in the first page load), if Reload is not true. @zyhfish The race condition is not the issue I was referring to, it can be solved with a polling and promise mechanism to ensure the Masonry object is defined befored executing the custom script. Again, I didn't want to add more complexity to the minimal repo. |
@mdmontesinos I tried out your repro module again... however I made the changes that i described above (ie. I loaded the external scripts in the Head and set Reload to false):
And the result seems to work fine: You can clearly see that the external JavaScript references were included in the head including the integrity and crossorigin attributes... and the module.js was included in the body using the custom page-script element so that it is executed on every page transition. |
@sbwalker Yes, the external js scripts are added to the head/body, but they aren't actually loaded if Reload is not true. I've updated the minimal repo to .NET 9, added a robust way to wait for them load, and disabled Reload. Also, I've updated the README to show how the grid should look if everything is working as intended and a status message (please verify that you get the same result). If Reload is not true, it only works if the first page load is in the page that contains the module. If the scripts were not loaded on the first hard refresh (Control + F5), and added after an enhanced navigation, they don't work. Obviously, this is with Static Render Mode and Enhanced Navigation, without interactivity. |
I think this thread outlines some of the challenges you are seeing with SSR and JavaScript in Blazor: I think a lot of people were hopeful that this would be resolved in .NET 9... however it looks like it was punted to .NET 10. So this means developers need to come up with their own custom solutions :( |
I have been doing some research related to Blazor Static Rendering and JavaScript. I created a new version of the PageScript custom HTML element which does the following:
The benefit of this approach:
However there are some limitations:
It would be possible to enhance Oqtane so that when a site is using Static Rendering, script elements would be rendered using the page-script approach by default. This would ensure that scripts are always loaded/executed on every Enhanced Navigation. And it would make it easier to integrate JavaScript with custom modules/themes. Note that the existing "Reload" concept would still be retained for backward compatibility ie. for those scenarios where you want to use JavaScript modules and hook the onUpdate() event. There is some run-time efficiency to this approach so in some cases the development complexity may be worth it. |
@sbwalker Thanks for your efforts on improving the js experience in Oqtane!
Does it allow any attribute (even custom ones)? For example, with custom attributes in the script I could achieve the GDPR compliance method I discussed in #4791 (comment).
This is a considerable limitation because scripts in the head are a performance bottleneck. Only critical scripts should be included in the head and the rest in the body to improve the loading in slow devices and networks.
Well, this is essentially the same behaviour as the current implementation, where you use the onUpdate method of the Reload property to hook the scripts. It's not such a big deal.
This is also quite problematic, as you need some kind of manual mechanism to ensure that dependencies are loaded before executing custom scripts. For example, https://github.com/mdmontesinos/mdmontesinos.Module.Issue4812/blob/master/Server/wwwroot/Modules/mdmontesinos.Module.Issue4812/Module.js
This would be the ideal approach, specially easing the process for new developers that are not familiar with these js limitations. |
Oqtane Info
Version - 5.2.4
Render Mode - Static
Interactivity - Server
Database - SQL Server
Describe the bug
Sometimes when loading a js external resource, it fails due to 'integrity' issues with the computed hash. For example, I'm using Swiper.js and including it from its cdn:
(Swiper js installation instructions do not include the integrity)
And I've got this several times, although it's inconsistent and I don't know yet the cause:
Also note that this happened both in an Azure deployment and local environment.
This results in failing to import the module:
Expected Behavior
Steps To Reproduce
Anything else?
The text was updated successfully, but these errors were encountered: