Skip to content

Comments

fix(next/image): fix image-optimizer.ts headers (#82114)#2

Open
MitchLewis930 wants to merge 1 commit intopr_032_beforefrom
pr_032_after
Open

fix(next/image): fix image-optimizer.ts headers (#82114)#2
MitchLewis930 wants to merge 1 commit intopr_032_beforefrom
pr_032_after

Conversation

@MitchLewis930
Copy link

@MitchLewis930 MitchLewis930 commented Jan 30, 2026

PR_032

Summary by CodeRabbit

  • Bug Fixes

    • Image optimizer no longer forwards request headers when handling internal image requests.
  • Tests

    • Added an integration test to validate header/cookie handling during image optimization.
    • Added a test API endpoint to simulate conditional cookie-based image responses.

The headers were forwarded to the serverless function for "internal"
images but not "external" images.

This changes the behavior to be the same for both such that neither
receive headers.
@coderabbitai
Copy link

coderabbitai bot commented Jan 30, 2026

📝 Walkthrough

Walkthrough

Removed forwarding of request headers from internal image requests in the image optimizer and added an integration API route and test that verify cookies are not propagated to internal image API routes.

Changes

Cohort / File(s) Summary
Image Optimizer Core
packages/next/src/server/image-optimizer.ts
Removed forwarding of request headers when creating the mocked/internal request in fetchInternalImage, so incoming cookies and other headers are not propagated.
Test Coverage & API
test/integration/image-optimizer/app/pages/api/conditional-cookie.js, test/integration/image-optimizer/test/util.ts
Added an API route that returns an image only when a Cookie header is present and a new integration test asserting that cookie headers are not forwarded to internal image requests (expects error/401 behavior).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 I nibble headers quiet and clean,
No crumbs of cookies on routes unseen.
Optimizer hops, keeps paths discreet,
Internal doors stay calm and neat.
A tiny change, a tidy feat.

🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is severely incomplete. Only 'PR_032' is provided, missing all required sections like 'What?', 'Why?', and 'How?' from the template. Expand the description with sections explaining what was changed, why it was necessary, and how it solves the problem. Include a link to the related issue (e.g., 'Fixes #82114').
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(next/image): fix image-optimizer.ts headers (#82114)' clearly summarizes the main change: fixing header handling in the image optimizer.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into pr_032_before

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch pr_032_after

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments
test/integration/image-optimizer/test/util.ts (1)

311-316: Consider asserting on the response body for better test clarity and consistency.

The test correctly verifies that cookies are not forwarded to internal image requests. For consistency with similar tests in this file (e.g., lines 1089-1098) and to aid debugging if the test fails, consider adding an assertion on the response body to explicitly verify the error message.

💡 Suggested improvement
 it('should not forward cookie header', async () => {
   const query = { w: ctx.w, q: 30, url: '/api/conditional-cookie' }
   const opts = { headers: { accept: 'image/webp', cookie: '1' } }
   const res = await fetchViaHTTP(ctx.appPort, '/_next/image', query, opts)
   expect(res.status).toBe(400)
+  expect(await res.text()).toBe(
+    `Unable to optimize image and unable to fallback to upstream image`
+  )
 })
📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 742a2c7 and 6b12c60.

📒 Files selected for processing (3)
  • packages/next/src/server/image-optimizer.ts
  • test/integration/image-optimizer/app/pages/api/conditional-cookie.js
  • test/integration/image-optimizer/test/util.ts
💤 Files with no reviewable changes (1)
  • packages/next/src/server/image-optimizer.ts
🧰 Additional context used
🧬 Code graph analysis (1)
test/integration/image-optimizer/test/util.ts (1)
test/lib/next-test-utils.ts (1)
  • fetchViaHTTP (173-181)
🔇 Additional comments (1)
test/integration/image-optimizer/app/pages/api/conditional-cookie.js (1)

1-11: LGTM!

Simple and effective test fixture. The logic correctly supports verifying that cookies are not forwarded: when the image optimizer omits the cookie header, this endpoint returns 401, causing the optimizer to return 400 (as expected by the corresponding test).

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@MitchLewis930
Copy link
Author

@coderabbitai full review

@coderabbitai
Copy link

coderabbitai bot commented Feb 13, 2026

✅ Actions performed

Full review triggered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants