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.

Sign up for GitHub

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

Jump to bottom

Serve isso.css separately instead of inline #704

Merged
merged 2 commits into from
Mar 22, 2021
Merged

Serve isso.css separately instead of inline #704

merged 2 commits into from
Mar 22, 2021

Conversation

ix5
Copy link
Member

@ix5 ix5 commented Feb 7, 2021 •
edited
Loading

Instead of embedding isso.css inside the client javascript (which required an style-src: unsafe-inline CSP), fetch isso.css from api.endpoint+"/css/isso.css".

Allow clients to override fetch location using data-isso-css-url="https://comments.example.org/css/isso.css"

Note: No modification needed for packaging since isso.css is already included via MANIFEST.in.

Fixes #584

@ix5 ix5 force-pushed the client-css-no-inline branch 2 times, most recently from 7e37f99 to a1889c0 Compare February 7, 2021 11:47
@jelmer
Copy link
Member

jelmer commented Feb 7, 2021

Doesn't this add an extra roundtrip?

@jelmer jelmer self-requested a review February 7, 2021 20:45
@ix5
Copy link
Member Author

ix5 commented Feb 7, 2021

Doesn't this add an extra roundtrip?

It sure does. All these "security" features cost a lot of requests, same as for CORS preflight :(

@vincentbernat
Copy link
Contributor

I serve JS from a CDN, not from the endpoint. It would be nice to make this configurable, but otherwise, no big trouble for me, as I also build a custom version (notably to remove languages I don't use to minimize size).

@ix5
Copy link
Member Author

ix5 commented Feb 8, 2021

I serve JS from a CDN, not from the endpoint. It would be nice to make this configurable, but otherwise, no big trouble for me, as I also build a custom version (notably to remove languages I don't use to minimize size).

Added data-isso-css-url so you can serve your styles from a CDN. Nice site btw, Mr MTU Ninja ;)

@ix5
Serve isso.css separately instead of inline
ca1a2c7 
Instead of embedding isso.css inside the client javascript
(which required an `style-src: unsafe-inline` CSP), fetch
isso.css from `api.endpoint+"/css/isso.css"`.

Allow clients to override fetch location using
`data-isso-css-url="https://comments.example.org/css/isso.css"`

---

Note: No modification needed for packaging since isso.css is
already included via MANIFEST.in.

Fixes isso-comments#584
@ix5
Copy link
Member Author

ix5 commented Mar 22, 2021

Rebased, @jelmer @vincentbernat @MorrisJobke any other suggestions?

@blatinier
Copy link
Collaborator

Everything is good there.
As for the added round trip it's not too bothersome since http/2 avoid creating multiple connexions.

@blatinier blatinier merged commit a7c1069 into isso-comments:master Mar 22, 2021
@ix5 ix5 deleted the client-css-no-inline branch March 22, 2021 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MorrisJobke MorrisJobke MorrisJobke left review comments

@blatinier blatinier blatinier approved these changes

@jelmer jelmer Awaiting requested review from jelmer

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support Content Security Policy (CSP) without style-src 'unsafe-inline'
5 participants
@ix5 @jelmer @vincentbernat @blatinier @MorrisJobke