Skip to content
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

[stable27] fix(files): service worker #41004

Closed
wants to merge 1 commit into from
Closed

[stable27] fix(files): service worker #41004

wants to merge 1 commit into from

Conversation

SuperSandro2000
Copy link

@SuperSandro2000 SuperSandro2000 commented Oct 19, 2023

This is a partial backport of 3344f0f

Summary

Backports a fix which allows the service worker of the files app to be reigstered

Refused to create a worker from 'https://nextcloud.mydomain.de/index.php/apps/files/preview-service-worker.js' because it violates the following Content Security Policy directive: "script-src 'nonce-aFNJRWFwcklWUlMvTVM5WDZxdnBtOEtyeWh4OVpzbHBGckh3NkpGeHk4OD06L0c1OEJOU0RiRk9IYVg1OGpzN2NycnZoa2xrZU51WWhmY216M3ZNMHVKaz0='". Note that 'worker-src' was not explicitly set, so 'script-src' is used as a fallback.

I deployed this on my server with NixOS and the error is gone and the networks tab showed that the service worker is being used. Since I am using h3/h2 the difference was not noticeable.

TODO

  • ...

Checklist

@SuperSandro2000
Copy link
Author

Not sure why the tests fail. I wouldn't expect this to have such big impact.

@skjnldsv
Copy link
Member

  • Cypress fails because your fork stable27 branch is not up-to-date
  • Drone fails because
     There was 1 failure:
     294 |  
     295 | 1) OCA\Files\Tests\Controller\ViewControllerTest::testIndexWithRegularBrowser
     296 | Failed asserting that two objects are equal.
     297 | --- Expected
     298 | +++ Actual
     299 | @@ @@
     300 | 'allowedChildSrcDomains' => Array ()
     301 | 'allowedFrameAncestors' => Array (...)
     302 | 'allowedWorkerSrcDomains' => Array (
     303 | +            0 => ''self''
     304 | )
     305 | 'allowedFormActionDomains' => Array (...)
     306 | 'reportTo' => Array ()
     307 |  
     308 | /drone/src/apps/files/tests/Controller/ViewControllerTest.php:419
    
  • Block merges during freezes fails because stable27 is in RC2, and therefore in feature freeze until next Thursday where final releases will be built :)

@SuperSandro2000
Copy link
Author

  • Cypress fails because your fork stable27 branch is not up-to-date

I've based this branch onto the main repos stable27 branch and just rebased it again and there were no changes.

  • Drone fails because

ammended that and ran the test locally which succeeded now.

@skjnldsv skjnldsv mentioned this pull request Oct 25, 2023
6 tasks
@szaimen szaimen changed the title fix(files): service worker [stable27] fix(files): service worker Oct 26, 2023
@Pytal
Copy link
Member

Pytal commented Oct 27, 2023

Cypress seems related

@SuperSandro2000
Copy link
Author

I am not sure how the errors are related to cypress. There seems to be a lot of errors because something with the filesystem is off, probably it is full or inodes ran out and most tests seem to fail with 503 which shouldn't happen based on my change.

@skjnldsv
Copy link
Member

@SuperSandro2000 your master branch is not up to date with our master branch, cypress is therefore failing. Please also rebase this branch to master

@SuperSandro2000
Copy link
Author

Just did that. It's sometimes a bit confusing from where github actions are being taken.

@SuperSandro2000
Copy link
Author

Still doesn't work and I frankly don't really have the time to debug why cypress is misbehaving. Can't you just cherry-pick this trivial fix otherwise we might as well wait for 28.0.0.

@blizzz blizzz mentioned this pull request Nov 13, 2023
@nickvergessen
Copy link
Member

It's sometimes a bit confusing from where github actions are being taken.

Administrators had to approve the run. Did that now

@nickvergessen
Copy link
Member

Performance testing / performance-8.0

Doesn't work on forks, should be fine.
Cypress I can't judge

@blizzz
Copy link
Member

blizzz commented Nov 16, 2023

moving to 27.1.5

@blizzz blizzz mentioned this pull request Dec 4, 2023
@SuperSandro2000
Copy link
Author

I have no idea why Cypress doesn't work and I inclined to just close this. I locally fixed this with a NixOS overlay.

@blizzz
Copy link
Member

blizzz commented Dec 7, 2023

moving to 27.1.6

This is a partial backport of 3344f0f

Co-authored-by: John Molakvoæ <14975046+skjnldsv@users.noreply.github.com>
Signed-off-by: Sandro Jäckel <sandro.jaeckel@gmail.com>
@solracsf
Copy link
Member

solracsf commented Jan 6, 2024

Cleaner version at #42608

@solracsf solracsf closed this Jan 6, 2024
@solracsf solracsf removed this from the Nextcloud 27.1.6 milestone Jan 6, 2024
@SuperSandro2000 SuperSandro2000 deleted the service-worker-csp branch January 6, 2024 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants