Skip to content

[fix] Enable built-in transitions without style-src 'unsafe-inline' CSP directive (part 2) #6988

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

Closed
wants to merge 1 commit into from

Conversation

DrCBeatz
Copy link

@DrCBeatz DrCBeatz commented Dec 5, 2021

This is a continuation of PR #6776 (I had to make a new PR while cleaning up the commit history), which fixes #6662. This version includes refactored tests and merge squashes the previous commits.

This PR allows Svelte's built-in transitions to work under a strict content security policy (CSP) without the style-src 'unsafe-inline' CSP directive. It checks for presence of an external CSS stylesheet with the title 'svelte-stylesheet' which contains no CSS rules (i.e. a blank stylesheet). This stylesheet is used at the beginning of the 1st transition rather creating a blank stylesheet and appending it to the DOM (which is in effect inline CSS, which a strict CSP won't allow). If the 'svelte-stylesheet' is not present, everything works the same as before (this also means that transitions will not work under a strict CSP without style-src 'unsafe-inline').

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with [feat], [fix], [chore], or [docs].
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with npm test and lint the project with npm run lint

To run the tests type npm run test -- -g svelte-styles-csp at the command line. To view tests in Chromium browser, set headless_browser to false and transition_delay to true in 'tests/svelte-styles-csp/index.ts'.

@DrCBeatz
Copy link
Author

Hi, could you please run the tests again? Thanks!

@Conduitry
Copy link
Member

Does this add extra code to every app whether or not they need to work in restricted CSP environments? Maybe this makes sense to add as a compiler option? I'm not sure.

@DrCBeatz
Copy link
Author

Hi Conduitry, thanks for looking at this.

The fix for this PR is 30 LOC in src/runtime/internal/style_manager.ts, everything else in the PR is for the tests. So this PR shouldn't cause a significant increase in app bundle size. Given that, I'm not sure if making the fix a compiler option would be needed (unless there were reasons other than bundle size).

@DrCBeatz
Copy link
Author

Hi Conduitry, I removed the fsevents dependency. Please let me know if you have any further questions/comments. Thanks again for your time. Cheers!

@DrCBeatz
Copy link
Author

Hi, I divided up the tests (loading stylesheets and running transitions are separate now). This should either keep the tests from timing out on Mac/Windows or at least show which part of the tests timeout. I assume the Linux tests pass because they run natively and execute quicker, while the Windows and Mac OS tests are timing out because they are running on a VM and hence are slower. Please run the tests again when you get the chance, thanks!

@DrCBeatz DrCBeatz force-pushed the master branch 2 times, most recently from f58d1dc to a9e4de1 Compare December 25, 2021 03:25
@lukas-nyberg
Copy link

Any updates on this? :) just ran into it

@DrCBeatz
Copy link
Author

DrCBeatz commented Jun 9, 2022

Hi Lukas, the tests on this PR still need work. I will have another look as soon as I have time. Cheers!

@DrCBeatz
Copy link
Author

I added unit tests for get_svelte_style_sheet_index() by creating a StyleSheetTestObj class and accepting that type as a parameter in get_svelte_style_sheet_index() (since you can't make a CSSStylesheet without appending it to the DOM). I also got rid of the old E2E tests because Puppeteer was causing Chromium to hang in headless mode, which was making the Github tests timeout.

If the tests could please be run again that would be great. ^_^ Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change transitions to not require style-src 'unsafe-inline' CSP.
3 participants