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

Do not redirect if requested font, style or script can not be found #33174

Merged
merged 1 commit into from
Aug 8, 2022

Conversation

ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Jul 8, 2022

If a page in Nextcloud requests a font, stylesheet or script that doesn't exist we currently redirect to the default page. This comes with the following consequences

  • The redirect causes one additional HTTP request per page load
  • The request to the default page causes server load
  • The requested default page causes traffic
  • The browser requests a font, CSS or JavaScript and gets HTML. This causes a parsing error, e.g. The stylesheet https://localhost/apps/dashboard/ was not loaded because its MIME type, “text/html”, is not “text/css”.

User routing is not affected by this. If you navigate to <domain>/apps/phpmymailapp Nextcloud still redirects to the default page. That behavior is desired.

Tested with FF and Chromium.

Ref https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-Fetch-Dest

@ChristophWurst ChristophWurst added bug 3. to review Waiting for reviews labels Jul 8, 2022
@ChristophWurst ChristophWurst added this to the Nextcloud 25 milestone Jul 8, 2022
@ChristophWurst ChristophWurst self-assigned this Jul 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2022

Possible performance regression detected

Show Output
1 queries added

≠ /remote.php/dav/files/test with 1 queries removed
  - UPDATE "oc_preferences" SET "configvalue" = :dcValue1 WHERE ("userid" = :dcValue2) AND ("appid" = :dcValue3) AND ("configkey" = :dcValue4)
= /remote.php/dav/files/test/test.txt
≠ /remote.php/dav/files/test/many_files with 1 queries added
  + UPDATE "oc_preferences" SET "configvalue" = :dcValue1 WHERE ("userid" = :dcValue2) AND ("appid" = :dcValue3) AND ("configkey" = :dcValue4)
= /remote.php/dav/files/test/new_file.txt
≠ /remote.php/dav/files/test/new_file.txt with 1 queries added
  + UPDATE "oc_preferences" SET "configvalue" = :dcValue1 WHERE ("userid" = :dcValue2) AND ("appid" = :dcValue3) AND ("configkey" = :dcValue4)

@ChristophWurst ChristophWurst changed the title Do not redirect if requested CSS can not be found Do not redirect if requested font, style or script can not be found Jul 8, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Jul 8, 2022

Possible performance regression detected

Show Output
1 queries added

= /remote.php/dav/files/test
≠ /remote.php/dav/files/test/test.txt with 1 queries removed
  - UPDATE "oc_preferences" SET "configvalue" = :dcValue1 WHERE ("userid" = :dcValue2) AND ("appid" = :dcValue3) AND ("configkey" = :dcValue4)
≠ /remote.php/dav/files/test/many_files with 1 queries added
  + UPDATE "oc_preferences" SET "configvalue" = :dcValue1 WHERE ("userid" = :dcValue2) AND ("appid" = :dcValue3) AND ("configkey" = :dcValue4)
= /remote.php/dav/files/test/new_file.txt
≠ /remote.php/dav/files/test/new_file.txt with 1 queries added
  + UPDATE "oc_preferences" SET "configvalue" = :dcValue1 WHERE ("userid" = :dcValue2) AND ("appid" = :dcValue3) AND ("configkey" = :dcValue4)

Copy link
Contributor

@kesselb kesselb left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jul 11, 2022
@PVince81
Copy link
Member

@ChristophWurst please cleanup the commit history

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst ChristophWurst force-pushed the fix/redirect-css-not-found branch from f838d20 to a1149b0 Compare August 8, 2022 12:10
@PVince81 PVince81 merged commit 00a01a1 into master Aug 8, 2022
@PVince81 PVince81 deleted the fix/redirect-css-not-found branch August 8, 2022 15:12
@skjnldsv skjnldsv mentioned this pull request Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug
Projects
Development

Successfully merging this pull request may close these issues.

5 participants