Skip to content

Commit

Permalink
Fix relative redirect (#71932)
Browse files Browse the repository at this point in the history
# Fixing a bug

## What?

Changed the formation of the redirect url to the correct way. _It’s
interesting that you had tests for this, but since only the main page
was tested, it wasn’t caught._

Expanded the tests, made a separate method to assign location, and
described it.

```js
new URL('./relative', 'https://example.com/subdir').href // 'https://example.com/relative'
new URL('./relative', 'https://example.com/subdir/').href // 'https://example.com/subdir/relative'
```

Fixes #71906 [#65893, #67966]

---------

Co-authored-by: Zack Tanner <1939140+ztanner@users.noreply.github.com>
  • Loading branch information
2 people authored and kdy1 committed Oct 30, 2024
1 parent 164b7ed commit 55746d0
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 4 deletions.
22 changes: 22 additions & 0 deletions packages/next/src/client/assign-location.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { addBasePath } from './add-base-path'

/**
* Function to correctly assign location to URL
*
* The method will add basePath, and will also correctly add location (including if it is a relative path)
* @param location Location that should be added to the url
* @param url Base URL to which the location should be assigned
*/
export function assignLocation(location: string, url: URL): URL {
if (location.startsWith('.')) {
const urlBase = url.origin + url.pathname
return new URL(
// In order for a relative path to be added to the current url correctly, the current url must end with a slash
// new URL('./relative', 'https://example.com/subdir').href -> 'https://example.com/relative'
// new URL('./relative', 'https://example.com/subdir/').href -> 'https://example.com/subdir/relative'
(urlBase.endsWith('/') ? urlBase : urlBase + '/') + location
)
}

return new URL(addBasePath(location), url.href)
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import {
type ServerActionAction,
type ServerActionMutable,
} from '../router-reducer-types'
import { addBasePath } from '../../../add-base-path'
import { assignLocation } from '../../../assign-location'
import { createHrefFromUrl } from '../create-href-from-url'
import { handleExternalUrl } from './navigate-reducer'
import { applyRouterStatePatchToTree } from '../apply-router-state-patch-to-tree'
Expand Down Expand Up @@ -129,9 +129,8 @@ async function fetchServerAction(
}

const redirectLocation = location
? new URL(
addBasePath(location),
// Ensure relative redirects in Server Actions work, e.g. redirect('./somewhere-else')
? assignLocation(
location,
new URL(state.canonicalUrl, window.location.href)
)
: undefined
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
'use client'

import { startTransition } from 'react'
import { absoluteRedirect, relativeRedirect } from '../actions'

export default function Page() {
return (
<>
<p>hello subdir page</p>
<button
onClick={async () => {
startTransition(async () => {
await relativeRedirect()
})
}}
id="relative-subdir-redirect"
>
relative redirect
</button>
<button
onClick={async () => {
startTransition(async () => {
await absoluteRedirect()
})
}}
id="absolute-subdir-redirect"
>
absolute redirect
</button>
</>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <p id="page-loaded">hello subdir nested page</p>
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,30 @@ describe('server-actions-relative-redirect', () => {
return 'success'
}, 'success')
})

it('should work with relative redirect from subdir', async () => {
const browser = await next.browser('/subdir')
await browser.waitForElementByCss('#relative-subdir-redirect').click()

await check(async () => {
expect(await browser.waitForElementByCss('#page-loaded').text()).toBe(
'hello subdir nested page'
)

return 'success'
}, 'success')
})

it('should work with absolute redirect from subdir', async () => {
const browser = await next.browser('/subdir')
await browser.waitForElementByCss('#absolute-subdir-redirect').click()

await check(async () => {
expect(await browser.waitForElementByCss('#page-loaded').text()).toBe(
'hello nested page'
)

return 'success'
}, 'success')
})
})

0 comments on commit 55746d0

Please sign in to comment.