Skip to content

Avatar never revokes its blob URLs, and a stale response can show the wrong user's picture #17526

Description

@omlahore

Summary

frontend/src/_ui/Avatar/index.jsx never revokes the blob URLs it creates, because the cleanup function is written but never returned. Separately, the same effect has no guard against a stale response, so a row in the virtualized user list can end up showing a different user's picture.

The code

React.useEffect(() => {
    async function fetchAvatar() {
      const blob = await userService.getAvatar(avatarId);
      setAvatar(URL.createObjectURL(blob));
    }
    if (avatarId) fetchAvatar();

    () => avatar && URL.revokeObjectURL(avatar);
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [avatarId]);

Line 18 is missing a return. As written it constructs an arrow function and discards it, so URL.revokeObjectURL is never reached. The intent is clearly there, it just is not wired up. Every other place in the frontend that creates a blob URL does revoke it properly, Camera.jsx, PDF.jsx, Chat/index.js, _lib/generate-file.js and BulkUploadDrawer, which is what made this one stand out.

no-unused-expressions is not enabled in eslint.config.mjs, so nothing flagged it.

Impact

Avatar is rendered per row in UsersTable.jsx and in VirtualizedUserList.jsx. Because the list is virtualized, avatarId changes on the same mounted component as rows are recycled during scrolling, so this is not a one-off allocation. Each change fetches a new blob and leaks the previous URL for the lifetime of the page.

The same recycling causes the second problem. There is no ignore flag, so if a request for the previous avatarId resolves after the component has moved to a new one, the late response calls setAvatar and overwrites the correct picture. In a members list that means a row showing the wrong person's avatar, which is more confusing than a blank one.

Expected vs actual

Expected: the object URL for an avatar is released when the avatar changes or the component unmounts, and a response that arrives after avatarId has moved on is discarded.

Actual: no URL is ever released, and a late response wins.

Notes

I have a fix ready with tests, three cases covering revoke-on-unmount, revoke-on-id-change and ignoring the stale response. All three fail against main and pass with the change. I am happy to open the PR, or to leave it if you would rather assign this elsewhere. Just say which.

One thing I ran into while writing the tests, which is separate from this issue and I am not proposing to fix here: the frontend jest config sets "testEnvironment": "jest-environment-jsdom" but jest-environment-jsdom is not in frontend/package.json. Since Jest 28 it is not bundled, so on a fresh clone npx jest fails with Test environment jest-environment-jsdom cannot be found before running anything. Existing tests such as src/_helpers/__tests__/validateName.test.js hit the same wall. No CI workflow runs frontend jest so nothing is red because of it, but it does mean the suite cannot be run locally without installing that package by hand. Happy to file it separately if useful.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions