From 892dc2789657c4161137af191b49bcb7723ea7d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristo=CC=81fer=20R?= Date: Mon, 28 Oct 2024 00:50:24 -0400 Subject: [PATCH 01/10] Make sure a potential port component is considered during hostname validation --- src/termio/stream_handler.zig | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/src/termio/stream_handler.zig b/src/termio/stream_handler.zig index 90a33e8b7..00762bc73 100644 --- a/src/termio/stream_handler.zig +++ b/src/termio/stream_handler.zig @@ -1057,13 +1057,36 @@ pub const StreamHandler = struct { // Get the raw string of the URI. Its unclear to me if the various // tags of this enum guarantee no percent-encoding so we just // check all of it. This isn't a performance critical path. - const host = switch (host_component) { - .raw => |v| v, - .percent_encoded => |v| v, + const host = host: { + const h = switch (host_component) { + .raw => |v| v, + .percent_encoded => |v| v, + }; + if (h.len == 0 or std.mem.eql(u8, "localhost", h)) { + break :host_valid true; + } + + // When the "Private Wi-Fi address" setting is toggled on macOS the hostname + // is set to a string of digits separated by a colon, e.g. '12:34:56:78:90:12'. + // The URI will be parsed as if the last set o digit is a port, so we need to + // make sure that part is included when it's set. + if (uri.port) |port| { + // 65_535 is considered the highest port number on Linux. + const PORT_NUMBER_MAX_DIGITS = 5; + + // Make sure there is space for a max length hostname + the max number of digits. + var host_and_port_buf: [posix.HOST_NAME_MAX + PORT_NUMBER_MAX_DIGITS]u8 = undefined; + + const host_and_port = std.fmt.bufPrint(&host_and_port_buf, "{s}:{d}", .{ h, port }) catch |err| { + log.warn("failed to get full hostname for OSC 7 validation: {}", .{err}); + break :host_valid false; + }; + + break :host host_and_port; + } else { + break :host h; + } }; - if (host.len == 0 or std.mem.eql(u8, "localhost", host)) { - break :host_valid true; - } // Otherwise, it must match our hostname. var buf: [posix.HOST_NAME_MAX]u8 = undefined; From 3a3da82aa98513de45fbc1abd081ea93d420c7b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristo=CC=81fer=20R?= Date: Thu, 31 Oct 2024 18:09:36 -0400 Subject: [PATCH 02/10] Update explanation for number of digits in port number The explanation now refers to RFC 793 instead of just claiming some arbitrary value as truth. The previous value was correct, but now there is a proper source for the correct value. --- src/termio/stream_handler.zig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/termio/stream_handler.zig b/src/termio/stream_handler.zig index 00762bc73..bd2017f89 100644 --- a/src/termio/stream_handler.zig +++ b/src/termio/stream_handler.zig @@ -1071,7 +1071,9 @@ pub const StreamHandler = struct { // The URI will be parsed as if the last set o digit is a port, so we need to // make sure that part is included when it's set. if (uri.port) |port| { - // 65_535 is considered the highest port number on Linux. + // RFC 793 defines port numbers as 16-bit numbers. 5 digits is sufficient to represent + // the maximum since 2^16 - 1 = 65_535. + // See https://www.rfc-editor.org/rfc/rfc793#section-3.1. const PORT_NUMBER_MAX_DIGITS = 5; // Make sure there is space for a max length hostname + the max number of digits. From 3b0a34afbca40694b3b170b7f1cb681e9454435a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristo=CC=81fer=20R?= Date: Thu, 31 Oct 2024 21:55:02 -0400 Subject: [PATCH 03/10] Extract OSC 7 hostname parsing into helper functions --- src/termio/stream_handler.zig | 111 ++++++++++++++++++++-------------- 1 file changed, 65 insertions(+), 46 deletions(-) diff --git a/src/termio/stream_handler.zig b/src/termio/stream_handler.zig index bd2017f89..8db67f66c 100644 --- a/src/termio/stream_handler.zig +++ b/src/termio/stream_handler.zig @@ -1030,6 +1030,48 @@ pub const StreamHandler = struct { self.terminal.markSemanticPrompt(.command); } + pub fn bufPrintHostnameFromFileUri(buf: []u8, uri: std.Uri) ![]const u8 { + // Get the raw string of the URI. Its unclear to me if the various + // tags of this enum guarantee no percent-encoding so we just + // check all of it. This isn't a performance critical path. + const host_component = uri.host orelse return error.NoHostnameInUri; + const host = switch (host_component) { + .raw => |v| v, + .percent_encoded => |v| v, + }; + + // When the "Private Wi-Fi address" setting is toggled on macOS the hostname + // is set to a string of digits separated by a colon, e.g. '12:34:56:78:90:12'. + // The URI will be parsed as if the last set o digit is a port, so we need to + // make sure that part is included when it's set. + if (uri.port) |port| { + var fbs = std.io.fixedBufferStream(buf); + std.fmt.format(fbs.writer().any(), "{s}:{d}", .{ host, port }) catch |err| switch (err) { + error.NoSpaceLeft => return error.NoSpaceLeft, + else => unreachable, + }; + + return fbs.getWritten(); + } + + return host; + } + + pub fn isLocalHostname(hostname: []const u8) !bool { + // A 'localhost' hostname is always considered local. + if (std.mem.eql(u8, "localhost", hostname)) { + return true; + } + + // If hostname is not "localhost" it must match our hostname. + var buf: [posix.HOST_NAME_MAX]u8 = undefined; + const ourHostname = posix.gethostname(&buf) catch |err| { + return err; + }; + + return std.mem.eql(u8, hostname, ourHostname); + } + pub fn reportPwd(self: *StreamHandler, url: []const u8) !void { if (builtin.os.tag == .windows) { log.warn("reportPwd unimplemented on windows", .{}); @@ -1048,57 +1090,34 @@ pub const StreamHandler = struct { return; } + // RFC 793 defines port numbers as 16-bit numbers. 5 digits is sufficient to represent + // the maximum since 2^16 - 1 = 65_535. + // See https://www.rfc-editor.org/rfc/rfc793#section-3.1. + const PORT_NUMBER_MAX_DIGITS = 5; + // Make sure there is space for a max length hostname + the max number of digits. + var host_and_port_buf: [posix.HOST_NAME_MAX + PORT_NUMBER_MAX_DIGITS]u8 = undefined; + + const hostname = bufPrintHostnameFromFileUri(&host_and_port_buf, uri) catch |err| switch (err) { + error.NoHostnameInUri => { + log.warn("OSC 7 uri must contain a hostname: {}", .{err}); + return; + }, + error.NoSpaceLeft => |e| { + log.warn("failed to get full hostname for OSC 7 validation: {}", .{e}); + return; + }, + }; + // OSC 7 is a little sketchy because anyone can send any value from // any host (such an SSH session). The best practice terminals follow // is to valid the hostname to be local. - const host_valid = host_valid: { - const host_component = uri.host orelse break :host_valid false; - - // Get the raw string of the URI. Its unclear to me if the various - // tags of this enum guarantee no percent-encoding so we just - // check all of it. This isn't a performance critical path. - const host = host: { - const h = switch (host_component) { - .raw => |v| v, - .percent_encoded => |v| v, - }; - if (h.len == 0 or std.mem.eql(u8, "localhost", h)) { - break :host_valid true; - } - - // When the "Private Wi-Fi address" setting is toggled on macOS the hostname - // is set to a string of digits separated by a colon, e.g. '12:34:56:78:90:12'. - // The URI will be parsed as if the last set o digit is a port, so we need to - // make sure that part is included when it's set. - if (uri.port) |port| { - // RFC 793 defines port numbers as 16-bit numbers. 5 digits is sufficient to represent - // the maximum since 2^16 - 1 = 65_535. - // See https://www.rfc-editor.org/rfc/rfc793#section-3.1. - const PORT_NUMBER_MAX_DIGITS = 5; - - // Make sure there is space for a max length hostname + the max number of digits. - var host_and_port_buf: [posix.HOST_NAME_MAX + PORT_NUMBER_MAX_DIGITS]u8 = undefined; - - const host_and_port = std.fmt.bufPrint(&host_and_port_buf, "{s}:{d}", .{ h, port }) catch |err| { - log.warn("failed to get full hostname for OSC 7 validation: {}", .{err}); - break :host_valid false; - }; - - break :host host_and_port; - } else { - break :host h; - } - }; - - // Otherwise, it must match our hostname. - var buf: [posix.HOST_NAME_MAX]u8 = undefined; - const hostname = posix.gethostname(&buf) catch |err| { + const host_valid = isLocalHostname(hostname) catch |err| switch (err) { + error.PermissionDenied, error.Unexpected => { log.warn("failed to get hostname for OSC 7 validation: {}", .{err}); - break :host_valid false; - }; - - break :host_valid std.mem.eql(u8, host, hostname); + return; + }, }; + if (!host_valid) { log.warn("OSC 7 host must be local", .{}); return; From 03bb16fcec53ba884c04e725a48fe828b907308d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristo=CC=81fer=20R?= Date: Mon, 4 Nov 2024 16:54:38 -0500 Subject: [PATCH 04/10] Move hostname helpers to src/os/hostname.zig --- src/os/hostname.zig | 44 ++++++++++++++++++++++++++++++++ src/termio/stream_handler.zig | 47 +++-------------------------------- 2 files changed, 47 insertions(+), 44 deletions(-) create mode 100644 src/os/hostname.zig diff --git a/src/os/hostname.zig b/src/os/hostname.zig new file mode 100644 index 000000000..0bed2d547 --- /dev/null +++ b/src/os/hostname.zig @@ -0,0 +1,44 @@ +const std = @import("std"); +const posix = std.posix; + +pub fn bufPrintHostnameFromFileUri(buf: []u8, uri: std.Uri) ![]const u8 { + // Get the raw string of the URI. Its unclear to me if the various + // tags of this enum guarantee no percent-encoding so we just + // check all of it. This isn't a performance critical path. + const host_component = uri.host orelse return error.NoHostnameInUri; + const host = switch (host_component) { + .raw => |v| v, + .percent_encoded => |v| v, + }; + + // When the "Private Wi-Fi address" setting is toggled on macOS the hostname + // is set to a string of digits separated by a colon, e.g. '12:34:56:78:90:12'. + // The URI will be parsed as if the last set o digit is a port, so we need to + // make sure that part is included when it's set. + if (uri.port) |port| { + var fbs = std.io.fixedBufferStream(buf); + std.fmt.format(fbs.writer().any(), "{s}:{d}", .{ host, port }) catch |err| switch (err) { + error.NoSpaceLeft => return error.NoSpaceLeft, + else => unreachable, + }; + + return fbs.getWritten(); + } + + return host; +} + +pub fn isLocalHostname(hostname: []const u8) !bool { + // A 'localhost' hostname is always considered local. + if (std.mem.eql(u8, "localhost", hostname)) { + return true; + } + + // If hostname is not "localhost" it must match our hostname. + var buf: [posix.HOST_NAME_MAX]u8 = undefined; + const ourHostname = posix.gethostname(&buf) catch |err| { + return err; + }; + + return std.mem.eql(u8, hostname, ourHostname); +} diff --git a/src/termio/stream_handler.zig b/src/termio/stream_handler.zig index 8db67f66c..c97c533ea 100644 --- a/src/termio/stream_handler.zig +++ b/src/termio/stream_handler.zig @@ -5,6 +5,7 @@ const xev = @import("xev"); const apprt = @import("../apprt.zig"); const build_config = @import("../build_config.zig"); const configpkg = @import("../config.zig"); +const hostname = @import("../os/hostname.zig"); const renderer = @import("../renderer.zig"); const termio = @import("../termio.zig"); const terminal = @import("../terminal/main.zig"); @@ -1030,48 +1031,6 @@ pub const StreamHandler = struct { self.terminal.markSemanticPrompt(.command); } - pub fn bufPrintHostnameFromFileUri(buf: []u8, uri: std.Uri) ![]const u8 { - // Get the raw string of the URI. Its unclear to me if the various - // tags of this enum guarantee no percent-encoding so we just - // check all of it. This isn't a performance critical path. - const host_component = uri.host orelse return error.NoHostnameInUri; - const host = switch (host_component) { - .raw => |v| v, - .percent_encoded => |v| v, - }; - - // When the "Private Wi-Fi address" setting is toggled on macOS the hostname - // is set to a string of digits separated by a colon, e.g. '12:34:56:78:90:12'. - // The URI will be parsed as if the last set o digit is a port, so we need to - // make sure that part is included when it's set. - if (uri.port) |port| { - var fbs = std.io.fixedBufferStream(buf); - std.fmt.format(fbs.writer().any(), "{s}:{d}", .{ host, port }) catch |err| switch (err) { - error.NoSpaceLeft => return error.NoSpaceLeft, - else => unreachable, - }; - - return fbs.getWritten(); - } - - return host; - } - - pub fn isLocalHostname(hostname: []const u8) !bool { - // A 'localhost' hostname is always considered local. - if (std.mem.eql(u8, "localhost", hostname)) { - return true; - } - - // If hostname is not "localhost" it must match our hostname. - var buf: [posix.HOST_NAME_MAX]u8 = undefined; - const ourHostname = posix.gethostname(&buf) catch |err| { - return err; - }; - - return std.mem.eql(u8, hostname, ourHostname); - } - pub fn reportPwd(self: *StreamHandler, url: []const u8) !void { if (builtin.os.tag == .windows) { log.warn("reportPwd unimplemented on windows", .{}); @@ -1097,7 +1056,7 @@ pub const StreamHandler = struct { // Make sure there is space for a max length hostname + the max number of digits. var host_and_port_buf: [posix.HOST_NAME_MAX + PORT_NUMBER_MAX_DIGITS]u8 = undefined; - const hostname = bufPrintHostnameFromFileUri(&host_and_port_buf, uri) catch |err| switch (err) { + const hostname_from_uri = hostname.bufPrintHostnameFromFileUri(&host_and_port_buf, uri) catch |err| switch (err) { error.NoHostnameInUri => { log.warn("OSC 7 uri must contain a hostname: {}", .{err}); return; @@ -1111,7 +1070,7 @@ pub const StreamHandler = struct { // OSC 7 is a little sketchy because anyone can send any value from // any host (such an SSH session). The best practice terminals follow // is to valid the hostname to be local. - const host_valid = isLocalHostname(hostname) catch |err| switch (err) { + const host_valid = hostname.isLocalHostname(hostname_from_uri) catch |err| switch (err) { error.PermissionDenied, error.Unexpected => { log.warn("failed to get hostname for OSC 7 validation: {}", .{err}); return; From 78abd051a21cf493fb353dd72c651795dbf22401 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristo=CC=81fer=20R?= Date: Mon, 4 Nov 2024 17:13:12 -0500 Subject: [PATCH 05/10] os/hostname: test isLocalHostname --- src/os/hostname.zig | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/os/hostname.zig b/src/os/hostname.zig index 0bed2d547..dcc802b65 100644 --- a/src/os/hostname.zig +++ b/src/os/hostname.zig @@ -42,3 +42,18 @@ pub fn isLocalHostname(hostname: []const u8) !bool { return std.mem.eql(u8, hostname, ourHostname); } + +test "isLocalHostname returns true when provided hostname is localhost" { + try std.testing.expect(try isLocalHostname("localhost")); +} + +test "isLocalHostname returns true when hostname is local" { + var buf: [posix.HOST_NAME_MAX]u8 = undefined; + const localHostname = try posix.gethostname(&buf); + + try std.testing.expect(try isLocalHostname(localHostname)); +} + +test "isLocalHostname returns false when hostname is not local" { + try std.testing.expectEqual(false, try isLocalHostname("not-the-local-hostname")); +} From 9ae6806e30e1d83dfa697ed26bac39db2dbad912 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristo=CC=81fer=20R?= Date: Mon, 4 Nov 2024 17:25:00 -0500 Subject: [PATCH 06/10] os/hostname: test bufPrintHostnameFromFileUri Note that this includes some failing tests because I want to make the uri handling better and more specific. It's a little bit too general right now so I want to lock it down to: 1. macOS only; and 2. valid mac address values because that's how the macOS private Wi-Fi address thing works; randomizes your mac address and sets that as your hostname. --- src/os/hostname.zig | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/src/os/hostname.zig b/src/os/hostname.zig index dcc802b65..31738a90f 100644 --- a/src/os/hostname.zig +++ b/src/os/hostname.zig @@ -43,6 +43,50 @@ pub fn isLocalHostname(hostname: []const u8) !bool { return std.mem.eql(u8, hostname, ourHostname); } +test "bufPrintHostnameFromFileUri succeeds with ascii hostname" { + const uri = try std.Uri.parse("file://localhost/"); + + var buf: [posix.HOST_NAME_MAX]u8 = undefined; + const actual = try bufPrintHostnameFromFileUri(&buf, uri); + + try std.testing.expectEqualStrings("localhost", actual); +} + +test "bufPrintHostnameFromFileUri succeeds with hostname as mac address" { + const uri = try std.Uri.parse("file://12:34:56:78:90:12"); + + var buf: [posix.HOST_NAME_MAX]u8 = undefined; + const actual = try bufPrintHostnameFromFileUri(&buf, uri); + + try std.testing.expectEqualStrings("12:34:56:78:90:12", actual); +} + +test "bufPrintHostnameFromFileUri returns only hostname when there is a port component in the URI" { + // First: try with a non-2-digit port, to test general port handling. + const four_port_uri = try std.Uri.parse("file://has-a-port:1234"); + + var four_port_buf: [posix.HOST_NAME_MAX]u8 = undefined; + const four_port_actual = try bufPrintHostnameFromFileUri(&four_port_buf, four_port_uri); + + try std.testing.expectEqualStrings("has-a-port", four_port_actual); + + // Second: try with a 2-digit port to test mac-address handling. + const two_port_uri = try std.Uri.parse("file://has-a-port:12"); + + var two_port_buf: [posix.HOST_NAME_MAX]u8 = undefined; + const two_port_actual = try bufPrintHostnameFromFileUri(&two_port_buf, two_port_uri); + + try std.testing.expectEqualStrings("has-a-port", two_port_actual); + + // Third: try with a mac-address that has a port-component added to it to test mac-address handling. + const mac_with_port_uri = try std.Uri.parse("file://12:34:56:78:90:12:1234"); + + var mac_with_port_buf: [posix.HOST_NAME_MAX]u8 = undefined; + const mac_with_port_actual = try bufPrintHostnameFromFileUri(&mac_with_port_buf, mac_with_port_uri); + + try std.testing.expectEqualStrings("12:34:56:78:90:12", mac_with_port_actual); +} + test "isLocalHostname returns true when provided hostname is localhost" { try std.testing.expect(try isLocalHostname("localhost")); } From e85b11403145ea01aff8efcbbf5e1480334f5ff6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristo=CC=81fer=20R?= Date: Mon, 4 Nov 2024 18:58:50 -0500 Subject: [PATCH 07/10] os/hostname: add better validation for mac-address hostnames --- src/os/hostname.zig | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/src/os/hostname.zig b/src/os/hostname.zig index 31738a90f..81ef0e6e3 100644 --- a/src/os/hostname.zig +++ b/src/os/hostname.zig @@ -12,10 +12,26 @@ pub fn bufPrintHostnameFromFileUri(buf: []u8, uri: std.Uri) ![]const u8 { }; // When the "Private Wi-Fi address" setting is toggled on macOS the hostname - // is set to a string of digits separated by a colon, e.g. '12:34:56:78:90:12'. - // The URI will be parsed as if the last set o digit is a port, so we need to - // make sure that part is included when it's set. + // is set to a random mac address, e.g. '12:34:56:78:90:ab'. + // The URI will be parsed as if the last set of digits is a port number, so + // we need to make sure that part is included when it's set. + + // We're only interested in special port handling when the current hostname is a + // partial MAC address that's potentially missing the last component. + // If that's not the case we just return the plain URI hostname directly. + // NOTE: This implementation is not sufficient to verify a valid mac address, but + // it's probably sufficient for this specific purpose. + if (host.len != 14 or std.mem.count(u8, host, ":") != 4) { + return host; + } + if (uri.port) |port| { + // If the port is not a 2-digit number we're not looking at a partial MAC-address, + // and instead just a regular port so we return the plain URI hostname. + if (port < 10 or port > 99) { + return host; + } + var fbs = std.io.fixedBufferStream(buf); std.fmt.format(fbs.writer().any(), "{s}:{d}", .{ host, port }) catch |err| switch (err) { error.NoSpaceLeft => return error.NoSpaceLeft, From 9c2f260351f9445133e2f7dde5cef90975cb7268 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kristo=CC=81fer=20R?= Date: Mon, 4 Nov 2024 19:20:09 -0500 Subject: [PATCH 08/10] os/hostname: add and use explicit error structs --- src/os/hostname.zig | 41 +++++++++++++++++++++++++++++++++++------ 1 file changed, 35 insertions(+), 6 deletions(-) diff --git a/src/os/hostname.zig b/src/os/hostname.zig index 81ef0e6e3..e05586167 100644 --- a/src/os/hostname.zig +++ b/src/os/hostname.zig @@ -1,11 +1,21 @@ const std = @import("std"); const posix = std.posix; -pub fn bufPrintHostnameFromFileUri(buf: []u8, uri: std.Uri) ![]const u8 { +const HostnameParsingError = error{ + NoHostnameInUri, + NoSpaceLeft, +}; + +const LocalHostnameValidationError = error{ + PermissionDenied, + Unexpected, +}; + +pub fn bufPrintHostnameFromFileUri(buf: []u8, uri: std.Uri) HostnameParsingError![]const u8 { // Get the raw string of the URI. Its unclear to me if the various // tags of this enum guarantee no percent-encoding so we just // check all of it. This isn't a performance critical path. - const host_component = uri.host orelse return error.NoHostnameInUri; + const host_component = uri.host orelse return HostnameParsingError.NoHostnameInUri; const host = switch (host_component) { .raw => |v| v, .percent_encoded => |v| v, @@ -34,7 +44,7 @@ pub fn bufPrintHostnameFromFileUri(buf: []u8, uri: std.Uri) ![]const u8 { var fbs = std.io.fixedBufferStream(buf); std.fmt.format(fbs.writer().any(), "{s}:{d}", .{ host, port }) catch |err| switch (err) { - error.NoSpaceLeft => return error.NoSpaceLeft, + error.NoSpaceLeft => return HostnameParsingError.NoSpaceLeft, else => unreachable, }; @@ -44,7 +54,7 @@ pub fn bufPrintHostnameFromFileUri(buf: []u8, uri: std.Uri) ![]const u8 { return host; } -pub fn isLocalHostname(hostname: []const u8) !bool { +pub fn isLocalHostname(hostname: []const u8) LocalHostnameValidationError!bool { // A 'localhost' hostname is always considered local. if (std.mem.eql(u8, "localhost", hostname)) { return true; @@ -52,8 +62,9 @@ pub fn isLocalHostname(hostname: []const u8) !bool { // If hostname is not "localhost" it must match our hostname. var buf: [posix.HOST_NAME_MAX]u8 = undefined; - const ourHostname = posix.gethostname(&buf) catch |err| { - return err; + const ourHostname = posix.gethostname(&buf) catch |err| switch (err) { + error.PermissionDenied => return LocalHostnameValidationError.PermissionDenied, + error.Unexpected => return LocalHostnameValidationError.Unexpected, }; return std.mem.eql(u8, hostname, ourHostname); @@ -103,6 +114,24 @@ test "bufPrintHostnameFromFileUri returns only hostname when there is a port com try std.testing.expectEqualStrings("12:34:56:78:90:12", mac_with_port_actual); } +test "bufPrintHostnameFromFileUri returns NoHostnameInUri error when hostname is missing from uri" { + const uri = try std.Uri.parse("file:///"); + + var buf: [posix.HOST_NAME_MAX]u8 = undefined; + const actual = bufPrintHostnameFromFileUri(&buf, uri); + + try std.testing.expectError(HostnameParsingError.NoHostnameInUri, actual); +} + +test "bufPrintHostnameFromFileUri returns NoSpaceLeft error when provided buffer has insufficient size" { + const uri = try std.Uri.parse("file://12:34:56:78:90:12/"); + + var buf: [5]u8 = undefined; + const actual = bufPrintHostnameFromFileUri(&buf, uri); + + try std.testing.expectError(HostnameParsingError.NoSpaceLeft, actual); +} + test "isLocalHostname returns true when provided hostname is localhost" { try std.testing.expect(try isLocalHostname("localhost")); } From 4a263f43afa946047a4f967799af5c12cc0a6501 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 5 Nov 2024 10:30:48 -0800 Subject: [PATCH 09/10] stylistic changes --- src/os/hostname.zig | 80 ++++++++++++++++------------------- src/os/main.zig | 5 +++ src/termio/stream_handler.zig | 17 +++++--- 3 files changed, 53 insertions(+), 49 deletions(-) diff --git a/src/os/hostname.zig b/src/os/hostname.zig index e05586167..6956ed71f 100644 --- a/src/os/hostname.zig +++ b/src/os/hostname.zig @@ -1,22 +1,21 @@ const std = @import("std"); const posix = std.posix; -const HostnameParsingError = error{ +pub const HostnameParsingError = error{ NoHostnameInUri, NoSpaceLeft, }; -const LocalHostnameValidationError = error{ - PermissionDenied, - Unexpected, -}; - -pub fn bufPrintHostnameFromFileUri(buf: []u8, uri: std.Uri) HostnameParsingError![]const u8 { +/// Print the hostname from a file URI into a buffer. +pub fn bufPrintHostnameFromFileUri( + buf: []u8, + uri: std.Uri, +) HostnameParsingError![]const u8 { // Get the raw string of the URI. Its unclear to me if the various // tags of this enum guarantee no percent-encoding so we just // check all of it. This isn't a performance critical path. - const host_component = uri.host orelse return HostnameParsingError.NoHostnameInUri; - const host = switch (host_component) { + const host_component = uri.host orelse return error.NoHostnameInUri; + const host: []const u8 = switch (host_component) { .raw => |v| v, .percent_encoded => |v| v, }; @@ -31,42 +30,42 @@ pub fn bufPrintHostnameFromFileUri(buf: []u8, uri: std.Uri) HostnameParsingError // If that's not the case we just return the plain URI hostname directly. // NOTE: This implementation is not sufficient to verify a valid mac address, but // it's probably sufficient for this specific purpose. - if (host.len != 14 or std.mem.count(u8, host, ":") != 4) { - return host; - } + if (host.len != 14 or std.mem.count(u8, host, ":") != 4) return host; - if (uri.port) |port| { - // If the port is not a 2-digit number we're not looking at a partial MAC-address, - // and instead just a regular port so we return the plain URI hostname. - if (port < 10 or port > 99) { - return host; - } + // If we don't have a port then we can return the hostname as-is because + // it's not a partial MAC-address. + const port = uri.port orelse return host; - var fbs = std.io.fixedBufferStream(buf); - std.fmt.format(fbs.writer().any(), "{s}:{d}", .{ host, port }) catch |err| switch (err) { - error.NoSpaceLeft => return HostnameParsingError.NoSpaceLeft, - else => unreachable, - }; + // If the port is not a 2-digit number we're not looking at a partial + // MAC-address, and instead just a regular port so we return the plain + // URI hostname. + if (port < 10 or port > 99) return host; - return fbs.getWritten(); - } + var fbs = std.io.fixedBufferStream(buf); + try std.fmt.format( + fbs.writer(), + "{s}:{d}", + .{ host, port }, + ); - return host; + return fbs.getWritten(); } +pub const LocalHostnameValidationError = error{ + PermissionDenied, + Unexpected, +}; + +/// Checks if a hostname is local to the current machine. This matches +/// both "localhost" and the current hostname of the machine (as returned +/// by `gethostname`). pub fn isLocalHostname(hostname: []const u8) LocalHostnameValidationError!bool { // A 'localhost' hostname is always considered local. - if (std.mem.eql(u8, "localhost", hostname)) { - return true; - } + if (std.mem.eql(u8, "localhost", hostname)) return true; // If hostname is not "localhost" it must match our hostname. var buf: [posix.HOST_NAME_MAX]u8 = undefined; - const ourHostname = posix.gethostname(&buf) catch |err| switch (err) { - error.PermissionDenied => return LocalHostnameValidationError.PermissionDenied, - error.Unexpected => return LocalHostnameValidationError.Unexpected, - }; - + const ourHostname = try posix.gethostname(&buf); return std.mem.eql(u8, hostname, ourHostname); } @@ -75,7 +74,6 @@ test "bufPrintHostnameFromFileUri succeeds with ascii hostname" { var buf: [posix.HOST_NAME_MAX]u8 = undefined; const actual = try bufPrintHostnameFromFileUri(&buf, uri); - try std.testing.expectEqualStrings("localhost", actual); } @@ -84,7 +82,6 @@ test "bufPrintHostnameFromFileUri succeeds with hostname as mac address" { var buf: [posix.HOST_NAME_MAX]u8 = undefined; const actual = try bufPrintHostnameFromFileUri(&buf, uri); - try std.testing.expectEqualStrings("12:34:56:78:90:12", actual); } @@ -94,7 +91,6 @@ test "bufPrintHostnameFromFileUri returns only hostname when there is a port com var four_port_buf: [posix.HOST_NAME_MAX]u8 = undefined; const four_port_actual = try bufPrintHostnameFromFileUri(&four_port_buf, four_port_uri); - try std.testing.expectEqualStrings("has-a-port", four_port_actual); // Second: try with a 2-digit port to test mac-address handling. @@ -102,7 +98,6 @@ test "bufPrintHostnameFromFileUri returns only hostname when there is a port com var two_port_buf: [posix.HOST_NAME_MAX]u8 = undefined; const two_port_actual = try bufPrintHostnameFromFileUri(&two_port_buf, two_port_uri); - try std.testing.expectEqualStrings("has-a-port", two_port_actual); // Third: try with a mac-address that has a port-component added to it to test mac-address handling. @@ -110,7 +105,6 @@ test "bufPrintHostnameFromFileUri returns only hostname when there is a port com var mac_with_port_buf: [posix.HOST_NAME_MAX]u8 = undefined; const mac_with_port_actual = try bufPrintHostnameFromFileUri(&mac_with_port_buf, mac_with_port_uri); - try std.testing.expectEqualStrings("12:34:56:78:90:12", mac_with_port_actual); } @@ -119,7 +113,6 @@ test "bufPrintHostnameFromFileUri returns NoHostnameInUri error when hostname is var buf: [posix.HOST_NAME_MAX]u8 = undefined; const actual = bufPrintHostnameFromFileUri(&buf, uri); - try std.testing.expectError(HostnameParsingError.NoHostnameInUri, actual); } @@ -128,7 +121,6 @@ test "bufPrintHostnameFromFileUri returns NoSpaceLeft error when provided buffer var buf: [5]u8 = undefined; const actual = bufPrintHostnameFromFileUri(&buf, uri); - try std.testing.expectError(HostnameParsingError.NoSpaceLeft, actual); } @@ -139,10 +131,12 @@ test "isLocalHostname returns true when provided hostname is localhost" { test "isLocalHostname returns true when hostname is local" { var buf: [posix.HOST_NAME_MAX]u8 = undefined; const localHostname = try posix.gethostname(&buf); - try std.testing.expect(try isLocalHostname(localHostname)); } test "isLocalHostname returns false when hostname is not local" { - try std.testing.expectEqual(false, try isLocalHostname("not-the-local-hostname")); + try std.testing.expectEqual( + false, + try isLocalHostname("not-the-local-hostname"), + ); } diff --git a/src/os/main.zig b/src/os/main.zig index 7eed97445..fbe0ac411 100644 --- a/src/os/main.zig +++ b/src/os/main.zig @@ -17,6 +17,7 @@ const resourcesdir = @import("resourcesdir.zig"); // Namespaces pub const args = @import("args.zig"); pub const cgroup = @import("cgroup.zig"); +pub const hostname = @import("hostname.zig"); pub const passwd = @import("passwd.zig"); pub const xdg = @import("xdg.zig"); pub const windows = @import("windows.zig"); @@ -42,3 +43,7 @@ pub const clickInterval = mouse.clickInterval; pub const open = openpkg.open; pub const pipe = pipepkg.pipe; pub const resourcesDir = resourcesdir.resourcesDir; + +test { + @import("std").testing.refAllDecls(@This()); +} diff --git a/src/termio/stream_handler.zig b/src/termio/stream_handler.zig index c97c533ea..2d399e8c1 100644 --- a/src/termio/stream_handler.zig +++ b/src/termio/stream_handler.zig @@ -5,7 +5,7 @@ const xev = @import("xev"); const apprt = @import("../apprt.zig"); const build_config = @import("../build_config.zig"); const configpkg = @import("../config.zig"); -const hostname = @import("../os/hostname.zig"); +const internal_os = @import("../os/main.zig"); const renderer = @import("../renderer.zig"); const termio = @import("../termio.zig"); const terminal = @import("../terminal/main.zig"); @@ -1055,8 +1055,10 @@ pub const StreamHandler = struct { const PORT_NUMBER_MAX_DIGITS = 5; // Make sure there is space for a max length hostname + the max number of digits. var host_and_port_buf: [posix.HOST_NAME_MAX + PORT_NUMBER_MAX_DIGITS]u8 = undefined; - - const hostname_from_uri = hostname.bufPrintHostnameFromFileUri(&host_and_port_buf, uri) catch |err| switch (err) { + const hostname_from_uri = internal_os.hostname.bufPrintHostnameFromFileUri( + &host_and_port_buf, + uri, + ) catch |err| switch (err) { error.NoHostnameInUri => { log.warn("OSC 7 uri must contain a hostname: {}", .{err}); return; @@ -1070,13 +1072,16 @@ pub const StreamHandler = struct { // OSC 7 is a little sketchy because anyone can send any value from // any host (such an SSH session). The best practice terminals follow // is to valid the hostname to be local. - const host_valid = hostname.isLocalHostname(hostname_from_uri) catch |err| switch (err) { - error.PermissionDenied, error.Unexpected => { + const host_valid = internal_os.hostname.isLocalHostname( + hostname_from_uri, + ) catch |err| switch (err) { + error.PermissionDenied, + error.Unexpected, + => { log.warn("failed to get hostname for OSC 7 validation: {}", .{err}); return; }, }; - if (!host_valid) { log.warn("OSC 7 host must be local", .{}); return; From c8b99f78914ec017be3fb425711032c78e8e02a2 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Tue, 5 Nov 2024 10:36:11 -0800 Subject: [PATCH 10/10] remove refalldecls test --- src/os/main.zig | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/os/main.zig b/src/os/main.zig index fbe0ac411..48a712d40 100644 --- a/src/os/main.zig +++ b/src/os/main.zig @@ -43,7 +43,3 @@ pub const clickInterval = mouse.clickInterval; pub const open = openpkg.open; pub const pipe = pipepkg.pipe; pub const resourcesDir = resourcesdir.resourcesDir; - -test { - @import("std").testing.refAllDecls(@This()); -}