-
Notifications
You must be signed in to change notification settings - Fork 338
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
Set cache-control headers to reduce server load (fixes #412) #1641
Conversation
I don't see the code comment you're referring to in the changes. |
Sorry I forgot to commit some changes, check again now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a small change to make, but it's not consequential enough to mark as request changes.
I’m not a fan of all the middleware being in the same file. We should keep it a folder, yes? |
const user = UserService.Instance; | ||
let caching; | ||
if (user.auth()) { | ||
caching = "private"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a max-age
could be useful here as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That will cause problems if you write a post, then go to the post listing which is cached, and is missing your new post. In any case the main server load is probably from unauthenticated users so this should already help a lot.
Does this resolve #1375? |
}) { | ||
res.setHeader( | ||
"Content-Security-Policy", | ||
`default-src 'self'; manifest-src *; connect-src *; img-src * data:; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline'; form-action 'self'; base-uri 'self'; frame-src *; media-src *` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these really necessary?
unsafe-inline
and unsafe-eval
introduce serious security risks.
Please avoid usage or use with nonce
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Afaik they are required by the js framework, would be great if we could get rid of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pixlguru you can try removing these as a separate PR, but make sure you do real testing on it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I could do this after 18.1 is out, if I remember correctly ,last time I took a look unsave-eval was used by websocket code, so there is a chance that removing it has no side-effects.
See code comment for explanation.