diff options
author | 2023-07-23 05:13:48 -0700 | |
---|---|---|
committer | 2023-07-23 05:13:48 -0700 | |
commit | b02f097f4d77472bf4c2672eebb2d807a09d3536 (patch) | |
tree | 6baeb417961a2518f357525aa1a7fe839cd1b3c4 | |
parent | 4e852918a3bc8a08d4bd88c69865ad59e9c3bfc3 (diff) | |
download | bun-b02f097f4d77472bf4c2672eebb2d807a09d3536.tar.gz bun-b02f097f4d77472bf4c2672eebb2d807a09d3536.tar.zst bun-b02f097f4d77472bf4c2672eebb2d807a09d3536.zip |
Fix bugs with connecting to localhost (#3758)
* Fix bugs with connecting to localhost
* Update uws
* More logs
* Allow not setting a hostname
* Make server.hostname & server.protocol faster
* Fixup
* normalize listening host
* Fix test
---------
Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>
-rw-r--r-- | src/bun.js/api/bun/socket.zig | 17 | ||||
-rw-r--r-- | src/bun.js/api/server.zig | 70 | ||||
m--------- | src/deps/uws | 0 | ||||
-rw-r--r-- | src/deps/uws.zig | 14 | ||||
-rw-r--r-- | src/feature_flags.zig | 2 | ||||
-rw-r--r-- | test/js/bun/net/tcp-server.test.ts | 2 | ||||
-rw-r--r-- | test/js/bun/websocket/websocket-server.test.ts | 4 |
7 files changed, 71 insertions, 38 deletions
diff --git a/src/bun.js/api/bun/socket.zig b/src/bun.js/api/bun/socket.zig index 2f35a48a5..ce43e3574 100644 --- a/src/bun.js/api/bun/socket.zig +++ b/src/bun.js/api/bun/socket.zig @@ -18,6 +18,14 @@ const ZigString = JSC.ZigString; const BoringSSL = bun.BoringSSL; const X509 = @import("./x509.zig"); +fn normalizeListeningHost(host: [:0]const u8) ?[*:0]const u8 { + if (host.len == 0 or strings.eqlComptime(host, "0.0.0.0")) { + return null; + } + + return host.ptr; +} + // const Corker = struct { // ptr: ?*[16384]u8 = null, // holder: ?*anyopaque = null, @@ -132,13 +140,6 @@ noinline fn getSSLException(globalThis: *JSC.JSGlobalObject, defaultMessage: []c } fn normalizeHost(input: anytype) @TypeOf(input) { - if (input.len == 0) { - return "localhost"; - } - - if (strings.eqlComptime(input, "localhost")) - return "127.0.0.1"; - return input; } const BinaryType = JSC.BinaryType; @@ -650,7 +651,7 @@ pub const Listener = struct { const socket = uws.us_socket_context_listen( @intFromBool(ssl_enabled), socket_context, - normalizeHost(@as([:0]const u8, host)), + normalizeListeningHost(host), c.port, socket_flags, 8, diff --git a/src/bun.js/api/server.zig b/src/bun.js/api/server.zig index 4da7c6300..801745aee 100644 --- a/src/bun.js/api/server.zig +++ b/src/bun.js/api/server.zig @@ -121,7 +121,7 @@ const BlobFileContentResult = struct { pub const ServerConfig = struct { port: u16 = 0, - hostname: [*:0]const u8 = "localhost", + hostname: ?[*:0]const u8 = null, // TODO: use webkit URL parser instead of bun's base_url: URL = URL{}, @@ -658,7 +658,7 @@ pub const ServerConfig = struct { var args = ServerConfig{ .port = 3000, - .hostname = "0.0.0.0", + .hostname = null, .development = true, }; var has_hostname = false; @@ -874,7 +874,7 @@ pub const ServerConfig = struct { } } else { const hostname: string = - if (has_hostname and std.mem.span(args.hostname).len > 0) std.mem.span(args.hostname) else "0.0.0.0"; + if (has_hostname and std.mem.span(args.hostname.?).len > 0) std.mem.span(args.hostname.?) else "0.0.0.0"; const needsBrackets: bool = strings.isIPV6Address(hostname) and hostname[0] != '['; @@ -4528,6 +4528,9 @@ pub fn NewServer(comptime ssl_enabled_: bool, comptime debug_mode_: bool) type { poll_ref: JSC.PollRef = .{}, temporary_url_buffer: std.ArrayListUnmanaged(u8) = .{}, + cached_hostname: bun.String = bun.String.empty, + cached_protocol: bun.String = bun.String.empty, + flags: packed struct(u3) { deinit_scheduled: bool = false, terminated: bool = false, @@ -4978,15 +4981,29 @@ pub fn NewServer(comptime ssl_enabled_: bool, comptime debug_mode_: bool) type { } pub fn getHostname(this: *ThisServer, globalThis: *JSGlobalObject) JSC.JSValue { - return ZigString.init(bun.span(this.config.hostname)).toValue(globalThis); + if (this.cached_hostname.isEmpty()) { + if (this.listener) |listener| { + var buf: [1024]u8 = [_]u8{0} ** 1024; + var len: i32 = 1024; + listener.socket().remoteAddress(&buf, &len); + if (len > 0) { + this.cached_hostname = bun.String.create(buf[0..@as(usize, @intCast(len))]); + } + } + + if (this.cached_hostname.isEmpty()) + this.cached_hostname = bun.String.create(bun.span(this.config.hostname orelse "localhost")); + } + + return this.cached_hostname.toJS(globalThis); } - pub fn getProtocol(_: *ThisServer, globalThis: *JSGlobalObject) JSC.JSValue { - if (comptime ssl_enabled) { - return ZigString.init("https:").toValue(globalThis); - } else { - return ZigString.init("http:").toValue(globalThis); + pub fn getProtocol(this: *ThisServer, globalThis: *JSGlobalObject) JSC.JSValue { + if (this.cached_protocol.isEmpty()) { + this.cached_protocol = bun.String.create(if (ssl_enabled) "https" else "http"); } + + return this.cached_protocol.toJS(globalThis); } pub fn getDevelopment( @@ -5064,6 +5081,17 @@ pub fn NewServer(comptime ssl_enabled_: bool, comptime debug_mode_: bool) type { pub fn deinit(this: *ThisServer) void { httplog("deinit", .{}); + this.cached_hostname.deref(); + this.cached_protocol.deref(); + + if (this.config.hostname) |host| { + bun.default_allocator.destroy(host); + } + + if (this.config.base_url.href.len > 0) { + bun.default_allocator.free(this.config.base_url.href); + } + this.app.destroy(); const allocator = this.allocator; allocator.destroy(this); @@ -5438,22 +5466,18 @@ pub fn NewServer(comptime ssl_enabled_: bool, comptime debug_mode_: bool) type { this.app.get("/src:/*", *ThisServer, this, onSrcRequest); } - - const hostname = bun.span(this.config.hostname); - - // When "localhost" is specified, we omit the hostname entirely - // Otherwise, "curl http://localhost:3000" doesn't actually work due to IPV6 vs IPV4 issues - // This prints a spurious log si_destination_compare on macOS but only when debugger is connected - var host: [*:0]const u8 = undefined; + var host: ?[*:0]const u8 = null; var host_buff: [1024:0]u8 = undefined; - if (hostname.len == 0 or (!ssl_enabled and strings.eqlComptime(hostname, "localhost"))) { - host = ""; - } else if (hostname.len > 2 and hostname[0] == '[') { - // remove "[" and "]" from hostname - host = std.fmt.bufPrintZ(&host_buff, "{s}", .{hostname[1 .. hostname.len - 1]}) catch unreachable; - } else { - host = this.config.hostname; + if (this.config.hostname) |existing| { + const hostname = bun.span(existing); + + if (hostname.len > 2 and hostname[0] == '[') { + // remove "[" and "]" from hostname + host = std.fmt.bufPrintZ(&host_buff, "{s}", .{hostname[1 .. hostname.len - 1]}) catch unreachable; + } else { + host = this.config.hostname; + } } this.ref(); diff --git a/src/deps/uws b/src/deps/uws -Subproject f29c6e24c33483c342bbc83c41cc032f42fbf77 +Subproject 0e9ddbe6944c3ca6b6957e5938d77d8ed9a8181 diff --git a/src/deps/uws.zig b/src/deps/uws.zig index 15e6eaa68..43711e696 100644 --- a/src/deps/uws.zig +++ b/src/deps/uws.zig @@ -285,6 +285,8 @@ pub fn NewSocketHandler(comptime is_ssl: bool) type { ctx: Context, comptime socket_field_name: []const u8, ) ?*Context { + debug("connect({s}, {d})", .{ host, port }); + var stack_fallback = std.heap.stackFallback(1024, bun.default_allocator); var allocator = stack_fallback.get(); var host_ = allocator.dupeZ(u8, host) catch return null; @@ -333,6 +335,7 @@ pub fn NewSocketHandler(comptime is_ssl: bool) type { socket_ctx: *SocketContext, ctx: *anyopaque, ) ?ThisSocket { + debug("connect(unix:{s})", .{path}); var stack_fallback = std.heap.stackFallback(1024, bun.default_allocator); var allocator = stack_fallback.get(); var path_ = allocator.dupeZ(u8, path) catch return null; @@ -356,6 +359,7 @@ pub fn NewSocketHandler(comptime is_ssl: bool) type { socket_ctx: *SocketContext, ptr: *anyopaque, ) ?ThisSocket { + debug("connect({s}, {d})", .{ host, port }); var stack_fallback = std.heap.stackFallback(1024, bun.default_allocator); var allocator = stack_fallback.get(); var host_ = allocator.dupeZ(u8, host) catch return null; @@ -952,9 +956,9 @@ extern fn us_socket_context_on_connect_error(ssl: i32, context: ?*SocketContext, extern fn us_socket_context_on_end(ssl: i32, context: ?*SocketContext, on_end: *const fn (*Socket) callconv(.C) ?*Socket) void; extern fn us_socket_context_ext(ssl: i32, context: ?*SocketContext) ?*anyopaque; -pub extern fn us_socket_context_listen(ssl: i32, context: ?*SocketContext, host: [*c]const u8, port: i32, options: i32, socket_ext_size: i32) ?*ListenSocket; +pub extern fn us_socket_context_listen(ssl: i32, context: ?*SocketContext, host: ?[*:0]const u8, port: i32, options: i32, socket_ext_size: i32) ?*ListenSocket; pub extern fn us_socket_context_listen_unix(ssl: i32, context: ?*SocketContext, path: [*c]const u8, options: i32, socket_ext_size: i32) ?*ListenSocket; -pub extern fn us_socket_context_connect(ssl: i32, context: ?*SocketContext, host: [*c]const u8, port: i32, source_host: [*c]const u8, options: i32, socket_ext_size: i32) ?*Socket; +pub extern fn us_socket_context_connect(ssl: i32, context: ?*SocketContext, host: ?[*:0]const u8, port: i32, source_host: [*c]const u8, options: i32, socket_ext_size: i32) ?*Socket; pub extern fn us_socket_context_connect_unix(ssl: i32, context: ?*SocketContext, path: [*c]const u8, options: i32, socket_ext_size: i32) ?*Socket; pub extern fn us_socket_is_established(ssl: i32, s: ?*Socket) i32; pub extern fn us_socket_close_connecting(ssl: i32, s: ?*Socket) ?*Socket; @@ -1431,6 +1435,10 @@ pub fn NewApp(comptime ssl: bool) type { } return us_socket_local_port(ssl_flag, @as(*uws.Socket, @ptrCast(this))); } + + pub fn socket(this: *@This()) NewSocketHandler(ssl) { + return .{ .socket = @ptrCast(this) }; + } }; pub fn get( @@ -2015,7 +2023,7 @@ extern fn uws_app_listen_with_config( extern fn uws_constructor_failed(ssl: i32, app: *uws_app_t) bool; extern fn uws_num_subscribers(ssl: i32, app: *uws_app_t, topic: [*c]const u8, topic_length: usize) c_uint; extern fn uws_publish(ssl: i32, app: *uws_app_t, topic: [*c]const u8, topic_length: usize, message: [*c]const u8, message_length: usize, opcode: Opcode, compress: bool) bool; -extern fn uws_get_native_handle(ssl: i32, app: *uws_app_t) ?*anyopaque; +extern fn uws_get_native_handle(ssl: i32, app: *anyopaque) ?*anyopaque; extern fn uws_remove_server_name(ssl: i32, app: *uws_app_t, hostname_pattern: [*c]const u8) void; extern fn uws_add_server_name(ssl: i32, app: *uws_app_t, hostname_pattern: [*c]const u8) void; extern fn uws_add_server_name_with_options(ssl: i32, app: *uws_app_t, hostname_pattern: [*c]const u8, options: us_bun_socket_context_options_t) void; diff --git a/src/feature_flags.zig b/src/feature_flags.zig index 0a0c920a4..6c19df844 100644 --- a/src/feature_flags.zig +++ b/src/feature_flags.zig @@ -104,7 +104,7 @@ pub const disable_lolhtml = false; /// on macOS that specifically impacts localhost and not /// other ipv4 hosts. This is a workaround for that. /// "localhost" fails to connect. -pub const hardcode_localhost_to_127_0_0_1 = true; +pub const hardcode_localhost_to_127_0_0_1 = false; /// React doesn't do anything with jsxs /// If the "jsxs" import is development, "jsxs" isn't supported diff --git a/test/js/bun/net/tcp-server.test.ts b/test/js/bun/net/tcp-server.test.ts index 18fa29231..ef8026a6c 100644 --- a/test/js/bun/net/tcp-server.test.ts +++ b/test/js/bun/net/tcp-server.test.ts @@ -33,7 +33,7 @@ it("remoteAddress works", async () => { data() {}, }, port: 0, - hostname: "localhost", + hostname: "127.0.0.1", }); await Bun.connect({ diff --git a/test/js/bun/websocket/websocket-server.test.ts b/test/js/bun/websocket/websocket-server.test.ts index 191bb6f92..dfe23e57f 100644 --- a/test/js/bun/websocket/websocket-server.test.ts +++ b/test/js/bun/websocket/websocket-server.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, afterEach } from "bun:test"; import type { Server, Subprocess, WebSocketHandler } from "bun"; import { serve, spawn } from "bun"; import { bunEnv, bunExe, nodeExe } from "harness"; - +import { isIP } from "node:net"; const strings = [ { label: "string (ascii)", @@ -219,7 +219,7 @@ describe("ServerWebSocket", () => { })); test("remoteAddress", done => ({ open(ws) { - expect(ws.remoteAddress).toMatch(/^(?:[0-9]{1,3}\.){3}[0-9]{1,3}$/); + expect(isIP(ws.remoteAddress)).toBeGreaterThan(0); done(); }, })); |