reactjs / reactjs/react.dev

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

オープン
#8,134 コメント 5 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

type: typos
主要言語
JavaScript
スター
11.8k
フォーク
7.9k
平均マージ
1日 11時間
マージ済み PR(30日)
11

説明

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.

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

調査の方向性

https://react.dev/reference/react/Suspense の Suspense リファレンスページを確認し、fetchData の例とそのグローバルキャッシュに注目してください。Issue の Next.js app-router の手順で動作を再現し、その後、キャッシュの有効期間とサーバー/クライアントでのフェッチ動作が正確になり、認証済みデータの安全でない扱いを例が示唆しないようにドキュメントを更新してください。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
javascript, next.js, react
領域
documentation, security
issue の種類
ドキュメント
難易度
4/5
見積もり時間
3〜5日
活発さ
停滞
明瞭さ
おおむね明確
初心者へのやさしさ
35/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。