Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions crates/next-api/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1089,7 +1089,11 @@ impl Project {
self.next_config().computed_asset_prefix().owned().await?,
)
} else {
get_server_chunking_context(options)
get_server_chunking_context(
options,
self.client_relative_path().owned().await?,
self.next_config().computed_asset_prefix().owned().await?,
)
})
}

Expand All @@ -1115,10 +1119,14 @@ impl Project {
get_edge_chunking_context_with_client_assets(
options,
self.client_relative_path().owned().await?,
self.next_config().computed_asset_prefix(),
self.next_config().computed_asset_prefix().owned().await?,
)
} else {
get_edge_chunking_context(options)
get_edge_chunking_context(
options,
self.client_relative_path().owned().await?,
self.next_config().computed_asset_prefix().owned().await?,
)
})
}

Expand Down
9 changes: 7 additions & 2 deletions crates/next-core/src/next_edge/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ pub struct EdgeChunkingContextOptions {
pub async fn get_edge_chunking_context_with_client_assets(
options: EdgeChunkingContextOptions,
client_root: FileSystemPath,
asset_prefix: ResolvedVc<Option<RcStr>>,
asset_prefix: Option<RcStr>,
) -> Result<Vc<Box<dyn ChunkingContext>>> {
let EdgeChunkingContextOptions {
mode,
Expand All @@ -237,7 +237,7 @@ pub async fn get_edge_chunking_context_with_client_assets(
environment.to_resolved().await?,
next_mode.runtime_type(),
)
.asset_base_path(asset_prefix.owned().await?)
.asset_base_path(asset_prefix)
.minify_type(if *turbo_minify.await? {
MinifyType::Minify {
// React needs deterministic function names to work correctly.
Expand Down Expand Up @@ -279,6 +279,8 @@ pub async fn get_edge_chunking_context_with_client_assets(
#[turbo_tasks::function]
pub async fn get_edge_chunking_context(
options: EdgeChunkingContextOptions,
client_root: FileSystemPath,
asset_prefix: Option<RcStr>,
) -> Result<Vc<Box<dyn ChunkingContext>>> {
let EdgeChunkingContextOptions {
mode,
Expand All @@ -305,6 +307,9 @@ pub async fn get_edge_chunking_context(
environment.to_resolved().await?,
next_mode.runtime_type(),
)
.client_roots_override(rcstr!("client"), client_root.clone())
.asset_root_path_override(rcstr!("client"), client_root.join("static/media")?)
.asset_base_path_override(rcstr!("client"), asset_prefix.unwrap())
// Since one can't read files in edge directly, any asset need to be fetched
// instead. This special blob url is handled by the custom fetch
// implementation in the edge sandbox. It will respond with the
Expand Down
4 changes: 3 additions & 1 deletion crates/next-core/src/next_image/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ impl StructuredImageModuleType {
blur_placeholder_mode: BlurPlaceholderMode,
module_asset_context: ResolvedVc<ModuleAssetContext>,
) -> Result<Vc<Box<dyn Module>>> {
let static_asset = StaticUrlJsModule::new(*source).to_resolved().await?;
let static_asset = StaticUrlJsModule::new(*source, Some(rcstr!("client")))
.to_resolved()
.await?;
Ok(module_asset_context
.process(
Vc::upcast(
Expand Down
5 changes: 5 additions & 0 deletions crates/next-core/src/next_server/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1079,6 +1079,8 @@ pub async fn get_server_chunking_context_with_client_assets(
#[turbo_tasks::function]
pub async fn get_server_chunking_context(
options: ServerChunkingContextOptions,
client_root: FileSystemPath,
asset_prefix: Option<RcStr>,
) -> Result<Vc<NodeJsChunkingContext>> {
let ServerChunkingContextOptions {
mode,
Expand Down Expand Up @@ -1108,6 +1110,9 @@ pub async fn get_server_chunking_context(
environment.to_resolved().await?,
next_mode.runtime_type(),
)
.client_roots_override(rcstr!("client"), client_root.clone())
.asset_root_path_override(rcstr!("client"), client_root.join("static/media")?)
.asset_prefix_override(rcstr!("client"), asset_prefix.unwrap())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code calls .unwrap() on asset_prefix which is Option<RcStr>, causing a panic if the value is None. This violates Rust best practices and the type contract.

View Details
📝 Patch Details
diff --git a/crates/next-core/src/next_server/context.rs b/crates/next-core/src/next_server/context.rs
index 8628016ff5..58072b24fe 100644
--- a/crates/next-core/src/next_server/context.rs
+++ b/crates/next-core/src/next_server/context.rs
@@ -1111,9 +1111,13 @@ pub async fn get_server_chunking_context(
         next_mode.runtime_type(),
     )
     .client_roots_override(rcstr!("client"), client_root.clone())
-    .asset_root_path_override(rcstr!("client"), client_root.join("static/media")?)
-    .asset_prefix_override(rcstr!("client"), asset_prefix.unwrap())
-    .minify_type(if *turbo_minify.await? {
+    .asset_root_path_override(rcstr!("client"), client_root.join("static/media")?);
+
+    if let Some(prefix) = asset_prefix {
+        builder = builder.asset_prefix_override(rcstr!("client"), prefix);
+    }
+
+    builder = builder.minify_type(if *turbo_minify.await? {
         MinifyType::Minify {
             mangle: (!*no_mangling.await?).then_some(MangleType::OptimalSize),
         }

Analysis

Unsafe unwrap() on Option in get_server_chunking_context violates type contract

What fails: get_server_chunking_context() in crates/next-core/src/next_server/context.rs:1115 calls .unwrap() on asset_prefix: Option<RcStr> parameter when passing it to .asset_prefix_override(), which will panic if the value is None.

How to reproduce: While the current caller computed_asset_prefix() always returns Some(...), the function signature permits None. If computed_asset_prefix() is modified to return None (which its Vc<Option<RcStr>> return type allows), the code will panic at runtime with "called Option::unwrap() on a None value".

Result: The code uses .unwrap() without handling the None case, violating the type contract. This is the only unwrap in the entire file and inconsistent with the similar function get_server_chunking_context_with_client_assets (line 1034), which correctly passes Option<RcStr> directly to .asset_prefix().

Expected: Functions receiving Option<T> should handle both Some and None cases. Per Rust error handling best practices, unwrap should only be used when failure is provably impossible, but here the type signature explicitly permits None.

Fixed by: Conditionally calling .asset_prefix_override() only when asset_prefix is Some, using if let Some(prefix) = asset_prefix { ... } pattern.

.minify_type(if *turbo_minify.await? {
MinifyType::Minify {
mangle: (!*no_mangling.await?).then_some(MangleType::OptimalSize),
Expand Down
8 changes: 8 additions & 0 deletions test/e2e/url/app/api/edge/route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import imported from '../../../public/vercel.png'
const url = new URL('../../../public/vercel.png', import.meta.url).toString()

export function GET(req, res) {
return Response.json({ imported, url })
}

export const runtime = 'edge'
6 changes: 6 additions & 0 deletions test/e2e/url/app/api/route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import imported from '../../public/vercel.png'
const url = new URL('../../public/vercel.png', import.meta.url).toString()

export function GET(req, res) {
return Response.json({ imported, url })
}
14 changes: 14 additions & 0 deletions test/e2e/url/app/client-edge/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
'use client'

import imported from '../../public/vercel.png'
const url = new URL('../../public/vercel.png', import.meta.url).toString()

export default function Index(props) {
return (
<main>
Hello {imported.src}+{url}
</main>
)
}

export const runtime = 'edge'
12 changes: 12 additions & 0 deletions test/e2e/url/app/client/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
'use client'

import imported from '../../public/vercel.png'
const url = new URL('../../public/vercel.png', import.meta.url).toString()

export default function Index(props) {
return (
<main>
Hello {imported.src}+{url}
</main>
)
}
7 changes: 7 additions & 0 deletions test/e2e/url/app/layout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function RootLayout({ children }) {
return (
<html lang="en">
<body>{children}</body>
</html>
)
}
17 changes: 17 additions & 0 deletions test/e2e/url/app/manifest.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import icon from '../public/vercel.png'
const url = new URL('../public/vercel.png', import.meta.url).toString()

export default function manifest() {
return {
short_name: 'Next.js',
name: 'Next.js',
icons: [
{
src: icon.src,
type: 'image/png',
sizes: '512x512',
},
],
description: url,
}
}
9 changes: 9 additions & 0 deletions test/e2e/url/app/opengraph-image.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import imported from '../public/vercel.png'
const url = new URL('../public/vercel.png', import.meta.url).toString()

export const contentType = 'text/json'

// Image generation
export default async function Image() {
return Response.json({ imported, url })
}
12 changes: 12 additions & 0 deletions test/e2e/url/app/rsc-edge/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import imported from '../../public/vercel.png'
const url = new URL('../../public/vercel.png', import.meta.url).toString()

export default function Index(props) {
return (
<main>
Hello {imported.src}+{url}
</main>
)
}

export const runtime = 'edge'
10 changes: 10 additions & 0 deletions test/e2e/url/app/rsc/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import imported from '../../public/vercel.png'
const url = new URL('../../public/vercel.png', import.meta.url).toString()

export default function Index(props) {
return (
<main>
Hello {imported.src}+{url}
</main>
)
}
13 changes: 13 additions & 0 deletions test/e2e/url/middleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { NextRequest, NextResponse } from 'next/server'

// @ts-ignore
import imported from './public/vercel.png'
const url = new URL('./public/vercel.png', import.meta.url).toString()

export async function middleware(req: NextRequest) {
if (req.nextUrl.toString().endsWith('/middleware')) {
return Response.json({ imported, url })
}

return NextResponse.next()
}
7 changes: 0 additions & 7 deletions test/e2e/url/pages/api/basename.js

This file was deleted.

13 changes: 13 additions & 0 deletions test/e2e/url/pages/api/pages-edge/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import imported from '../../../public/vercel.png'
const url = new URL('../../../public/vercel.png', import.meta.url)

export default (req, res) => {
return new Response(
JSON.stringify({
imported,
url: url.toString(),
})
)
}

export const runtime = 'experimental-edge'
12 changes: 12 additions & 0 deletions test/e2e/url/pages/api/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import fs from 'fs'

import imported from '../../../public/vercel.png'
const url = new URL('../../../public/vercel.png', import.meta.url)

export default (req, res) => {
res.send({
imported,
url: url.toString(),
size: fs.readFileSync(url).length,
})
}
7 changes: 0 additions & 7 deletions test/e2e/url/pages/api/size.js

This file was deleted.

20 changes: 20 additions & 0 deletions test/e2e/url/pages/pages-edge/ssr.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import imported from '../../public/vercel.png'

export function getServerSideProps() {
return {
props: {
url: new URL('../../public/vercel.png', import.meta.url).toString(),
},
}
}

export default function Index({ url }) {
return (
<main>
Hello {imported.src}+
{new URL('../../public/vercel.png', import.meta.url).toString()}+{url}
</main>
)
}

export const runtime = 'experimental-edge'
12 changes: 12 additions & 0 deletions test/e2e/url/pages/pages-edge/static.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import imported from '../../public/vercel.png'
const url = new URL('../../public/vercel.png', import.meta.url).toString()

export default function Index(props) {
return (
<main>
Hello {imported.src}+{url}
</main>
)
}

export const runtime = 'experimental-edge'
18 changes: 18 additions & 0 deletions test/e2e/url/pages/pages/ssg.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import imported from '../../public/vercel.png'

export async function getStaticProps() {
return {
props: {
url: new URL('../../public/vercel.png', import.meta.url).toString(),
},
}
}

export default function Index({ url }) {
return (
<main>
Hello {imported.src}+
{new URL('../../public/vercel.png', import.meta.url).toString()}+{url}
</main>
)
}
18 changes: 18 additions & 0 deletions test/e2e/url/pages/pages/ssr.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import imported from '../../public/vercel.png'

export function getServerSideProps() {
return {
props: {
url: new URL('../../public/vercel.png', import.meta.url).toString(),
},
}
}

export default function Index({ url }) {
return (
<main>
Hello {imported.src}+
{new URL('../../public/vercel.png', import.meta.url).toString()}+{url}
</main>
)
}
10 changes: 10 additions & 0 deletions test/e2e/url/pages/pages/static.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import imported from '../../public/vercel.png'
const url = new URL('../../public/vercel.png', import.meta.url).toString()

export default function Index(props) {
return (
<main>
Hello {imported.src}+{url}
</main>
)
}
15 changes: 0 additions & 15 deletions test/e2e/url/pages/ssg.js

This file was deleted.

15 changes: 0 additions & 15 deletions test/e2e/url/pages/ssr.js

This file was deleted.

9 changes: 0 additions & 9 deletions test/e2e/url/pages/static.js

This file was deleted.

Loading
Loading