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
3 changes: 3 additions & 0 deletions src/http/InternalState.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ pub struct InternalStateFlags {
pub(crate) is_redirect_pending: bool,
pub(crate) is_libdeflate_fast_path_disabled: bool,
pub(crate) resend_request_body_on_redirect: bool,
/// Read in `do_redirect` once the pool/close decision has read `unix_socket_path`.
pub(crate) redirect_is_cross_origin: bool,
/// Set when the TLS handshake completed but the user-supplied JS
/// `checkServerIdentity` callback has not yet approved the peer
/// certificate. While set, `on_writable` must not write any HTTP
Expand Down Expand Up @@ -90,6 +92,7 @@ impl InternalStateFlags {
is_redirect_pending: false,
is_libdeflate_fast_path_disabled: false,
resend_request_body_on_redirect: false,
redirect_is_cross_origin: false,
is_waiting_for_cert_check: false,
receive_paused: false,
body_compressed: false,
Expand Down
18 changes: 13 additions & 5 deletions src/http/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2558,9 +2558,6 @@ impl<'a> HTTPClient<'a> {
self.flags.is_streaming_request_body = false;
}

// Decided before unix_socket_path is cleared below: a unix-socket connection must not be pooled.
let keep_alive_possible = self.is_keep_alive_possible();
self.unix_socket_path = b"";
// TODO: what we do with stream body?
let request_body: &[u8] = if self.state.flags.resend_request_body_on_redirect
&& matches!(self.state.original_request_body, HTTPRequestBody::Bytes(_))
Expand Down Expand Up @@ -2589,7 +2586,7 @@ impl<'a> HTTPClient<'a> {
bun_core::scoped_log!(fetch, "close the tunnel");
self.close_proxy_tunnel(true);
GenHttpContext::<IS_SSL>::close_socket(socket);
} else if keep_alive_possible
} else if self.is_keep_alive_possible()
&& self.is_request_fully_sent()
&& !socket.is_closed_or_has_error()
{
Expand All @@ -2616,6 +2613,8 @@ impl<'a> HTTPClient<'a> {
// (handleResponseMetadata already repointed this.url at the new one).
self.prev_redirect = Vec::new();

self.leave_unix_socket_for_cross_origin_redirect();

// TODO: should this check be before decrementing the redirect count?
// the current logic will allow one less redirect than requested
if self.remaining_redirect_count == 0 {
Expand All @@ -2633,6 +2632,13 @@ impl<'a> HTTPClient<'a> {
self.start(HTTPRequestBody::Bytes(request_body));
}

/// With `unix` the URL's authority is a placeholder, so a same-origin hop stays on the socket.
fn leave_unix_socket_for_cross_origin_redirect(&mut self) {
if self.state.flags.redirect_is_cross_origin {
self.unix_socket_path = b"";
}
}

/// Re-resolve `http_proxy` against the post-redirect `self.url`. The
/// decision was made once on the JS thread at request creation; without
/// this, a redirect into a `no_proxy`-exempt host would still be sent via
Expand Down Expand Up @@ -4246,10 +4252,10 @@ impl<'a> HTTPClient<'a> {
fn do_redirect_multiplexed(&mut self) {
debug_assert!(self.flags.protocol != Protocol::Http1_1);
bun_core::scoped_log!(fetch, "doRedirectMultiplexed");
self.leave_unix_socket_for_cross_origin_redirect();
if matches!(self.state.original_request_body, HTTPRequestBody::Stream(_)) {
self.flags.is_streaming_request_body = false;
}
self.unix_socket_path = b"";
let request_body: &[u8] = if self.state.flags.resend_request_body_on_redirect
&& matches!(self.state.original_request_body, HTTPRequestBody::Bytes(_))
{
Expand Down Expand Up @@ -5139,6 +5145,8 @@ impl<'a> HTTPClient<'a> {
}
}

self.state.flags.redirect_is_cross_origin = !is_same_origin;

// https://fetch.spec.whatwg.org/#concept-http-redirect-fetch
// If request's current URL's origin is not same origin with
// locationURL's origin, then for each headerName of CORS
Expand Down
151 changes: 144 additions & 7 deletions test/js/web/fetch/fetch.unix.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { serve, ServeOptions, Server } from "bun";
import { afterAll, expect, it } from "bun:test";
import { afterAll, beforeEach, describe, expect, it } from "bun:test";
import { mkdirSync, rmSync } from "fs";
import { isWindows, tmpdirSync } from "harness";
import { request } from "http";
Expand Down Expand Up @@ -248,9 +248,9 @@ it("handle redirect to non-unix", async () => {
// Following a redirect hands a reusable connection to the keep-alive pool,
// keyed by the request URL's host and port. A unix-socket connection is not
// reusable that way: the request URL below names the TCP server, so pooling
// the unix connection would serve the TCP hop (and any later fetch of that
// host:port) from the unix server.
it("does not pool the unix-socket connection whose redirect is being followed", async () => {
// either unix connection (the one that carried the 3xx or the one that served
// the hop) would make the plain fetch of that host:port talk to the unix server.
it("does not pool the unix-socket connections of a followed redirect", async () => {
startServer({
fetch(req) {
return new Response(`tcp ${new URL(req.url).pathname}`);
Expand All @@ -268,8 +268,145 @@ it("does not pool the unix-socket connection whose redirect is being followed",

const results: string[] = [];
for (let i = 0; i < 5; i++) {
const response = await fetch(`http://127.0.0.1:${server.port}/hello`, { unix: path });
results.push(`${response.status} ${response.redirected} ${await response.text()}`);
const redirected = await fetch(`http://127.0.0.1:${server.port}/hello`, { unix: path });
const direct = await fetch(`http://127.0.0.1:${server.port}/world`);
results.push(`${redirected.status} ${redirected.redirected} ${await redirected.text()} | ${await direct.text()}`);
}
expect(results).toEqual(Array(5).fill("200 true tcp /world"));
expect(results).toEqual(Array(5).fill("200 true unix /world | tcp /world"));
});

// With `unix`, the URL's authority only fills in the Host header and is what a
// relative Location resolves against. A redirect that stays on that authority
// is still addressed to the unix server, so it is followed over the socket. To
// make a hop that leaves the socket visible (instead of failing to connect),
// every URL below names a live TCP listener.
describe("redirects over a unix socket", () => {
let unixSeen: string[], tcpSeen: string[];
let path: string, origin: string;

beforeEach(() => {
unixSeen = [];
tcpSeen = [];
startServer({
fetch(req) {
tcpSeen.push(`${req.method} ${new URL(req.url).pathname} authorization=${req.headers.get("authorization")}`);
return new Response("tcp");
},
});
origin = `http://127.0.0.1:${server.port}`;
path = startServerUnix({
async fetch(req) {
const { pathname } = new URL(req.url);
unixSeen.push(`${req.method} ${pathname} authorization=${req.headers.get("authorization")}`);
switch (pathname) {
case "/relative":
return new Response(null, { status: 302, headers: { Location: "/done" } });
case "/absolute":
return new Response(null, { status: 302, headers: { Location: `${origin}/done` } });
case "/chain":
return new Response(null, { status: 302, headers: { Location: "/relative" } });
case "/resend":
return new Response(null, { status: 307, headers: { Location: "/echo" } });
case "/leave":
return new Response(null, { status: 302, headers: { Location: `${server.url.origin}/done` } });
case "/echo":
return new Response(`unix echo ${await req.text()}`);
}
return new Response(`unix ${pathname}`);
},
});
});

const headers = { Authorization: "Bearer daemon-token" };

async function summarize(response: Response) {
return {
status: response.status,
redirected: response.redirected,
url: response.url,
body: await response.text(),
unixSeen,
tcpSeen,
};
}

it("follows a relative Location over the socket", async () => {
const response = await fetch(`${origin}/relative`, { unix: path, headers });
expect(await summarize(response)).toEqual({
status: 200,
redirected: true,
url: `${origin}/done`,
body: "unix /done",
unixSeen: ["GET /relative authorization=Bearer daemon-token", "GET /done authorization=Bearer daemon-token"],
tcpSeen: [],
});
});

it("follows an absolute Location on the same authority over the socket", async () => {
const response = await fetch(`${origin}/absolute`, { unix: path, headers });
expect(await summarize(response)).toEqual({
status: 200,
redirected: true,
url: `${origin}/done`,
body: "unix /done",
unixSeen: ["GET /absolute authorization=Bearer daemon-token", "GET /done authorization=Bearer daemon-token"],
tcpSeen: [],
});
});

it("stays on the socket across several hops", async () => {
const response = await fetch(`${origin}/chain`, { unix: path, headers });
expect(await summarize(response)).toEqual({
status: 200,
redirected: true,
url: `${origin}/done`,
body: "unix /done",
unixSeen: [
"GET /chain authorization=Bearer daemon-token",
"GET /relative authorization=Bearer daemon-token",
"GET /done authorization=Bearer daemon-token",
],
tcpSeen: [],
});
});

it("resends the body of a 307 over the socket", async () => {
const response = await fetch(`${origin}/resend`, { unix: path, headers, method: "POST", body: "payload" });
expect(await summarize(response)).toEqual({
status: 200,
redirected: true,
url: `${origin}/echo`,
body: "unix echo payload",
unixSeen: ["POST /resend authorization=Bearer daemon-token", "POST /echo authorization=Bearer daemon-token"],
tcpSeen: [],
});
});

it("works with the usual placeholder host in the URL", async () => {
const response = await fetch("http://localhost/relative", { unix: path, headers });
expect(await summarize(response)).toEqual({
status: 200,
redirected: true,
url: "http://localhost/done",
body: "unix /done",
unixSeen: ["GET /relative authorization=Bearer daemon-token", "GET /done authorization=Bearer daemon-token"],
tcpSeen: [],
});
});

// A Location on another authority names a different server (see "handle
// redirect to non-unix" above): that hop leaves the socket and, like any
// cross-origin redirect, goes out without the credentials.
it("a Location on another authority leaves the socket without the credentials", async () => {
expect(server.url.origin).not.toBe(origin);
const response = await fetch(`${origin}/leave`, { unix: path, headers });
expect(await summarize(response)).toEqual({
status: 200,
redirected: true,
url: `${server.url.origin}/done`,
body: "tcp",
unixSeen: ["GET /leave authorization=Bearer daemon-token"],
tcpSeen: ["GET /done authorization=null"],
});
});
});
Loading