Skip to content

Don't use inline scripts and function constructor #4310

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

Merged

Conversation

mohe2015
Copy link
Contributor

@mohe2015 mohe2015 commented Jun 22, 2025

This allows a stricter Content-Security-Policy which is required for web extensions.

Related #3866 #808 and possibly #1484

@mohe2015 mohe2015 requested a review from a team as a code owner June 22, 2025 15:57
@mohe2015
Copy link
Contributor Author

mohe2015 commented Jun 22, 2025

call to Function() blocked by CSP

There is still something missing

@mohe2015
Copy link
Contributor Author

let result = match Function::new_with_args("dioxus", &code).call1(&JsValue::NULL, &channels)
is the problematic line of code

@mohe2015 mohe2015 force-pushed the feature/strict-content-security-policy branch from e1c5f51 to d75bf9e Compare June 22, 2025 17:00
@mohe2015 mohe2015 changed the title Don't use inline scripts Don't use inline scripts and function constructor Jun 22, 2025
This allows a stricter Content-Security-Policy which is required for web extensions.
@mohe2015 mohe2015 force-pushed the feature/strict-content-security-policy branch from d75bf9e to 7092b45 Compare June 22, 2025 17:01
@mohe2015
Copy link
Contributor Author

Should be ready now

@ealmloff
Copy link
Member

ealmloff commented Jun 23, 2025

I don't think this will work with fullstack streaming like the suspense-carousel playwright test. We need the script to start running before the html is fully loaded which is why we are using an inline script tag to start the wasm. I think we can set a nonce for the inline script or add the hash of inline script contents to the CSP instead

@mohe2015
Copy link
Contributor Author

I don't think this will work with fullstack streaming like the suspense-carousel playwright test. We need the script to start running before the html is fully loaded which is why we are using an inline script tag to start the wasm. I think we can set a nonce for the inline script to allow stricter CSPs

That would not work for web extensions (Reading manifest: Error processing content_security_policy.extension_pages: ‘script-src’ directive contains a forbidden 'nonce-*' keyword) but now I understand why the script needs to start running before the html is fully loaded.

It seems that loading the module script asynchronously is possible and seems to pass the test.

@mohe2015
Copy link
Contributor Author

I don't understand the CI failures here. What do I need to do?

@ealmloff
Copy link
Member

CI failures are unrelated dependency issues I'm working on fixing them in #4312

Copy link
Member

@ealmloff ealmloff left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! The async flag works for fullstack streaming when I tested it. Overall, I think this is a better approach, but there are a few spots that could be refactored a bit

@mohe2015 mohe2015 force-pushed the feature/strict-content-security-policy branch from ed3bd16 to acce49a Compare June 25, 2025 15:49
@mohe2015
Copy link
Contributor Author

mohe2015 commented Jun 25, 2025

Should be ready but I have not tested this yet and don't have more time today.

@mohe2015 mohe2015 requested a review from ealmloff June 26, 2025 15:37
@mohe2015
Copy link
Contributor Author

mohe2015 commented Jun 26, 2025

Looked like it works for me.

@ealmloff ealmloff added web relating to the web renderer for dioxus cli Related to the dioxus-cli program labels Jun 26, 2025
Copy link
Member

@ealmloff ealmloff left a comment

Choose a reason for hiding this comment

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

Looks great, thanks! Confirmed this has the right loading behavior for streaming in debug and release mode

@ealmloff ealmloff merged commit 6cc3797 into DioxusLabs:main Jun 26, 2025
19 checks passed
@mohe2015 mohe2015 deleted the feature/strict-content-security-policy branch June 26, 2025 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Related to the dioxus-cli program web relating to the web renderer for dioxus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants