fix ipv6, @domain, mmdb byte swaps, geocache location

build.zig

@@ -105,6 +105,18 @@ pub fn build(b: *std.Build) void {
         // With a byte array both sides agree (40 / 8). Nothing here reads
         // uint128-typed database fields, so this only affects layout.
         .MMDB_UINT128_IS_BYTE_ARRAY = 1,
+        // Tell libmaxminddb the target's byte order.
+        //
+        // Database floats and doubles are stored big-endian, and
+        // `get_ieee754_double` only reverses them when this is set; otherwise it
+        // memcpy's the raw bytes and returns a nonsense value on a little-endian
+        // host. Autotools normally sets this, so a hand-written config header
+        // that omits it silently produces garbage coordinates.
+        //
+        // Getting this right in the C layer means callers do not have to
+        // compensate, which they previously did with a byteswap that was correct
+        // only by accident on little-endian targets and wrong on big-endian ones.
+        .MMDB_LITTLE_ENDIAN = @as(u8, if (target.result.cpu.arch.endian() == .little) 1 else 0),
     });
 
     maxminddb.root_module.addConfigHeader(maxminddb_config);

src/Config.zig

@@ -103,7 +103,17 @@ pub fn load(allocator: std.mem.Allocator, env: *const std.process.Environ.Map) !
                 24;
             break :blk hours * std.time.s_per_hour;
         },
-        .geocache_file = if (env.get("WTTR_GEOCACHE_FILE")) |v| try allocator.dupe(u8, v) else try std.fs.path.join(allocator, &[_][]const u8{ default_cache_dir, "geocache.json" }),
+        // Derived from WTTR_CACHE_DIR like every other cache path. It previously
+        // ignored it and always used the default location, so pointing the server
+        // at a different cache directory silently left this one file behind.
+        .geocache_file = blk: {
+            if (env.get("WTTR_GEOCACHE_FILE")) |v| {
+                break :blk try allocator.dupe(u8, v);
+            }
+            break :blk try std.fmt.allocPrint(allocator, "{s}/geocache.json", .{
+                env.get("WTTR_CACHE_DIR") orelse default_cache_dir,
+            });
+        },
         .geoip_fallback = blk: {
             if (env.get("WTTR_GEOIP_FALLBACK")) |v| {
                 if (std.mem.eql(u8, v, "ip2location")) break :blk .ip2location;

src/location/GeoIp.zig

@@ -260,7 +260,11 @@ pub fn isUSIp(self: *GeoIP, ip: []const u8) bool {
 
 /// Maximum accuracy radius (in km) to trust from GeoLite2. Entries with a
 /// radius above this are too coarse for weather lookups (e.g. backbone/transit
-/// IPs that MaxMind maps to the wrong city) and should fall back to IP2Location.
+/// IPs that MaxMind maps to the wrong city) and should fall back to the
+/// configured online provider.
+///
+/// Note this only catches entries GeoLite2 admits are vague. An entry can be
+/// precise *and* wrong, which is what pins exist for.
 const max_accuracy_radius_km = 200;
 
 fn extractCoordinates(self: *GeoIP, ip: []const u8, result: c.MMDB_lookup_result_s) ?Location {
@@ -268,15 +272,15 @@ fn extractCoordinates(self: *GeoIP, ip: []const u8, result: c.MMDB_lookup_result
 
     var entry_copy = result.entry;
 
-    // Check accuracy_radius first -- reject low-confidence entries so we
+    // Check accuracy_radius first -- reject low-confidence entries so we fall
-    // fall back to the IP2Location online lookup instead.
+    // back to the configured online provider instead.
     // SAFETY: accuracy_data set by MMDB_get_value
     var accuracy_data: c.MMDB_entry_data_s = undefined;
     const acc_status = c.MMDB_get_value(&entry_copy, &accuracy_data, "location", "accuracy_radius", @as([*c]const u8, null));
     if (acc_status == c.MMDB_SUCCESS and accuracy_data.has_data) {
         const radius = accuracy_data.unnamed_0.uint16;
         if (radius > max_accuracy_radius_km) {
-            log.info("GeoLite2 accuracy_radius for ip {s} is {d} km (>{d} km threshold), falling back to IP2Location", .{ ip, radius, max_accuracy_radius_km });
+            log.info("GeoLite2 accuracy_radius for ip {s} is {d} km (>{d} km threshold), falling back to the online provider", .{ ip, radius, max_accuracy_radius_km });
             return null;
         }
     }
@@ -301,16 +305,13 @@ fn extractCoordinates(self: *GeoIP, ip: []const u8, result: c.MMDB_lookup_result
         return null;
     }
 
-    var coords = [_]f64{ latitude_data.unnamed_0.double_value, longitude_data.unnamed_0.double_value };
+    // No byte swapping here: `build.zig` sets `MMDB_LITTLE_ENDIAN` for the target,
-
+    // so libmaxminddb converts the database's big-endian doubles to host order in
-    // Depending on how this is compiled, the byteswap may or may not be necessary
+    // `get_ieee754_double` before we ever see them. If that define were ever
-    // original c, compiled with zig, statically linked: byteSwap
+    // dropped, these coordinates would silently become nonsense, which the
-    // pre=built, dynamically linked, do not byte swap
+    // "lookup works" test below is positioned to catch.
-    // I'm not sure precisely what causes this
+    const latitude = latitude_data.unnamed_0.double_value;
-    std.mem.byteSwapAllElements(f64, &coords);
+    const longitude = longitude_data.unnamed_0.double_value;
-
-    const latitude = coords[0];
-    const longitude = coords[1];
 
     // Extract location name parts
     // SAFETY: value set by MMDB_get_value

src/location/resolver.zig

@@ -3,6 +3,7 @@ const GeoIp = @import("GeoIp.zig");
 const GeoCache = @import("GeoCache.zig");
 const Airports = @import("Airports.zig");
 const Coordinates = @import("../Coordinates.zig");
+const packIp = @import("Ip2location.zig").packIp;
 
 const log = std.log.scoped(.resolver);
 
@@ -141,7 +142,7 @@ pub const Resolver = struct {
             switch (result) {
                 .address => |addr| {
                     var buf: [64]u8 = undefined;
-                    const ip_str = try std.fmt.bufPrint(&buf, "{f}", .{addr});
+                    const ip_str = try formatAddressBare(addr, &buf);
                     return self.resolveIP(ip_str);
                 },
                 .canonical_name => continue,
@@ -258,6 +259,27 @@ pub const Resolver = struct {
         return self.resolveGeocoded(code);
     }
 
+    /// Formats an address with no port and no brackets.
+    ///
+    /// Every formatter in `std.Io.net` appends ":port", and the IPv6 one also
+    /// brackets the address. Feeding that to `resolveIP` produced strings like
+    /// "1.2.3.4:0", which are not addresses, so *every* `@domain` lookup failed
+    /// with LocationNotFound.
+    fn formatAddressBare(addr: std.Io.net.IpAddress, buf: []u8) ![]const u8 {
+        return switch (addr) {
+            .ip4 => |a| std.fmt.bufPrint(buf, "{d}.{d}.{d}.{d}", .{
+                a.bytes[0], a.bytes[1], a.bytes[2], a.bytes[3],
+            }),
+            .ip6 => |a| blk: {
+                const bare: std.Io.net.Ip6Address.Unresolved = .{
+                    .bytes = a.bytes,
+                    .interface_name = null,
+                };
+                break :blk std.fmt.bufPrint(buf, "{f}", .{&bare});
+            },
+        };
+    }
+
     fn isAlpha(s: []const u8) bool {
         for (s) |c| {
             if (!std.ascii.isAlphabetic(c)) return false;
@@ -265,17 +287,17 @@ pub const Resolver = struct {
         return true;
     }
 
+    /// Whether `s` is an address we can look up.
+    ///
+    /// This counted dots and required exactly three, so it only ever recognized
+    /// IPv4. An IPv6 client address fell through to `.city_name` and was sent to
+    /// the geocoder as a place name, e.g. asking Nominatim to find a town called
+    /// "2001:4860:4860::8888".
+    ///
+    /// Deferring to `packIp` keeps this in step with what the rest of the
+    /// pipeline accepts, including IPv4-mapped IPv6 forms.
     fn isIPAddress(s: []const u8) bool {
-        // Simple check for IPv4
+        return packIp(s) != null;
-        var dots: u8 = 0;
-        for (s) |c| {
-            if (c == '.') {
-                dots += 1;
-            } else if (!std.ascii.isDigit(c)) {
-                return false;
-            }
-        }
-        return dots == 3;
     }
 };
 
@@ -284,12 +306,54 @@ test "detect IP address" {
     try std.testing.expect(!Resolver.isIPAddress("not.an.ip"));
 }
 
+test "detect IP address: IPv6 forms" {
+    // Previously these were not recognized as addresses at all, so a v6 client
+    // was handed to the geocoder as though it were the name of a place.
+    try std.testing.expect(Resolver.isIPAddress("2001:4860:4860::8888"));
+    try std.testing.expect(Resolver.isIPAddress("::1"));
+    try std.testing.expect(Resolver.isIPAddress("fe80::1"));
+    try std.testing.expect(Resolver.isIPAddress("::ffff:12.94.132.170"));
+}
+
+test "detect IP address: rejects things that only look numeric" {
+    try std.testing.expect(!Resolver.isIPAddress("1.2.3"));
+    try std.testing.expect(!Resolver.isIPAddress("1.2.3.4.5"));
+    try std.testing.expect(!Resolver.isIPAddress("999.1.1.1"));
+    try std.testing.expect(!Resolver.isIPAddress("London"));
+    try std.testing.expect(!Resolver.isIPAddress(""));
+    // A range is not a query; pins parse CIDR, lookups do not.
+    try std.testing.expect(!Resolver.isIPAddress("12.94.132.0/24"));
+}
+
+test "formatAddressBare omits the port and brackets" {
+    var buf: [64]u8 = undefined;
+
+    const v4 = try std.Io.net.IpAddress.parse("12.94.132.170", 0);
+    try std.testing.expectEqualStrings("12.94.132.170", try Resolver.formatAddressBare(v4, &buf));
+
+    const v6 = try std.Io.net.IpAddress.parse("2001:4860:4860::8888", 0);
+    try std.testing.expectEqualStrings("2001:4860:4860::8888", try Resolver.formatAddressBare(v6, &buf));
+}
+
+test "formatAddressBare output is accepted back as an address" {
+    var buf: [64]u8 = undefined;
+    // The round trip is the property that matters: `resolveIP` re-parses this
+    // string, and the old formatting produced "1.2.3.4:0", which it rejected.
+    for ([_][]const u8{ "8.8.8.8", "2001:4860:4860::8888", "::1" }) |text| {
+        const addr = try std.Io.net.IpAddress.parse(text, 0);
+        const formatted = try Resolver.formatAddressBare(addr, &buf);
+        try std.testing.expect(Resolver.isIPAddress(formatted));
+    }
+}
+
 test "detect location type" {
     try std.testing.expectEqual(LocationType.ip_address, Resolver.detectType("8.8.8.8"));
     try std.testing.expectEqual(LocationType.domain_name, Resolver.detectType("@github.com"));
     try std.testing.expectEqual(LocationType.special_location, Resolver.detectType("~Eiffel+Tower"));
     try std.testing.expectEqual(LocationType.airport_code, Resolver.detectType("muc"));
     try std.testing.expectEqual(LocationType.city_name, Resolver.detectType("London"));
+    try std.testing.expectEqual(LocationType.ip_address, Resolver.detectType("2001:4860:4860::8888"));
+    try std.testing.expectEqual(LocationType.ip_address, Resolver.detectType("::1"));
 }
 
 test "resolver init" {