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

middleware.ts treated as a client side component #54549

Closed
1 task done
mansdahlstrom1 opened this issue Aug 25, 2023 · 13 comments · Fixed by #55394
Closed
1 task done

middleware.ts treated as a client side component #54549

mansdahlstrom1 opened this issue Aug 25, 2023 · 13 comments · Fixed by #55394
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked Runtime Related to Node.js or Edge Runtime with Next.js.

Comments

@mansdahlstrom1
Copy link

mansdahlstrom1 commented Aug 25, 2023

Verify canary release

  • I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
      Platform: darwin
      Arch: arm64
      Version: Darwin Kernel Version 22.5.0: Thu Jun  8 22:22:20 PDT 2023; root:xnu-8796.121.3~7/RELEASE_ARM64_T6000
    Binaries:
      Node: 16.20.0
      npm: 8.19.4
      Yarn: 1.22.19
      pnpm: 8.4.0
    Relevant Packages:
      next: 13.4.20-canary.6
      eslint-config-next: 13.4.19
      react: 18.2.0
      react-dom: 18.2.0
      typescript: 5.2.2
    Next.js Config:
      output: N/A

Which area(s) of Next.js are affected? (leave empty if unsure)

Middleware / Edge (API routes, runtime)

Link to the code that reproduces this issue or a replay of the bug

https://github.com/mansdahlstrom1/middleware-client-component-bug

To Reproduce

  1. Create new Next app
  2. Add middleware
  3. mark middleware as server-only with import "server-only";

Describe the Bug

When using middleware we expect the middleware to run server-side only and to be able to use features like Cookies etc. But we are not able to do this as Next seem to think that the middleware is running client side.

Expected Behavior

MIddleware can be marked as server-only. Server side only features work in middleware.ts file.

OR

Clearer error message that explains the real error.

Which browser are you using? (if relevant)

116.0.5845.110

How are you deploying your application? (if relevant)

No response

NEXT-1607

@mansdahlstrom1 mansdahlstrom1 added the bug Issue was opened via the bug report template. label Aug 25, 2023
@github-actions github-actions bot added the Runtime Related to Node.js or Edge Runtime with Next.js. label Aug 25, 2023
@raflymln
Copy link

raflymln commented Sep 3, 2023

I think you don't need to use import "server-only"; in middleware since that package is to mark server functions which separated in another file to not be able to be used on client component/client page. Middleware will always be executed on edge runtime which run on server, no matter if there is import "server-only" or not.

Is the bug still occurs if you remove the import "server-only" line?

@jerrycool123
Copy link

if you import another script marked with import "server-only";, the problem still persists.

@balazsorban44 balazsorban44 added the linear: next Confirmed issue that is tracked by the Next.js team. label Sep 11, 2023
@barryengineerapart
Copy link

barryengineerapart commented Sep 11, 2023

A good example where the problem arises is with AuthJS: Middleware (with sensitive information that should not be downloaded to the client) is marked as server-only (for good reason) and falls victim to this bug. In this case, server-only is saving us from sharing sensitive information on the client, but the question comes in: Why is middleware even being bundled in client code? Perhaps there is a good reason, but if that is the case, then there must be a way to ensure that certain middleware is only run on the server.

@barryengineerapart
Copy link

Anyone found a solution to this yet?

@barryengineerapart
Copy link

It's kinda sad, we are about to abandon Next because of this :( In a last ditch effort, @timneutkens please can you try weigh on this issue. I don't know why it is getting so little attention - I suspect on the following is true:

  1. It is very difficult to fix.
  2. I am making a mountain out of a molehill.
  3. Middleware is still under development and is not finalized yet?

@timneutkens
Copy link
Member

timneutkens commented Sep 14, 2023

@barryengineerapart I don't see why this issue would be a blocker for you, can you expand on that? It's just a matter of not using server-only in middleware for now until this issue is looked at / fixed. This is a bug, it's synced into our internal system but it's lower priority (less severe) than many other issues, so it'll take a while to get fixed from our side.

Middleware is not being bundled as client code / client component, server-only is a feature for React Server Components which is why it was implemented only for files that are compiled as server components. Middleware is compiled as "server-side code" instead. It's similar to pages/api in that way.

@mansdahlstrom1
Copy link
Author

mansdahlstrom1 commented Sep 14, 2023

The issue here is not only the server-only. For me, it's that we can not use cookies(). I left 2 branches in my reproduction repo.

  1. With server-only
  2. Without server-only. This will also break the middleware

Branches here:
https://github.com/mansdahlstrom1/middleware-client-component-bug/branches

You can see the diff here: mansdahlstrom1/middleware-client-component-bug@main...without-server-only

@barryengineerapart
Copy link

@barryengineerapart I don't see why this issue would be a blocker for you, can you expand on that? It's just a matter of not using server-only in middleware for now until this issue is looked at / fixed. This is a bug, it's synced into our internal system but it's lower priority (less severe) than many other issues, so it'll take a while to get fixed from our side.

Middleware is not being bundled as client code / client component, server-only is a feature for React Server Components which is why it was implemented only for files that are compiled as server components. Middleware is compiled as "server-side code" instead. It's similar to pages/api in that way.

Thanks for replying @timneutkens. Just some context: We are evaluating Next for a large company (not affiliated to this GH profile).

The big problem is that middleware is tied directly to authz, which contains incredibly sensitive information. I do understand that no sensitive files will be leaked to the client by including these modules in the middleware without the sever-only. The problem is that without the server-only protection, these files can mistakenly be included in the client bundle. For example, it is easy to accidentally import a sensitive module with VSCode’s help to auto complete imports (this can happen without anyone noticing).

The above problems implies that Next is unable to pass a security audit. I don't know the solution, but I would like Next to work, so if this is not too big of a deal, I can try get someone on our team to submit a patch.

@ztanner
Copy link
Member

ztanner commented Sep 14, 2023

The issue here is not only the server-only. For me, it's that we can not use cookies(). I left 2 branches in my reproduction repo.

  1. With server-only
  2. Without server-only. This will also break the middleware

Branches here: https://github.com/mansdahlstrom1/middleware-client-component-bug/branches

You can see the diff here: mansdahlstrom1/middleware-client-component-bug@main...without-server-only

Can you ensure you're using the latest canary? There was a bug accessing cookies in src/middleware but it should be fixed. For me there were no errors on your without-server-only branch after upgrading.

@mansdahlstrom1
Copy link
Author

thanks @ztanner. Will, try again with latest canary, when was this patched? Could you link to PR or release? Or is the patch only in canary stil?

@mansdahlstrom1
Copy link
Author

mansdahlstrom1 commented Sep 15, 2023

Was just able confirm that the cookies() bug was fixed in the latest canary! But there is stil the issue of server-only. Should we close this issue in favor of another server-only (#55206)?

@kodiakhq kodiakhq bot closed this as completed in #55394 Sep 18, 2023
kodiakhq bot pushed a commit that referenced this issue Sep 18, 2023
…nts targets to be available (#55394)

Users want to use `server-only` to restrict the middleware / app routes / pages api, but now it's failing as we're treating them as different webpack layers, but validating the `server-only` only with server components layers.

Here we modify the rules a bit to let everyone can use "server-only" for the bundles that targeting server-side.

For next-swc transformer, we introduce the new option `bundleType` which only has `"server" | "client" | "default"` 3 values:
* - `server`  for server-side targets, like server components, app routes, pages api, middleware
* - `client`   for client components targets such as client components app pages, or page routes under pages directory.
* - `default` for environment like jest, we don't validate module graph with swc, replaced the `disable_checks` introduced  [#54891](#54891).

Refactor a bit webpack-config to adapt to the new rules, after that `server-only` will be able to used in the server-side targets conventions like middleware and `pages/api`

Fixes #43700
Fixes #54549
Fixes #52833

Closes NEXT-1616
Closes NEXT-1607
Closes NEXT-1385
@mansdahlstrom1
Copy link
Author

Verified the fix with latest canary! Thank you!

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot added the locked label Oct 3, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. locked Runtime Related to Node.js or Edge Runtime with Next.js.
Projects
None yet
7 participants