From 94206f04452a1bdffd674f0c28d99f60101d40c9 Mon Sep 17 00:00:00 2001
From: Seiya Nuta <nuta@seiya.me>
Date: Tue, 18 Oct 2022 13:56:28 +0900
Subject: [PATCH] Append the fragment in NextUrl.toString() (#41501)

<!--
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 that you're making:
-->

Consider the following middleware which redirects to
`/path/to#fragment`.

```ts
import { NextResponse } from 'next/server';

export async function middleware(request) {
  const url = new URL('/path/to#fragment', request.url);
  return NextResponse.redirect(url);
}
```

However, it actually redirects to `/path/to`, namely it discards the
fragment part in the destination URL `#fragment`. This is because
`NextURL.toString()` does not append that part. This PR fixes the bug.

## Bug

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
---
 packages/next/server/web/next-url.ts             |  2 +-
 test/e2e/middleware-redirects/app/middleware.js  |  5 +++++
 test/e2e/middleware-redirects/test/index.test.ts | 14 ++++++++++++++
 test/unit/web-runtime/next-url.test.ts           |  9 +++++++++
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/packages/next/server/web/next-url.ts b/packages/next/server/web/next-url.ts
index 117667a53ec4c..8bef9c57ea9a3 100644
--- a/packages/next/server/web/next-url.ts
+++ b/packages/next/server/web/next-url.ts
@@ -181,7 +181,7 @@ export class NextURL {
   get href() {
     const pathname = this.formatPathname()
     const search = this.formatSearch()
-    return `${this.protocol}//${this.host}${pathname}${search}`
+    return `${this.protocol}//${this.host}${pathname}${search}${this.hash}`
   }
 
   set href(url: string) {
diff --git a/test/e2e/middleware-redirects/app/middleware.js b/test/e2e/middleware-redirects/app/middleware.js
index f24f39c3af6d7..392e8f98e7d38 100644
--- a/test/e2e/middleware-redirects/app/middleware.js
+++ b/test/e2e/middleware-redirects/app/middleware.js
@@ -75,4 +75,9 @@ export async function middleware(request) {
     url.searchParams.delete('pathname')
     return Response.redirect(url)
   }
+
+  if (url.pathname === '/with-fragment') {
+    console.log(String(new URL('/new-home#fragment', url)))
+    return Response.redirect(new URL('/new-home#fragment', url))
+  }
 }
diff --git a/test/e2e/middleware-redirects/test/index.test.ts b/test/e2e/middleware-redirects/test/index.test.ts
index f245b22fb38fb..2e780a04fc18c 100644
--- a/test/e2e/middleware-redirects/test/index.test.ts
+++ b/test/e2e/middleware-redirects/test/index.test.ts
@@ -157,6 +157,20 @@ describe('Middleware Redirect', () => {
         .join('\n')
       expect(errors).not.toContain('Failed to lookup route')
     })
+
+    // A regression test for https://github.com/vercel/next.js/pull/41501
+    it(`${label}should redirect with a fragment`, async () => {
+      const res = await fetchViaHTTP(next.url, `${locale}/with-fragment`)
+      const html = await res.text()
+      const $ = cheerio.load(html)
+      const browser = await webdriver(next.url, `${locale}/with-fragment`)
+      try {
+        expect(await browser.eval(`window.location.hash`)).toBe(`#fragment`)
+      } finally {
+        await browser.close()
+      }
+      expect($('.title').text()).toBe('Welcome to a new page')
+    })
   }
   tests()
   testsWithLocale()
diff --git a/test/unit/web-runtime/next-url.test.ts b/test/unit/web-runtime/next-url.test.ts
index 69c80c0a7a35b..03fd0beaf75e9 100644
--- a/test/unit/web-runtime/next-url.test.ts
+++ b/test/unit/web-runtime/next-url.test.ts
@@ -39,6 +39,15 @@ it('does noop changing to an invalid hostname', () => {
   expect(url.toString()).toEqual('https://foo.com/example')
 })
 
+it('preserves the fragment', () => {
+  const url = new NextURL(
+    'https://example.com/path/to?param1=value1#this-is-fragment'
+  )
+  expect(url.toString()).toEqual(
+    'https://example.com/path/to?param1=value1#this-is-fragment'
+  )
+})
+
 it('allows to change the whole href', () => {
   const url = new NextURL('https://localhost.com/foo')
   expect(url.hostname).toEqual('localhost.com')