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

Set cache-control headers to reduce server load (fixes #412) #1641

Merged
merged 5 commits into from
Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/server/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import RobotsHandler from "./handlers/robots-handler";
import ServiceWorkerHandler from "./handlers/service-worker-handler";
import ThemeHandler from "./handlers/theme-handler";
import ThemesListHandler from "./handlers/themes-list-handler";
import setDefaultCsp from "./middleware/set-default-csp";
import { setCacheControl, setDefaultCsp } from "./middleware";

const server = express();

Expand All @@ -19,6 +19,7 @@ const [hostname, port] = process.env["LEMMY_UI_HOST"]
server.use(express.json());
server.use(express.urlencoded({ extended: false }));
server.use("/static", express.static(path.resolve("./dist")));
server.use(setCacheControl);

if (!process.env["LEMMY_UI_DISABLE_CSP"] && !process.env["LEMMY_UI_DEBUG"]) {
server.use(setDefaultCsp);
Expand Down
42 changes: 42 additions & 0 deletions src/server/middleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import type { NextFunction, Response } from "express";
import { UserService } from "../shared/services";

export function setDefaultCsp({
res,
next,
}: {
res: Response;
next: NextFunction;
}) {
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 *`

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

Copy link
Member Author

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.

Copy link
Member

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.

Copy link

@pixlguru pixlguru Jun 28, 2023

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.

);

next();
}

// Set cache-control headers. If user is logged in, set `private` to prevent storing data in
// shared caches (eg nginx) and leaking of private data. If user is not logged in, allow caching
// all responses for 60 seconds to reduce load on backend and database. The specific cache
// interval is rather arbitrary and could be set higher (less server load) or lower (fresher data).
//
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Cache-Control
export function setCacheControl({
res,
next,
}: {
res: Response;
next: NextFunction;
}) {
const user = UserService.Instance;
var caching;
Nutomic marked this conversation as resolved.
Show resolved Hide resolved
if (user.auth()) {
caching = "private";
Copy link
Collaborator

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

Copy link
Member Author

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.

} else {
caching = "public, max-age=60";
}
res.setHeader("Cache-Control", caching);

next();
}