-
Notifications
You must be signed in to change notification settings - Fork 749
fix: bypass the request when the server.proxy is specified
#3816
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,6 @@ | ||||||||||||||||||||||||||||||||
| import * as path from "@std/path"; | ||||||||||||||||||||||||||||||||
| import { expect } from "@std/expect"; | ||||||||||||||||||||||||||||||||
| import { withTmpDir, writeFiles } from "../../fresh/src/test_utils.ts"; | ||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||
| waitFor, | ||||||||||||||||||||||||||||||||
| waitForText, | ||||||||||||||||||||||||||||||||
|
|
@@ -465,6 +466,91 @@ integrationTest( | |||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // https://github.com/denoland/fresh/issues/3814 | ||||||||||||||||||||||||||||||||
| integrationTest("vite dev - server.proxy bypasses Fresh routes", async () => { | ||||||||||||||||||||||||||||||||
| const api = new URLPattern({ pathname: "/api/*" }); | ||||||||||||||||||||||||||||||||
| const api2 = new URLPattern({ pathname: "/api2/*" }); | ||||||||||||||||||||||||||||||||
| const api3 = new URLPattern({ pathname: "/api3/*" }); | ||||||||||||||||||||||||||||||||
| await using proxy = Deno.serve({ | ||||||||||||||||||||||||||||||||
| hostname: "127.0.0.1", | ||||||||||||||||||||||||||||||||
| port: 0, | ||||||||||||||||||||||||||||||||
| }, (req) => { | ||||||||||||||||||||||||||||||||
| const url = new URL(req.url); | ||||||||||||||||||||||||||||||||
| if (api.test({ pathname: url.pathname })) { | ||||||||||||||||||||||||||||||||
| return new Response("api"); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| if (api2.test({ pathname: url.pathname })) { | ||||||||||||||||||||||||||||||||
| return new Response("api2"); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| if (api3.test({ pathname: url.pathname })) { | ||||||||||||||||||||||||||||||||
| return new Response("api3"); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| throw new Error("unreachable"); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| await using tmp = await withTmpDir({ | ||||||||||||||||||||||||||||||||
| dir: path.join(import.meta.dirname!, ".."), | ||||||||||||||||||||||||||||||||
| prefix: "tmp_vite_", | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| await writeFiles(tmp.dir, { | ||||||||||||||||||||||||||||||||
| "main.ts": `import { App } from "@fresh/core"; | ||||||||||||||||||||||||||||||||
| export const app = new App() | ||||||||||||||||||||||||||||||||
| .get("/", () => new Response("ok")); | ||||||||||||||||||||||||||||||||
| `, | ||||||||||||||||||||||||||||||||
| "vite.config.ts": `import { defineConfig } from "vite"; | ||||||||||||||||||||||||||||||||
| import { fresh } from "@fresh/plugin-vite"; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| export default defineConfig({ | ||||||||||||||||||||||||||||||||
| plugins: [fresh()], | ||||||||||||||||||||||||||||||||
| server: { | ||||||||||||||||||||||||||||||||
| proxy: { | ||||||||||||||||||||||||||||||||
| "/api": Deno.env.get("FRESH_TEST_PROXY_TARGET")!, | ||||||||||||||||||||||||||||||||
| "/api2": { | ||||||||||||||||||||||||||||||||
| target: Deno.env.get("FRESH_TEST_PROXY_TARGET")!, | ||||||||||||||||||||||||||||||||
| rewrite: (path) => path.replace(/^\\/api2\\/ping/, "/api2/pong"), | ||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||
| '^/api3/.*': { | ||||||||||||||||||||||||||||||||
| target: Deno.env.get("FRESH_TEST_PROXY_TARGET")!, | ||||||||||||||||||||||||||||||||
| changeOrigin: true, | ||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
| `, | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| await launchDevServer( | ||||||||||||||||||||||||||||||||
| tmp.dir, | ||||||||||||||||||||||||||||||||
| async (address) => { | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| const res = await fetch(`${address}/api/ping?x=1`); | ||||||||||||||||||||||||||||||||
| expect(res.status).toEqual(200); | ||||||||||||||||||||||||||||||||
| expect(await res.text()).toEqual("api"); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| const res = await fetch(`${address}/api2/pong?y=2`); | ||||||||||||||||||||||||||||||||
| expect(res.status).toEqual(200); | ||||||||||||||||||||||||||||||||
| expect(await res.text()).toEqual("api2"); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| const res = await fetch(`${address}/api3/pong?z=3`); | ||||||||||||||||||||||||||||||||
| expect(res.status).toEqual(200); | ||||||||||||||||||||||||||||||||
| expect(await res.text()).toEqual("api3"); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
Comment on lines
+536
to
+540
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding a negative-case assertion so we prove the bypass is selective — e.g. that
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exactly, thanks! |
||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| // Ensure the bypass is selective — non-proxied routes still hit Fresh. | ||||||||||||||||||||||||||||||||
| const res = await fetch(`${address}/`); | ||||||||||||||||||||||||||||||||
| expect(res.status).toEqual(200); | ||||||||||||||||||||||||||||||||
| expect(await res.text()).toEqual("ok"); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| FRESH_TEST_PROXY_TARGET: `http://127.0.0.1:${proxy.addr.port}`, | ||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| integrationTest("vite dev - source mapped stack traces", async () => { | ||||||||||||||||||||||||||||||||
| const res = await fetch(`${demoServer.address()}/tests/throw`); | ||||||||||||||||||||||||||||||||
| const text = await res.text(); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One behavioral gap worth flagging: Vite's proxy options support a
bypass(req, res, options)callback per entry that can returnfalseto opt out of proxying for a given request. With this matcher, such requests would still be short-circuited away from Fresh and then fall through Vite's proxy without being proxied, ending up as 404s. Probably uncommon in practice — fine to leave for a follow-up, but a code comment noting the limitation would help future readers.