reactjs / reactjs/react.dev

[Typo]: Suspense example code promoting data leakage when used in Next.js under app router

Open
#8,134 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

type: typos
Dominant language
JavaScript
Stars
11.8k
Forks
7.9k
Avg merge
1d 11h
Merged PRs (30d)
11

Description

Summary

The docs on React Suspense include examples that lead to data leakage when used in Next.js. The issue is that the cache map is constructed globally, which will remain across user sessions. Therefore, if any api fetch with user credentials happens, the returned data will leak to other users. This issue occurs both on app router's server and client components. I haven't tested the pages router yet.

Idk if Typo is the right type of issue. Feel free to tag it differently.

Page

https://react.dev/reference/react/Suspense

Details

In your documentation of React Suspense, you clearly state, users should use it in frameworks like Next.js:

Data fetching with Suspense-enabled frameworks like Relay and Next.js

Your example however fetches data like this:

let cache = new Map();

export function fetchData(url) {
  if (!cache.has(url)) {
    cache.set(url, getData(url));
  }
  return cache.get(url);
}

Which is then used like this:

const albums = use(fetchData(`/${artistId}/albums`));

I only tested this on the app router, but the pages router likely has similar issues. When this code is run server side, it will put the fetched data inside the cache and return the cached data in any new request. So, if I add authentication to this API fetch, we will get data leakage. The first user requests the page, data will be cached, and all subsequent fetches will expose the user data.

This also happens if "use client" is used in the app router. Because next will render static html and then hydrate it with React javascript.

In that sense, your example also kind of promotes fetching the api twice under the app router. Because both the server and the client will both fetch and suspend due to the api request.

Steps to reproduce

  • Create a next.js app with app router
    pnpm create next-app@latest my-app --yes
    cd my-app
    pnpm dev
    
  • Copy the example code into a component such as ArtistPage.js, Albums.js, data.js
  • Add a console.log to the getData function.
What happens:

If used blank, ArtistPage and Albums are treated as server components. The client won't fetch data and therefore doesn't console.log. The server will print once and then never again, since it has the data already.

If used under "use client" (add that to the top of ArtistPage.js and Albums.js), the server logging behaviour will stay the same, but the client will also log. (This shows the double fetching behaviour as well)

Why I think this is a big issue

Users looking at your docs usually won't know about all the intricacies of Next.js interacting with Suspense. Most users will just copy paste. I had several conversations with an LLM about this topic and it always assumes at first that the server wouldn't fetch, wouldn't store, wouldn't suspend, etc. This indicates that general human intuition would likely have the same assumptions.

Suggestions to fix

  • Moving the cache into a useRef + context. That would be re-constructed on every render and therefore fix the security issue.
  • Document the double fetching behaviour clearly. Or find a workaround such as moving the api fetch into a useEffect, which wouldn't run on the server.
  • Potentially negotiate something with the Next.js devs directly to make server rendering handle this differently. For example, I think it would make sense that the server html would include only the loading indicators and not fetch the data. But I believe that might be tricky in terms of implementation. Also because the js engine would run the promise trigger function before touching any React or Next.js functions.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Review the Suspense reference page at https://react.dev/reference/react/Suspense, focusing on the fetchData example and its global cache. Reproduce the behavior with the issue's Next.js app-router steps, then update the documentation so the cache lifetime and server/client fetching behavior are accurate and the example does not imply unsafe handling of authenticated data.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, next.js, react
Domain
documentation, security
Issue type
Documentation
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.