[Bug]: Questionable use of statics in UserService potentially causes wrong request-usersession association

[phiresky]

Requirements

• This is a bug report, and if not, please post to https://lemmy.ml/c/lemmy_support instead.
• Please check to see if this issue already exists.
• It's a single bug. Do not report multiple bugs in one issue.
• It's a frontend issue, not a backend issue; Otherwise please create an issue on the backend repo instead.

Summary

I heard that multiple people have allegedly experienced a situation where they load a page and are logged in as a different user - either very shortly until the page rerenders or possibly until a page refresh.

I looked at the code and it seems there might be a fairly simple explanation due to a race condition (I don't know much about Inferno, just React/SSR).

The catch all request handler:

lemmy-ui/src/server/handlers/catch-all-handler.tsx

Line 25 in 3d58bae

export default async (req: Request, res: Response) => {

calls initializeSite()

lemmy-ui/src/shared/utils/app/initialize-site.ts

Lines 5 to 6 in 3d58bae

	export default function initializeSite(site?: GetSiteResponse) {
	UserService.Instance.myUserInfo = site?.my_user;

Which sets the global UserService. After that, it does an asynchronous call to await activeRoute.fetchInitialData(initialFetchReq); During this time, the UserService.instance.userData may be replaced by a separate incoming http request. After that, it calls renderToString which will create the various JSX components.

In those, the user context is used both in class componets and via the myAuth() function, e.g.:

https://github.com/LemmyNet/lemmy-ui/blob/main/src/shared/components/app/app.tsx

At which point it might be the data from a different user though.

I think the solution would be to store UserService inside the react context instead of using a singleton.

Steps to Reproduce

I have not reproduced this issue.

Technical Details

Lemmy Instance Version

current main branch

Lemmy Instance URL

No response

[makotech222]

Good catch. UserService can safely be a singleton on the client, but on the SSR server it should not be.

[ghost]

Happens on lemmy.world too

Screen.Recording.2023-07-11.at.23.33.25.mp4

Seems like something that is easier to reproduce on bigger instances