Skip to content

fix: handle relative Location URLs in redirect rewriting (#20)#104

Merged
pi0 merged 4 commits intounjs:mainfrom
guoyangzhen:fix-relative-location
Mar 25, 2026
Merged

fix: handle relative Location URLs in redirect rewriting (#20)#104
pi0 merged 4 commits intounjs:mainfrom
guoyangzhen:fix-relative-location

Conversation

@guoyangzhen
Copy link
Copy Markdown
Contributor

@guoyangzhen guoyangzhen commented Mar 23, 2026

Linked issue

Closes #20

Description

When a proxied server responds with a relative URL in the Location header (e.g., Location: /api/books/1 for 201 Created), the proxy throws:

Uncaught TypeError: Invalid URL

This happens because new URL(proxyRes.headers.location) on line 38 doesn't provide a base URL. Per RFC 9110, the Location header is allowed to contain a relative URI-reference.

Fix

Pass target as the base URL to the URL constructor:

- const u = new URL(proxyRes.headers.location);
+ const u = new URL(proxyRes.headers.location, target);

This resolves relative paths against the proxy target, which is the correct behavior for a reverse proxy.

Summary by CodeRabbit

  • Bug Fixes

    • Better handling of relative redirects in proxied responses: Location headers are now resolved against the target and rewritten so redirected URLs become absolute and honor configured host/protocol rewrite settings.
  • Tests

    • Added tests covering relative-URL redirect resolution and rewrite behavior to ensure preserved paths and query strings when applying host rewrites.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 23, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1471ea67-072b-4cc2-8db3-c23a4e9e7459

📥 Commits

Reviewing files that changed from the base of the PR and between c107a09 and 4cc41c8.

📒 Files selected for processing (1)
  • test/middleware/web-outgoing.test.ts

📝 Walkthrough

Walkthrough

setRedirectHostRewrite now resolves Location response headers against the proxy target by using new URL(proxyRes.headers.location, target), so relative Location values (e.g., /api/books/1) are parsed and rewritten correctly.

Changes

Cohort / File(s) Summary
Redirect Host Rewrite
src/middleware/web-outgoing.ts
Build redirected URL with new URL(proxyRes.headers.location, target) to correctly resolve relative Location headers against the proxy target.
Tests (added)
test/middleware/web-outgoing.test.ts
Added tests verifying relative Location headers (e.g., /api/books/1, /redirect/here?foo=bar) are rewritten into absolute URLs using the configured hostRewrite.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Poem

🐰 A relative path once caused a plight,
I fed it a base and made it right,
From /api to http://ext-manual.com it’d roam,
Now redirects find their proper home,
Hoppity-hop, the headers sing tonight! 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: handling relative Location URLs in redirect rewriting, which directly matches the core fix in the PR.
Linked Issues check ✅ Passed The PR successfully addresses issue #20 by passing the proxy target as a base to the URL constructor when parsing the Location header, enabling correct handling of relative redirect URLs.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the issue: modification to URL constructor call and corresponding test cases for relative Location URL handling.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown
Member

@pi0 pi0 left a comment

Choose a reason for hiding this comment

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

thanks. AI automation detected i have double checked changes.

@pi0 pi0 merged commit 8724245 into unjs:main Mar 25, 2026
1 check was pending
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.

Relative URL for Location response header causes TypeError (e.g. 201 Created)

2 participants