xdg: treat empty env vars as not existing (#8830)

Replaces #8786 

The author of the original PR used AI agents to create that PR. To the
extent that this PR borrows code from that PR (mostly in the tests) AI
was used in the creation of this PR.

src/os/env.zig

@@ -95,6 +95,21 @@ pub fn getenv(alloc: Allocator, key: []const u8) Error!?GetEnvResult {
     };
 }
 
+/// Gets the value of an environment variable. Returns null if not found or the
+/// value is empty. This will allocate on Windows but not on other platforms.
+/// The returned value should have deinit called to do the proper cleanup no
+/// matter what platform you are on.
+pub fn getenvNotEmpty(alloc: Allocator, key: []const u8) !?GetEnvResult {
+    const result_ = try getenv(alloc, key);
+    if (result_) |result| {
+        if (result.value.len == 0) {
+            result.deinit(alloc);
+            return null;
+        }
+    }
+    return result_;
+}
+
 pub fn setenv(key: [:0]const u8, value: [:0]const u8) c_int {
     return switch (builtin.os.tag) {
         .windows => c._putenv_s(key.ptr, value.ptr),

src/os/xdg.zig

@@ -7,6 +7,7 @@ const assert = std.debug.assert;
 const Allocator = std.mem.Allocator;
 const posix = std.posix;
 const homedir = @import("homedir.zig");
+const env_os = @import("env.zig");
 
 pub const Options = struct {
     /// Subdirectories to join to the base. This avoids extra allocations
@@ -70,36 +71,22 @@ fn dir(
     // First check the env var. On Windows we have to allocate so this tracks
     // both whether we have the env var and whether we own it.
     // on Windows we treat `LOCALAPPDATA` as a fallback for `XDG_CONFIG_HOME`
-    const env_, const owned = switch (builtin.os.tag) {
-        else => .{ posix.getenv(internal_opts.env), false },
-        .windows => windows: {
-            if (std.process.getEnvVarOwned(alloc, internal_opts.env)) |env| {
-                break :windows .{ env, true };
-            } else |err| switch (err) {
-                error.EnvironmentVariableNotFound => {
-                    if (std.process.getEnvVarOwned(alloc, internal_opts.windows_env)) |env| {
-                        break :windows .{ env, true };
-                    } else |err2| switch (err2) {
-                        error.EnvironmentVariableNotFound => break :windows .{ null, false },
-                        else => return err,
-                    }
-                },
-                else => return err,
-            }
-        },
+    const env_ = try env_os.getenvNotEmpty(alloc, internal_opts.env) orelse switch (builtin.os.tag) {
+        else => null,
+        .windows => try env_os.getenvNotEmpty(alloc, internal_opts.windows_env),
     };
-    defer if (owned) if (env_) |v| alloc.free(v);
+    defer if (env_) |env| env.deinit(alloc);
 
     if (env_) |env| {
         // If we have a subdir, then we use the env as-is to avoid a copy.
         if (opts.subdir) |subdir| {
             return try std.fs.path.join(alloc, &[_][]const u8{
-                env,
+                env.value,
                 subdir,
             });
         }
 
-        return try alloc.dupe(u8, env);
+        return try alloc.dupe(u8, env.value);
     }
 
     // Get our home dir
@@ -169,6 +156,133 @@ test "cache directory paths" {
     }
 }
 
+test "fallback when xdg env empty" {
+    if (builtin.os.tag == .windows) return error.SkipZigTest;
+
+    const alloc = std.testing.allocator;
+
+    const saved_home = home: {
+        const home = std.posix.getenv("HOME") orelse break :home null;
+        break :home try alloc.dupeZ(u8, home);
+    };
+    defer env: {
+        const home = saved_home orelse {
+            _ = env_os.unsetenv("HOME");
+            break :env;
+        };
+        _ = env_os.setenv("HOME", home);
+        std.testing.allocator.free(home);
+    }
+    const temp_home = "/tmp/ghostty-test-home";
+    _ = env_os.setenv("HOME", temp_home);
+
+    const DirCase = struct {
+        name: [:0]const u8,
+        func: fn (Allocator, Options) anyerror![]u8,
+        default_subdir: []const u8,
+    };
+
+    const cases = [_]DirCase{
+        .{ .name = "XDG_CONFIG_HOME", .func = config, .default_subdir = ".config" },
+        .{ .name = "XDG_CACHE_HOME", .func = cache, .default_subdir = ".cache" },
+        .{ .name = "XDG_STATE_HOME", .func = state, .default_subdir = ".local/state" },
+    };
+
+    inline for (cases) |case| {
+        // Save and restore each environment variable
+        const saved_env = blk: {
+            const value = std.posix.getenv(case.name) orelse break :blk null;
+            break :blk try alloc.dupeZ(u8, value);
+        };
+        defer env: {
+            const value = saved_env orelse {
+                _ = env_os.unsetenv(case.name);
+                break :env;
+            };
+            _ = env_os.setenv(case.name, value);
+            alloc.free(value);
+        }
+
+        const expected = try std.fs.path.join(alloc, &[_][]const u8{
+            temp_home,
+            case.default_subdir,
+        });
+        defer alloc.free(expected);
+
+        // Test with empty string - should fallback to home
+        _ = env_os.setenv(case.name, "");
+        const actual = try case.func(alloc, .{});
+        defer alloc.free(actual);
+
+        try std.testing.expectEqualStrings(expected, actual);
+    }
+}
+
+test "fallback when xdg env empty and subdir" {
+    if (builtin.os.tag == .windows) return error.SkipZigTest;
+
+    const env = @import("env.zig");
+    const alloc = std.testing.allocator;
+
+    const saved_home = home: {
+        const home = std.posix.getenv("HOME") orelse break :home null;
+        break :home try alloc.dupeZ(u8, home);
+    };
+    defer env: {
+        const home = saved_home orelse {
+            _ = env.unsetenv("HOME");
+            break :env;
+        };
+        _ = env.setenv("HOME", home);
+        std.testing.allocator.free(home);
+    }
+
+    const temp_home = "/tmp/ghostty-test-home";
+    _ = env.setenv("HOME", temp_home);
+
+    const DirCase = struct {
+        name: [:0]const u8,
+        func: fn (Allocator, Options) anyerror![]u8,
+        default_subdir: []const u8,
+    };
+
+    const cases = [_]DirCase{
+        .{ .name = "XDG_CONFIG_HOME", .func = config, .default_subdir = ".config" },
+        .{ .name = "XDG_CACHE_HOME", .func = cache, .default_subdir = ".cache" },
+        .{ .name = "XDG_STATE_HOME", .func = state, .default_subdir = ".local/state" },
+    };
+
+    inline for (cases) |case| {
+        // Save and restore each environment variable
+        const saved_env = blk: {
+            const value = std.posix.getenv(case.name) orelse break :blk null;
+            break :blk try alloc.dupeZ(u8, value);
+        };
+        defer env: {
+            const value = saved_env orelse {
+                _ = env.unsetenv(case.name);
+                break :env;
+            };
+            _ = env.setenv(case.name, value);
+            alloc.free(value);
+        }
+
+        const expected = try std.fs.path.join(alloc, &[_][]const u8{
+            temp_home,
+            case.default_subdir,
+            "ghostty",
+        });
+        defer alloc.free(expected);
+
+        // Test with empty string - should fallback to home
+        _ = env.setenv(case.name, "");
+        const actual = try case.func(alloc, .{ .subdir = "ghostty" });
+        defer alloc.free(actual);
+
+        try std.testing.expectEqualStrings(expected, actual);
+    }
+}
+
 test parseTerminalExec {
     const testing = std.testing;