short-circuit OPTIONS requests to pages (vercel#65295)

Currently `OPTIONS` requests to page handlers (eg `pages/foo.tsx` or
`app/foo/page.tsx`) will respond as though it's handling a `GET`
request. There should be no reason for these routes to handle `OPTIONS`
requests as the only valid option is `GET`.

We do not need to special-case actions here because those will always be
invoked from the same origin as the canonical browser URL.

<!-- Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:

## For Contributors

### Improving Documentation

- Run `pnpm prettier-fix` to fix formatting issues before opening the
PR.
- Read the Docs Contribution Guide to ensure your contribution follows
the docs guidelines:
https://nextjs.org/docs/community/contribution-guide

### Adding or Updating Examples

- The "examples guidelines" are followed from our contributing doc
https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md
- Make sure the linting passes by running `pnpm build && pnpm lint`. See
https://github.com/vercel/next.js/blob/canary/contributing/repository/linting.md

### Fixing a bug

- Related issues linked using `fixes #number`
- Tests added. See:
https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md

### Adding a feature

- Implements an existing feature request or RFC. Make sure the feature
request has been accepted for implementation before opening a PR. (A
discussion must be opened, see
https://github.com/vercel/next.js/discussions/new?category=ideas)
- Related issues/discussions are linked using `fixes #number`
- e2e tests added
(https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
- Documentation added
- Telemetry added. In case of a feature if it's used or not.
- Errors have a helpful link attached, see
https://github.com/vercel/next.js/blob/canary/contributing.md


## For Maintainers

- Minimal description (aim for explaining to someone not on the team to
understand the PR)
- When linking to a Slack thread, you might want to share details of the
conclusion
- Link both the Linear (Fixes NEXT-xxx) and the GitHub issues
- Add review comments if necessary to explain to the reviewer the logic
behind a change

### What?

### Why?

### How?

Closes NEXT-
Fixes #

-->


Closes NEXT-3305

Author: ztanner

packages/next/src/server/base-server.ts

@@ -110,7 +110,10 @@ import { getTracer, isBubbledError, SpanKind } from './lib/trace/tracer'
 import { BaseServerSpan } from './lib/trace/constants'
 import { I18NProvider } from './future/helpers/i18n-provider'
 import { sendResponse } from './send-response'
-import { handleInternalServerErrorResponse } from './future/route-modules/helpers/response-handlers'
+import {
+  handleBadRequestResponse,
+  handleInternalServerErrorResponse,
+} from './future/route-modules/helpers/response-handlers'
 import {
   fromNodeOutgoingHttpHeaders,
   normalizeNextQueryParam,
@@ -2466,46 +2469,59 @@ export default abstract class Server<
 
             return null
           }
-        } else if (isPagesRouteModule(routeModule)) {
-          // Due to the way we pass data by mutating `renderOpts`, we can't extend
-          // the object here but only updating its `clientReferenceManifest` and
-          // `nextFontManifest` properties.
-          // https://github.com/vercel/next.js/blob/df7cbd904c3bd85f399d1ce90680c0ecf92d2752/packages/next/server/render.tsx#L947-L952
-          renderOpts.nextFontManifest = this.nextFontManifest
-          renderOpts.clientReferenceManifest =
-            components.clientReferenceManifest
-
-          const request = isNodeNextRequest(req) ? req.originalRequest : req
-          const response = isNodeNextResponse(res) ? res.originalResponse : res
-
-          // Call the built-in render method on the module.
-          result = await routeModule.render(
-            // TODO: fix this type
-            // @ts-expect-error - preexisting accepted this
-            request,
-            response,
-            {
-              page: pathname,
+        } else if (
+          isPagesRouteModule(routeModule) ||
+          isAppPageRouteModule(routeModule)
+        ) {
+          // An OPTIONS request to a page handler is invalid.
+          if (req.method === 'OPTIONS' && !is404Page) {
+            await sendResponse(req, res, handleBadRequestResponse())
+            return null
+          }
+
+          if (isPagesRouteModule(routeModule)) {
+            // Due to the way we pass data by mutating `renderOpts`, we can't extend
+            // the object here but only updating its `clientReferenceManifest` and
+            // `nextFontManifest` properties.
+            // https://github.com/vercel/next.js/blob/df7cbd904c3bd85f399d1ce90680c0ecf92d2752/packages/next/server/render.tsx#L947-L952
+            renderOpts.nextFontManifest = this.nextFontManifest
+            renderOpts.clientReferenceManifest =
+              components.clientReferenceManifest
+
+            const request = isNodeNextRequest(req) ? req.originalRequest : req
+            const response = isNodeNextResponse(res)
+              ? res.originalResponse
+              : res
+
+            // Call the built-in render method on the module.
+            result = await routeModule.render(
+              // TODO: fix this type
+              // @ts-expect-error - preexisting accepted this
+              request,
+              response,
+              {
+                page: pathname,
+                params: opts.params,
+                query,
+                renderOpts,
+              }
+            )
+          } else {
+            const module = components.routeModule as AppPageRouteModule
+
+            // Due to the way we pass data by mutating `renderOpts`, we can't extend the
+            // object here but only updating its `nextFontManifest` field.
+            // https://github.com/vercel/next.js/blob/df7cbd904c3bd85f399d1ce90680c0ecf92d2752/packages/next/server/render.tsx#L947-L952
+            renderOpts.nextFontManifest = this.nextFontManifest
+
+            // Call the built-in render method on the module.
+            result = await module.render(req, res, {
+              page: is404Page ? '/404' : pathname,
               params: opts.params,
               query,
               renderOpts,
-            }
-          )
-        } else if (isAppPageRouteModule(routeModule)) {
-          const module = components.routeModule as AppPageRouteModule
-
-          // Due to the way we pass data by mutating `renderOpts`, we can't extend the
-          // object here but only updating its `nextFontManifest` field.
-          // https://github.com/vercel/next.js/blob/df7cbd904c3bd85f399d1ce90680c0ecf92d2752/packages/next/server/render.tsx#L947-L952
-          renderOpts.nextFontManifest = this.nextFontManifest
-
-          // Call the built-in render method on the module.
-          result = await module.render(req, res, {
-            page: is404Page ? '/404' : pathname,
-            params: opts.params,
-            query,
-            renderOpts,
-          })
+            })
+          }
         } else {
           throw new Error('Invariant: Unknown route module type')
         }

test/e2e/app-dir/options-request/app/app-page/page.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>foo page</div>
+}

test/e2e/app-dir/options-request/app/app-route/route.ts

@@ -0,0 +1,5 @@
+export const dynamic = 'force-dynamic'
+
+export async function GET() {
+  return Response.json({ foo: 'bar' })
+}

test/e2e/app-dir/options-request/app/layout.tsx

@@ -0,0 +1,7 @@
+export default function Root({ children }: { children: React.ReactNode }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}

test/e2e/app-dir/options-request/app/page.tsx

@@ -0,0 +1,41 @@
+'use client'
+export default function Page() {
+  return (
+    <>
+      <button
+        onClick={async () => {
+          const response = await fetch('/app-route', { method: 'OPTIONS' })
+          console.log(await response.json())
+        }}
+      >
+        Trigger Options Request (/app route)
+      </button>
+      <button
+        onClick={async () => {
+          const response = await fetch('/app-page', { method: 'OPTIONS' })
+          console.log(await response.text())
+        }}
+      >
+        Trigger Options Request (/app page)
+      </button>
+      <button
+        onClick={async () => {
+          const response = await fetch('/pages-page', { method: 'OPTIONS' })
+          console.log(await response.text())
+        }}
+      >
+        Trigger Options Request (/pages page)
+      </button>
+      <button
+        onClick={async () => {
+          const response = await fetch('/api/pages-api-handler', {
+            method: 'OPTIONS',
+          })
+          console.log(await response.text())
+        }}
+      >
+        Trigger Options Request (/pages API route)
+      </button>
+    </>
+  )
+}

test/e2e/app-dir/options-request/next.config.js

@@ -0,0 +1,6 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {}
+
+module.exports = nextConfig

test/e2e/app-dir/options-request/options-request.test.ts

@@ -0,0 +1,40 @@
+import { nextTestSetup } from 'e2e-utils'
+
+describe('options-request', () => {
+  const { next } = nextTestSetup({
+    files: __dirname,
+  })
+
+  it.each(['/app-page', '/pages-page'])(
+    'should return a 400 status code when invoking %s with an OPTIONS request',
+    async (path) => {
+      const res = await next.fetch(path, { method: 'OPTIONS' })
+      expect(res.status).toBe(400)
+      // There should be no response body
+      expect(await res.text()).toBe('')
+    }
+  )
+
+  // In app router, OPTIONS is auto-implemented if not provided
+  it('should respond with a 204 No Content when invoking an app route handler with an OPTIONS request', async () => {
+    const res = await next.fetch('/app-route', { method: 'OPTIONS' })
+    expect(res.status).toBe(204)
+    // There should be no response body
+    expect(await res.text()).toBe('')
+  })
+
+  // In pages router, unless the handler explicitly handles OPTIONS, it will handle the request normally
+  it('should respond with a 200 + response body when invoking a pages API route with an OPTIONS request', async () => {
+    const res = await next.fetch('/api/pages-api-handler', {
+      method: 'OPTIONS',
+    })
+    expect(res.status).toBe(200)
+    // There should be no response body
+    expect((await res.json()).message).toBe('Hello from Next.js!')
+  })
+
+  it('should 404 for an OPTIONS request to a non-existent route', async () => {
+    const res = await next.fetch('/non-existent-route', { method: 'OPTIONS' })
+    expect(res.status).toBe(404)
+  })
+})

test/e2e/app-dir/options-request/pages/api/pages-api-handler.ts

@@ -0,0 +1,12 @@
+import type { NextApiRequest, NextApiResponse } from 'next'
+
+type ResponseData = {
+  message: string
+}
+
+export default function handler(
+  req: NextApiRequest,
+  res: NextApiResponse<ResponseData>
+) {
+  res.status(200).json({ message: 'Hello from Next.js!' })
+}

test/e2e/app-dir/options-request/pages/pages-page.tsx

@@ -0,0 +1,3 @@
+export default function Page() {
+  return <div>bar</div>
+}