From cbabb810ee12cb503deb561b9031756614c210af Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Tue, 11 Aug 2026 17:43:46 -0700 Subject: [PATCH] pin us/non-us as well for imperial/metric default --- src/cli/pin.zig | 19 ++- src/http/handler.zig | 98 +++++++++++- src/location/GeoCache.zig | 90 +++++++++++ src/location/GeoIp.zig | 288 +++++++++++++++++++++++++++++++++-- src/location/Ip2location.zig | 131 ++++++++++++++++ src/location/IpWhoIs.zig | 6 +- src/location/Pins.zig | 263 +++++++++++++++++++++++++++++--- src/location/resolver.zig | 94 +++++++++++- 8 files changed, 948 insertions(+), 41 deletions(-) diff --git a/src/cli/pin.zig b/src/cli/pin.zig index fd0198d..6729b93 100644 --- a/src/cli/pin.zig +++ b/src/cli/pin.zig @@ -109,7 +109,7 @@ pub fn runPin( var pins = try Pins.load(allocator, io, cfg.pins_file); defer pins.deinit(); - try pins.put(cidr, location.name, location.coords); + try pins.put(cidr, location.name, location.coords, location.iso_country); try pins.save(io, cfg.pins_file); var out_buf: [512]u8 = undefined; @@ -121,6 +121,13 @@ pub fn runPin( location.coords.latitude, location.coords.longitude, }); + // Surfaced because it decides units: without a country the pinned client + // falls back to whatever the database says about its address. + if (location.iso_country) |iso| { + try w.print("country {s} (units follow this)\n", .{iso[0..]}); + } else { + try w.writeAll("country unknown (units will follow the GeoLite2 entry for the address)\n"); + } try w.print("wrote {s}\n", .{cfg.pins_file}); try reportSignal(w, notifyServers(io)); try w.flush(); @@ -188,8 +195,14 @@ pub fn runList(allocator: std.mem.Allocator, io: std.Io, cfg: Config) !u8 { .{ .family = e.family, .network = e.network, .prefix_len = e.prefix_len }, &cidr_buf, ); - try w.print("{s: <20} {s} ({d:.4}, {d:.4})\n", .{ - text, e.name, e.coords.latitude, e.coords.longitude, + // The country is shown because it is what decides units, and a pin + // written before it was recorded shows "--" rather than looking complete. + try w.print("{s: <20} {s: <4} {s} ({d:.4}, {d:.4})\n", .{ + text, + if (e.iso_country) |*iso| iso[0..] else "--", + e.name, + e.coords.latitude, + e.coords.longitude, }); } try w.flush(); diff --git a/src/http/handler.zig b/src/http/handler.zig index d9fa42c..989b4e7 100644 --- a/src/http/handler.zig +++ b/src/http/handler.zig @@ -147,7 +147,11 @@ fn handleWeatherInternal( var render_options = params.render_options; // Determine if imperial units should be used - // Priority: explicit ?u or ?m > lang=us > US IP > default metric + // Priority: explicit ?u or ?m > lang=us > US client address > default metric + // + // The address is resolved through the full pin -> database -> fallback-cache + // chain, so an operator's pin decides units as well as location. It used to + // read the database directly, which rendered a pinned US client in metric. if (params.use_imperial == null) { // User did not ask for anything explicitly @@ -528,3 +532,95 @@ test "handler: format line 3" { try std.testing.expect(std.mem.indexOf(u8, pr.body, "California, United States:") != null); try std.testing.expect(std.mem.indexOf(u8, pr.body, "°C") != null); } + +test "handler: a pin's country selects units, not the database" { + const allocator = std.testing.allocator; + const MockHarness = @import("Server.zig").MockHarness; + const Pins = @import("../location/Pins.zig"); + const Location = @import("../location/resolver.zig").Location; + + var harness = try MockHarness.init(allocator); + defer harness.deinit(); + + // GeoLite2 places this AT&T address in Texas, so the database says US. The + // operator says London. Units have to follow the operator. + try harness.geoip.pins.put( + try Pins.parseCidr("12.94.132.0/24"), + "London, United Kingdom", + .{ .latitude = 51.5074, .longitude = -0.1278 }, + Location.isoFrom("GB"), + ); + + var ht = httpz.testing.init(.{}); + defer ht.deinit(); + + ht.url("/12.94.132.170?format=1"); + ht.param("location", "12.94.132.170"); + + try handleWeather(&harness.opts, ht.req, ht.res, "12.94.132.170"); + + try ht.expectStatus(200); + // Metric. Before unit selection consulted pins this rendered in Fahrenheit, + // because it read the database directly and the database still says US. + try ht.expectBody("☀️ +20°C\n"); +} + +test "handler: a US pin selects imperial where the database has no entry" { + const allocator = std.testing.allocator; + const MockHarness = @import("Server.zig").MockHarness; + const Pins = @import("../location/Pins.zig"); + const Location = @import("../location/resolver.zig").Location; + + var harness = try MockHarness.init(allocator); + defer harness.deinit(); + + // TEST-NET-3 is absent from GeoLite2, so without the pin there is no country + // and the client would fall back to metric despite being pinned to the US. + try harness.geoip.pins.put( + try Pins.parseCidr("203.0.113.0/24"), + "Seattle, Washington, United States", + .{ .latitude = 47.6, .longitude = -122.3 }, + Location.isoFrom("US"), + ); + + var ht = httpz.testing.init(.{}); + defer ht.deinit(); + + ht.url("/203.0.113.7?format=1"); + ht.param("location", "203.0.113.7"); + + try handleWeather(&harness.opts, ht.req, ht.res, "203.0.113.7"); + + try ht.expectStatus(200); + try ht.expectBody("☀️ +68°F\n"); +} + +test "handler: an explicit unit request still overrides the pin's country" { + const allocator = std.testing.allocator; + const MockHarness = @import("Server.zig").MockHarness; + const Pins = @import("../location/Pins.zig"); + const Location = @import("../location/resolver.zig").Location; + + var harness = try MockHarness.init(allocator); + defer harness.deinit(); + + try harness.geoip.pins.put( + try Pins.parseCidr("203.0.113.0/24"), + "Seattle, Washington, United States", + .{ .latitude = 47.6, .longitude = -122.3 }, + Location.isoFrom("US"), + ); + + var ht = httpz.testing.init(.{}); + defer ht.deinit(); + + // `?m` has to keep winning: the country is only consulted when the caller + // expressed no preference. + ht.url("/203.0.113.7?format=1&m"); + ht.param("location", "203.0.113.7"); + + try handleWeather(&harness.opts, ht.req, ht.res, "203.0.113.7"); + + try ht.expectStatus(200); + try ht.expectBody("☀️ +20°C\n"); +} diff --git a/src/location/GeoCache.zig b/src/location/GeoCache.zig index 69c1272..957d4f1 100644 --- a/src/location/GeoCache.zig +++ b/src/location/GeoCache.zig @@ -1,5 +1,6 @@ const std = @import("std"); const Coordinates = @import("../Coordinates.zig"); +const Location = @import("resolver.zig").Location; const srf = @import("srf"); const GeoCache = @This(); @@ -17,6 +18,10 @@ last_save: i64, pub const CachedLocation = struct { name: []const u8, coords: Coordinates, + /// ISO 3166-1 alpha-2 country code, upper case, when the geocoder reported + /// one. Cached so that `wttr pin ` records the country even + /// when the place name is answered from here rather than from Nominatim. + iso_country: ?[2]u8 = null, }; pub fn init(allocator: std.mem.Allocator, io: std.Io, cache_file: ?[]const u8) !GeoCache { @@ -66,6 +71,7 @@ pub fn put(self: *GeoCache, query: []const u8, location: CachedLocation) !void { const value = CachedLocation{ .name = try self.allocator.dupe(u8, location.name), .coords = location.coords, + .iso_country = location.iso_country, }; try self.cache.put(key, value); self.dirty = true; @@ -96,11 +102,17 @@ pub fn saveIfNeeded(self: *GeoCache) void { /// /// The query is stored alongside the result rather than used as an object key, /// because SRF records are flat field lists rather than a nested map. +/// +/// `iso` defaults to null so files written before it existed still load: SRF +/// fills a missing field from the Zig default and only errors when there is +/// none. It is also omitted on write when null, so unknown countries add +/// nothing to the file. const Record = struct { query: []const u8, name: []const u8, lat: f64, lon: f64, + iso: ?[]const u8 = null, }; fn load(allocator: std.mem.Allocator, cache: *std.StringHashMap(CachedLocation), content: []const u8) !void { @@ -129,6 +141,7 @@ fn load(allocator: std.mem.Allocator, cache: *std.StringHashMap(CachedLocation), const existing = try cache.fetchPut(cache_key, .{ .name = name_copy, .coords = .{ .latitude = record.lat, .longitude = record.lon }, + .iso_country = if (record.iso) |iso| Location.isoFrom(iso) else null, }); if (existing) |old| { allocator.free(old.key); @@ -159,6 +172,10 @@ fn save(self: *GeoCache, writer: *std.Io.Writer) !void { .name = entry.value_ptr.name, .lat = entry.value_ptr.coords.latitude, .lon = entry.value_ptr.coords.longitude, + // Borrowed from the map rather than copied: the map is not touched + // between here and the `print` below, and a by-value capture would + // leave the slice pointing at a dead loop temporary. + .iso = if (entry.value_ptr.iso_country) |*iso| iso[0..] else null, }; } @@ -327,3 +344,76 @@ test "save and load round-trip" { try std.testing.expectApproxEqAbs(@as(f64, 52.5200), berlin.?.coords.latitude, 0.0001); try std.testing.expectApproxEqAbs(@as(f64, 13.4050), berlin.?.coords.longitude, 0.0001); } + +test "save and load round-trip the country" { + const allocator = std.testing.allocator; + var cache1 = try GeoCache.init(allocator, std.testing.io, null); + defer cache1.deinit(); + + try cache1.put("Seattle,+Washington", .{ + .name = "Seattle, King County, Washington, United States", + .coords = .{ .latitude = 47.6038321, .longitude = -122.330062 }, + .iso_country = Location.isoFrom("us"), + }); + // Kept as unknown rather than acquiring a country on the round trip. + try cache1.put("Nowhere", .{ + .name = "Nowhere", + .coords = .{ .latitude = 1, .longitude = 2 }, + }); + + var buffer: [2048]u8 = undefined; + var writer = std.Io.Writer.fixed(&buffer); + try cache1.save(&writer); + + var cache2 = std.StringHashMap(CachedLocation).init(allocator); + defer { + var it = cache2.iterator(); + while (it.next()) |entry| { + allocator.free(entry.key_ptr.*); + allocator.free(entry.value_ptr.name); + } + cache2.deinit(); + } + + try load(allocator, &cache2, buffer[0..writer.end]); + + const seattle = cache2.get("Seattle,+Washington") orelse return error.TestUnexpectedResult; + // Normalized on the way in, so it reads back upper case even though + // Nominatim reports "us". + try std.testing.expectEqualStrings("US", &(seattle.iso_country.?)); + + const nowhere = cache2.get("Nowhere") orelse return error.TestUnexpectedResult; + try std.testing.expect(nowhere.iso_country == null); +} + +test "load: a geocache written before the country field still loads" { + const allocator = std.testing.allocator; + var cache_map = std.StringHashMap(CachedLocation).init(allocator); + defer { + var it = cache_map.iterator(); + while (it.next()) |entry| { + allocator.free(entry.key_ptr.*); + allocator.free(entry.value_ptr.name); + } + cache_map.deinit(); + } + + // The four-field shape. These entries are re-geocodable, but discarding them + // would mean a burst of Nominatim traffic on the first restart after upgrade. + const content = + \\#!srfv1 + \\#!long + \\query::London + \\name::London, UK + \\lat:num:51.5074 + \\lon:num:-0.1278 + \\ + ; + + try load(allocator, &cache_map, content); + + try std.testing.expectEqual(@as(usize, 1), cache_map.count()); + const london = cache_map.get("London") orelse return error.TestUnexpectedResult; + try std.testing.expectEqualStrings("London, UK", london.name); + try std.testing.expect(london.iso_country == null); +} diff --git a/src/location/GeoIp.zig b/src/location/GeoIp.zig index 4c21fa3..e9e48a2 100644 --- a/src/location/GeoIp.zig +++ b/src/location/GeoIp.zig @@ -41,6 +41,19 @@ const FallbackClient = union(enum) { }; } + /// Country code for an address already in the provider's cache. + /// + /// Cache-only by design: unit selection must not be able to trigger an + /// outbound request, and the cache is permanent, so anything previously + /// resolved is still there. + fn cachedCountry(self: FallbackClient, ip: []const u8) ?[2]u8 { + const packed_ip = Ip2location.packIp(ip) orelse return null; + return switch (self) { + .ip2location => |client| client.cache.getCountry(packed_ip.key), + .ipwhois => |client| client.cache.getCountry(packed_ip.key), + }; + } + fn deinit(self: FallbackClient, allocator: std.mem.Allocator) void { switch (self) { .ip2location => |client| { @@ -237,25 +250,61 @@ fn lookupInternal(mmdb: *c.MMDB_s, ip: []const u8) !c.MMDB_lookup_result_s { } pub fn isUSIp(self: *GeoIP, ip: []const u8) bool { - // `country_data` borrows from the mapped database via `result.entry`, so the - // whole read stays inside the shared lock. - self.lock.lockSharedUncancelable(self.io); - defer self.lock.unlockShared(self.io); + const iso = self.lookupCountry(ip) orelse return false; + return std.mem.eql(u8, &iso, "US"); +} - var result = lookupInternal(self.mmdb, ip) catch return false; - if (!result.found_entry) return false; +/// ISO 3166-1 alpha-2 country code for an address, for unit selection. +/// +/// Walks the same precedence as `lookup` -- pins, then the database, then the +/// online provider's cache -- so both an operator's pin and a previously fetched +/// fallback answer influence which units a client sees. This previously read the +/// database directly, which meant a pinned or fallback-resolved client could be +/// rendered in metric while the location it was given was in the US. +/// +/// Never performs a live fallback fetch. Units are not worth an outbound request +/// on their own, and for a request with no explicit location the resolution in +/// `handleWeather` has already populated the cache by the time this runs. The +/// residual case -- an explicit location query from an address seen for the first +/// time, whose only answer would come from the network -- gets the default units +/// once and the right ones on every later request. +/// +/// A matching pin with no recorded country falls through to the database rather +/// than answering "unknown", so pins written before the country was stored keep +/// behaving exactly as they did. +pub fn lookupCountry(self: *GeoIP, ip: []const u8) ?[2]u8 { + { + // Covers `pins` and `mmdb` only. The cache read below is left outside for + // the same reason `lookup` calls the fallback outside the lock: it is not + // guarded by this lock on the write side either. + self.lock.lockSharedUncancelable(self.io); + defer self.lock.unlockShared(self.io); - // SAFETY: The C API will initialize this on the next line - var country_data: c.MMDB_entry_data_s = undefined; - const status = c.MMDB_get_value(&result.entry, &country_data, "country", "iso_code", @as([*c]const u8, null)); + if (self.pins.lookupCountry(ip)) |iso| return iso; - if (status != c.MMDB_SUCCESS or !country_data.has_data) { - log.info("lookup found result, but no country available in data for ip {s}. MMDB_get_value returned {d}", .{ ip, status }); - return false; + if (lookupInternal(self.mmdb, ip)) |result| { + if (result.found_entry) { + if (readIso(result)) |iso| return iso; + } + } else |_| {} } - const country_code = country_data.unnamed_0.utf8_string[0..country_data.data_size]; - return std.mem.eql(u8, country_code, "US"); + return self.fallback_client.cachedCountry(ip); +} + +/// Reads `country/iso_code` from a lookup result. +/// +/// Shared by `lookupCountry` and `extractCoordinates` so there is one place that +/// knows where the country code lives and how it is validated. +fn readIso(result: c.MMDB_lookup_result_s) ?[2]u8 { + var entry_copy = result.entry; + + // SAFETY: iso_data is set by MMDB_get_value + var iso_data: c.MMDB_entry_data_s = undefined; + const status = c.MMDB_get_value(&entry_copy, &iso_data, "country", "iso_code", @as([*c]const u8, null)); + if (status != c.MMDB_SUCCESS or !iso_data.has_data) return null; + + return Location.isoFrom(iso_data.unnamed_0.utf8_string[0..iso_data.data_size]); } /// Maximum accuracy radius (in km) to trust from GeoLite2. Entries with a @@ -341,6 +390,11 @@ fn extractCoordinates(self: *GeoIP, ip: []const u8, result: c.MMDB_lookup_result else ""; + // The ISO code as well as the display name: unit selection needs the country + // itself, and deriving it by matching the name against "United States" would + // break on any of the other languages the database carries. + const iso_country = readIso(result); + const final_name = Location.buildDisplayName(self.allocator, city, subdivision, country, ip); return .{ @@ -350,6 +404,7 @@ fn extractCoordinates(self: *GeoIP, ip: []const u8, result: c.MMDB_lookup_result .latitude = latitude, .longitude = longitude, }, + .iso_country = iso_country, }; } @@ -511,6 +566,7 @@ test "a pin overrides a confident but wrong database answer" { try Pins.parseCidr("12.94.132.0/24"), "San Francisco, California, United States", .{ .latitude = 37.7749, .longitude = -122.4194 }, + Location.isoFrom("US"), ); try pins.save(io, pins_path); } @@ -553,7 +609,7 @@ test "reloadPins picks up a pin written after startup" { { var pins: Pins = .init(allocator); defer pins.deinit(); - try pins.put(try Pins.parseCidr("203.0.113.0/24"), "Somewhere", .{ .latitude = 1, .longitude = 2 }); + try pins.put(try Pins.parseCidr("203.0.113.0/24"), "Somewhere", .{ .latitude = 1, .longitude = 2 }, null); try pins.save(io, pins_path); } @@ -564,3 +620,205 @@ test "reloadPins picks up a pin written after startup" { defer hit.deinit(); try std.testing.expectEqualStrings("Somewhere", hit.name); } + +/// Shared setup for the `lookupCountry` tests. +/// +/// Both the pins file and the fallback cache are redirected into a temp +/// directory. The cache in particular is permanent and append-only, so a test +/// left pointing at the configured path would write entries into the developer's +/// real cache that nothing ever removes. +fn testConfigIn(allocator: std.mem.Allocator, io: std.Io, dir: []const u8) !Config { + var config = try Config.loadForTest(allocator); + errdefer config.deinit(allocator); + + if (@import("build_options").download_geoip) { + const GeoLite2 = @import("GeoLite2.zig"); + try GeoLite2.ensureDatabase(allocator, io, config.geolite_path); + } + + allocator.free(config.pins_file); + config.pins_file = try std.fmt.allocPrint(allocator, "{s}/pins", .{dir}); + allocator.free(config.ipwhois_cache_file); + config.ipwhois_cache_file = try std.fmt.allocPrint(allocator, "{s}/ipwhois", .{dir}); + + return config; +} + +test "lookupCountry: a pin decides units instead of the database" { + const allocator = std.testing.allocator; + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + const dir_len = try tmp.dir.realPath(io, &path_buf); + const dir = path_buf[0..dir_len]; + + var config = try testConfigIn(allocator, io, dir); + defer config.deinit(allocator); + + // GeoLite2 has this AT&T address in Texas, so the database says US. Unit + // selection used to read the database directly, which meant a client pinned + // to London was still rendered in Fahrenheit. + const ip = "12.94.132.170"; + + { + var geoip = GeoIP.init(allocator, io, config.geolite_path, config) catch + return error.SkipZigTest; + defer geoip.deinit(); + const unpinned = geoip.lookupCountry(ip) orelse return error.SkipZigTest; + if (!std.mem.eql(u8, &unpinned, "US")) return error.SkipZigTest; + } + + { + var pins: Pins = .init(allocator); + defer pins.deinit(); + try pins.put(try Pins.parseCidr("12.94.132.0/24"), "London, United Kingdom", .{ .latitude = 51.5, .longitude = -0.1 }, Location.isoFrom("GB")); + try pins.save(io, config.pins_file); + } + + var geoip = GeoIP.init(allocator, io, config.geolite_path, config) catch + return error.SkipZigTest; + defer geoip.deinit(); + + const iso = geoip.lookupCountry(ip) orelse return error.TestUnexpectedResult; + try std.testing.expectEqualStrings("GB", &iso); + try std.testing.expect(!geoip.isUSIp(ip)); +} + +test "lookupCountry: a pin applies where the database has no entry at all" { + const allocator = std.testing.allocator; + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + const dir_len = try tmp.dir.realPath(io, &path_buf); + const dir = path_buf[0..dir_len]; + + var config = try testConfigIn(allocator, io, dir); + defer config.deinit(allocator); + + // TEST-NET-3, reserved for documentation and absent from GeoLite2. Standing + // in for any address the database does not cover. + const ip = "203.0.113.7"; + + { + var pins: Pins = .init(allocator); + defer pins.deinit(); + try pins.put(try Pins.parseCidr("203.0.113.0/24"), "Seattle, Washington, United States", .{ .latitude = 47.6, .longitude = -122.3 }, Location.isoFrom("US")); + try pins.save(io, config.pins_file); + } + + var geoip = GeoIP.init(allocator, io, config.geolite_path, config) catch + return error.SkipZigTest; + defer geoip.deinit(); + + const iso = geoip.lookupCountry(ip) orelse return error.TestUnexpectedResult; + try std.testing.expectEqualStrings("US", &iso); + try std.testing.expect(geoip.isUSIp(ip)); +} + +test "lookupCountry: a pin with no country falls through to the database" { + const allocator = std.testing.allocator; + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + const dir_len = try tmp.dir.realPath(io, &path_buf); + const dir = path_buf[0..dir_len]; + + var config = try testConfigIn(allocator, io, dir); + defer config.deinit(allocator); + + const ip = "12.94.132.170"; + + // Pins written before the country was recorded exist on disk. They must keep + // behaving as they did rather than forcing every pinned client to metric. + { + var pins: Pins = .init(allocator); + defer pins.deinit(); + try pins.put(try Pins.parseCidr("12.94.132.0/24"), "Somewhere", .{ .latitude = 1, .longitude = 2 }, null); + try pins.save(io, config.pins_file); + } + + var geoip = GeoIP.init(allocator, io, config.geolite_path, config) catch + return error.SkipZigTest; + defer geoip.deinit(); + + // The pin still decides the location... + const pinned = geoip.lookup(ip) orelse return error.TestUnexpectedResult; + defer pinned.deinit(); + try std.testing.expectEqualStrings("Somewhere", pinned.name); + + // ...while the country comes from the database, as it did before pins existed. + const iso = geoip.lookupCountry(ip) orelse return error.SkipZigTest; + try std.testing.expectEqualStrings("US", &iso); +} + +test "lookupCountry: a cached fallback answer decides units" { + const allocator = std.testing.allocator; + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + const dir_len = try tmp.dir.realPath(io, &path_buf); + const dir = path_buf[0..dir_len]; + + var config = try testConfigIn(allocator, io, dir); + defer config.deinit(allocator); + + // Absent from GeoLite2, so the only source of a country is the cache. This is + // the case unit selection could not see before: the fallback runs only when + // the database declines, and units never consulted the fallback at all. + const ip = "198.51.100.7"; + const key = Ip2location.packIp(ip).?; + + { + var cache = try Ip2location.Cache.init(allocator, io, config.ipwhois_cache_file); + defer cache.deinit(); + try cache.put(key.key, key.family, .{ + .allocator = allocator, + .name = "Seattle, Washington, United States", + .coords = .{ .latitude = 47.6, .longitude = -122.3 }, + .iso_country = Location.isoFrom("US"), + }); + } + + var geoip = GeoIP.init(allocator, io, config.geolite_path, config) catch + return error.SkipZigTest; + defer geoip.deinit(); + + const iso = geoip.lookupCountry(ip) orelse return error.TestUnexpectedResult; + try std.testing.expectEqualStrings("US", &iso); + try std.testing.expect(geoip.isUSIp(ip)); +} + +test "lookupCountry: null when no source knows the address" { + const allocator = std.testing.allocator; + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + const dir_len = try tmp.dir.realPath(io, &path_buf); + const dir = path_buf[0..dir_len]; + + var config = try testConfigIn(allocator, io, dir); + defer config.deinit(allocator); + + var geoip = GeoIP.init(allocator, io, config.geolite_path, config) catch + return error.SkipZigTest; + defer geoip.deinit(); + + // No pin, nothing in the database, nothing cached. Must not resolve to a + // country, and must not attempt a network request to find one. + try std.testing.expect(geoip.lookupCountry("203.0.113.7") == null); + try std.testing.expect(!geoip.isUSIp("203.0.113.7")); + + // A malformed address must be rejected rather than reaching any provider. + try std.testing.expect(geoip.lookupCountry("not-an-address") == null); + try std.testing.expect(!geoip.isUSIp("not-an-address")); +} diff --git a/src/location/Ip2location.zig b/src/location/Ip2location.zig index 95f5f94..d332358 100644 --- a/src/location/Ip2location.zig +++ b/src/location/Ip2location.zig @@ -165,6 +165,7 @@ fn fetch(self: *Self, ip_str: []const u8) !Location { .latitude = @floatCast(lat.float), .longitude = @floatCast(lon.float), }, + .iso_country = Location.isoFrom(getString(obj, "country_code")), }; } @@ -195,11 +196,16 @@ pub const Cache = struct { /// /// The address is stored as text rather than the `u128` used for the in-memory /// key, so the file stays legible to whoever has to look at it. + /// + /// `iso` defaults to null so cache files written before it existed still + /// load: SRF fills a missing field from the Zig default. Entries cached + /// without a country resolve their units from the database instead. const Record = struct { ip: []const u8, lat: f64, lon: f64, name: []const u8, + iso: ?[]const u8 = null, }; pub fn init(allocator: Allocator, io: std.Io, path: []const u8) !Cache { @@ -292,6 +298,7 @@ pub const Cache = struct { .allocator = self.allocator, .name = name_copy, .coords = .{ .latitude = record.lat, .longitude = record.lon }, + .iso_country = if (record.iso) |iso| Location.isoFrom(iso) else null, }); if (existing) |old| self.allocator.free(old.value.name); } @@ -303,9 +310,19 @@ pub const Cache = struct { .allocator = self.allocator, .name = self.allocator.dupe(u8, entry.name) catch return null, .coords = entry.coords, + .iso_country = entry.iso_country, }; } + /// Country code for a cached address, without allocating or fetching. + /// + /// Unit selection only needs the country, and doing it this way keeps that + /// path free of both an allocation and any chance of a network request. + pub fn getCountry(self: *Cache, ip: u128) ?[2]u8 { + const entry = self.entries.getPtr(ip) orelse return null; + return entry.iso_country; + } + pub fn put(self: *Cache, ip: u128, family: u8, loc: Location) !void { const name_copy = try self.allocator.dupe(u8, loc.name); errdefer self.allocator.free(name_copy); @@ -314,6 +331,7 @@ pub const Cache = struct { .allocator = self.allocator, .name = name_copy, .coords = loc.coords, + .iso_country = loc.iso_country, }); if (existing) |old| self.allocator.free(old.value.name); @@ -330,6 +348,7 @@ pub const Cache = struct { .lat = loc.coords.latitude, .lon = loc.coords.longitude, .name = loc.name, + .iso = if (loc.iso_country) |*iso| iso[0..] else null, }}, .{ .emit_directives = false })}); // 0.16 has no seek+write; append at the current end offset. @@ -446,3 +465,115 @@ test "Cache: formatKey renders both families without a port" { const v6 = packIp("2001:db8::1").?; try std.testing.expectEqualStrings("2001:db8::1", try Cache.formatKey(v6.key, v6.family, &buf)); } + +test "Cache: round-trips the country through the file" { + const allocator = std.testing.allocator; + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + const dir_len = try tmp.dir.realPath(io, &path_buf); + const path = try std.fmt.allocPrint(allocator, "{s}/ip.cache", .{path_buf[0..dir_len]}); + defer allocator.free(path); + + const us = packIp("198.51.100.1").?; + const unknown = packIp("198.51.100.2").?; + + { + var cache = try Cache.init(allocator, io, path); + defer cache.deinit(); + + try cache.put(us.key, us.family, .{ + .allocator = allocator, + .name = "Seattle, Washington, United States", + .coords = .{ .latitude = 47.6, .longitude = -122.3 }, + .iso_country = Location.isoFrom("US"), + }); + // An entry cached with no country has to stay that way rather than + // acquiring one, so units fall back to the database for it. + try cache.put(unknown.key, unknown.family, .{ + .allocator = allocator, + .name = "Nowhere", + .coords = .{ .latitude = 1, .longitude = 2 }, + }); + } + + var reopened = try Cache.init(allocator, io, path); + defer reopened.deinit(); + + const got = reopened.get(us.key) orelse return error.TestUnexpectedResult; + defer got.deinit(); + try std.testing.expectEqualStrings("US", &(got.iso_country.?)); + + const iso = reopened.getCountry(us.key) orelse return error.TestUnexpectedResult; + try std.testing.expectEqualStrings("US", &iso); + + try std.testing.expect(reopened.getCountry(unknown.key) == null); +} + +test "Cache: getCountry is null for an address that was never cached" { + const allocator = std.testing.allocator; + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + const dir_len = try tmp.dir.realPath(io, &path_buf); + const path = try std.fmt.allocPrint(allocator, "{s}/ip.cache", .{path_buf[0..dir_len]}); + defer allocator.free(path); + + var cache = try Cache.init(allocator, io, path); + defer cache.deinit(); + + // Must not be mistaken for "cached with no country": both read as null here, + // and in both cases the caller falls through, so this pins the shape rather + // than a distinction the caller makes. + try std.testing.expect(cache.getCountry(packIp("203.0.113.9").?.key) == null); +} + +test "Cache: a file written before the country field still loads" { + const allocator = std.testing.allocator; + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + const dir_len = try tmp.dir.realPath(io, &path_buf); + const path = try std.fmt.allocPrint(allocator, "{s}/ip.cache", .{path_buf[0..dir_len]}); + defer allocator.free(path); + + // Produced by writing the four-field record, then reopened against the + // five-field one. This cache is permanent and never evicted, so dropping it + // on a schema change would mean re-fetching every address ever resolved. + { + var cache = try Cache.init(allocator, io, path); + defer cache.deinit(); + const OldRecord = struct { + ip: []const u8, + lat: f64, + lon: f64, + name: []const u8, + }; + var line_buf: [512]u8 = undefined; + const line = try std.fmt.bufPrint(&line_buf, "{f}", .{srf.fmt(OldRecord, &.{.{ + .ip = "198.51.100.7", + .lat = 47.6, + .lon = -122.3, + .name = "Seattle, Washington, United States", + }}, .{ .emit_directives = false })}); + const file = cache.file.?; + const end = try file.length(io); + try file.writePositionalAll(io, line, end); + } + + var reopened = try Cache.init(allocator, io, path); + defer reopened.deinit(); + + const key = packIp("198.51.100.7").?.key; + const got = reopened.get(key) orelse return error.TestUnexpectedResult; + defer got.deinit(); + try std.testing.expectEqualStrings("Seattle, Washington, United States", got.name); + try std.testing.expectEqual(@as(f64, 47.6), got.coords.latitude); + try std.testing.expect(got.iso_country == null); +} diff --git a/src/location/IpWhoIs.zig b/src/location/IpWhoIs.zig index d408256..9f4988e 100644 --- a/src/location/IpWhoIs.zig +++ b/src/location/IpWhoIs.zig @@ -65,8 +65,9 @@ fn fetch(self: *Self, ip_str: []const u8) !Location { var w = std.Io.Writer.fixed(&buf); try w.writeAll("https://ipwho.is/"); try w.writeAll(ip_str); - // Request only the fields we need - try w.writeAll("?fields=city,region,country,latitude,longitude&output=json"); + // Request only the fields we need. `country_code` drives unit selection, + // which cannot be derived from `country` without matching display text. + try w.writeAll("?fields=city,region,country,country_code,latitude,longitude&output=json"); var response_buf: [4096]u8 = undefined; var writer = std.Io.Writer.fixed(&response_buf); @@ -139,6 +140,7 @@ fn fetch(self: *Self, ip_str: []const u8) !Location { .latitude = lat_val, .longitude = lon_val, }, + .iso_country = Location.isoFrom(getString(obj, "country_code")), }; } diff --git a/src/location/Pins.zig b/src/location/Pins.zig index bb13de3..53cc5ac 100644 --- a/src/location/Pins.zig +++ b/src/location/Pins.zig @@ -30,6 +30,11 @@ pub const Entry = struct { prefix_len: u8, name: []const u8, coords: Coordinates, + /// ISO 3166-1 alpha-2 country code, upper case, when it was known at the + /// time the pin was written. Null for pins created before this was recorded, + /// which `GeoIp.lookupCountry` treats as "no opinion" and resolves from the + /// database instead of forcing a wrong answer. + iso_country: ?[2]u8 = null, }; pub fn init(allocator: std.mem.Allocator) Pins { @@ -120,7 +125,7 @@ pub fn formatCidr(cidr: Cidr, buf: []u8) ![]const u8 { /// /// Longest-prefix ordering means an exact host pin naturally wins over a range /// covering it, without needing a separate precedence rule. -pub fn lookup(self: *const Pins, allocator: std.mem.Allocator, ip_str: []const u8) ?Location { +fn bestMatch(self: *const Pins, ip_str: []const u8) ?*const Entry { const packed_ip = packIp(ip_str) orelse return null; var best: ?*const Entry = null; @@ -129,17 +134,32 @@ pub fn lookup(self: *const Pins, allocator: std.mem.Allocator, ip_str: []const u if (maskToPrefix(packed_ip.key, e.family, e.prefix_len) != e.network) continue; if (best == null or e.prefix_len > best.?.prefix_len) best = e; } + return best; +} - const entry = best orelse return null; +/// Longest-prefix match for `ip_str`, or null when no pin applies. +pub fn lookup(self: *const Pins, allocator: std.mem.Allocator, ip_str: []const u8) ?Location { + const entry = self.bestMatch(ip_str) orelse return null; return .{ .allocator = allocator, .name = allocator.dupe(u8, entry.name) catch return null, .coords = entry.coords, + .iso_country = entry.iso_country, }; } +/// Country code of the matching pin, without allocating. +/// +/// Null both when no pin matches and when the matching pin has no recorded +/// country, because the caller treats those the same way: fall through to the +/// database rather than claim the country is unknown. +pub fn lookupCountry(self: *const Pins, ip_str: []const u8) ?[2]u8 { + const entry = self.bestMatch(ip_str) orelse return null; + return entry.iso_country; +} + /// Inserts a pin, replacing any existing pin for the same network. -pub fn put(self: *Pins, cidr: Cidr, name: []const u8, coords: Coordinates) !void { +pub fn put(self: *Pins, cidr: Cidr, name: []const u8, coords: Coordinates, iso_country: ?[2]u8) !void { const name_copy = try self.allocator.dupe(u8, name); errdefer self.allocator.free(name_copy); @@ -148,6 +168,7 @@ pub fn put(self: *Pins, cidr: Cidr, name: []const u8, coords: Coordinates) !void self.allocator.free(e.name); e.name = name_copy; e.coords = coords; + e.iso_country = iso_country; return; } } @@ -158,6 +179,7 @@ pub fn put(self: *Pins, cidr: Cidr, name: []const u8, coords: Coordinates) !void .prefix_len = cidr.prefix_len, .name = name_copy, .coords = coords, + .iso_country = iso_country, }); } @@ -181,11 +203,15 @@ pub fn remove(self: *Pins, cidr: Cidr) bool { /// part for free: place names routinely contain commas ("San Francisco, /// California, United States"), which a comma-delimited format has to special /// case. +/// +/// `iso` defaults to null so pins written before it existed still load: SRF +/// fills a missing field from the Zig default rather than rejecting the record. const Record = struct { cidr: []const u8, lat: f64, lon: f64, name: []const u8, + iso: ?[]const u8 = null, }; /// Loads pins from `path`. A missing file yields an empty set: having no @@ -228,7 +254,12 @@ pub fn load(allocator: std.mem.Allocator, io: std.Io, path: []const u8) !Pins { // `put` copies the name, so it does not matter that the parsed strings // belong to the iterator's arena. - try pins.put(cidr, record.name, .{ .latitude = record.lat, .longitude = record.lon }); + try pins.put( + cidr, + record.name, + .{ .latitude = record.lat, .longitude = record.lon }, + if (record.iso) |iso| Location.isoFrom(iso) else null, + ); } return pins; @@ -255,7 +286,7 @@ pub fn save(self: *const Pins, io: std.Io, path: []const u8) !void { self.allocator.free(cidr_texts); } - for (self.entries.items, 0..) |e, i| { + for (self.entries.items, 0..) |*e, i| { var buf: [64]u8 = undefined; const text = try formatCidr( .{ .family = e.family, .network = e.network, .prefix_len = e.prefix_len }, @@ -268,6 +299,10 @@ pub fn save(self: *const Pins, io: std.Io, path: []const u8) !void { .lat = e.coords.latitude, .lon = e.coords.longitude, .name = e.name, + // Borrowed from the entry rather than copied: the list is not + // mutated before the `print` below, and a by-value capture would + // leave this pointing at a dead loop temporary. + .iso = if (e.iso_country) |*iso| iso[0..] else null, }; } @@ -337,7 +372,7 @@ test "lookup: matches an address inside the range" { var pins: Pins = .init(allocator); defer pins.deinit(); - try pins.put(try parseCidr("12.94.132.0/24"), "San Francisco, California", .{ .latitude = 37.7749, .longitude = -122.4194 }); + try pins.put(try parseCidr("12.94.132.0/24"), "San Francisco, California", .{ .latitude = 37.7749, .longitude = -122.4194 }, null); const hit = pins.lookup(allocator, "12.94.132.170") orelse return error.TestUnexpectedResult; defer hit.deinit(); @@ -350,7 +385,7 @@ test "lookup: ignores an address outside the range" { var pins: Pins = .init(allocator); defer pins.deinit(); - try pins.put(try parseCidr("12.94.132.0/24"), "San Francisco", .{ .latitude = 37.7749, .longitude = -122.4194 }); + try pins.put(try parseCidr("12.94.132.0/24"), "San Francisco", .{ .latitude = 37.7749, .longitude = -122.4194 }, null); try std.testing.expect(pins.lookup(allocator, "12.94.133.1") == null); } @@ -361,8 +396,8 @@ test "lookup: longest prefix wins over a broader range" { defer pins.deinit(); // Deliberately inserted broad-first so the result cannot come from ordering. - try pins.put(try parseCidr("12.0.0.0/8"), "Broad", .{ .latitude = 1, .longitude = 1 }); - try pins.put(try parseCidr("12.94.132.0/24"), "Specific", .{ .latitude = 2, .longitude = 2 }); + try pins.put(try parseCidr("12.0.0.0/8"), "Broad", .{ .latitude = 1, .longitude = 1 }, null); + try pins.put(try parseCidr("12.94.132.0/24"), "Specific", .{ .latitude = 2, .longitude = 2 }, null); const hit = pins.lookup(allocator, "12.94.132.170") orelse return error.TestUnexpectedResult; defer hit.deinit(); @@ -374,8 +409,8 @@ test "lookup: an exact host pin beats a range containing it" { var pins: Pins = .init(allocator); defer pins.deinit(); - try pins.put(try parseCidr("12.94.132.170"), "Host", .{ .latitude = 2, .longitude = 2 }); - try pins.put(try parseCidr("12.94.132.0/24"), "Range", .{ .latitude = 1, .longitude = 1 }); + try pins.put(try parseCidr("12.94.132.170"), "Host", .{ .latitude = 2, .longitude = 2 }, null); + try pins.put(try parseCidr("12.94.132.0/24"), "Range", .{ .latitude = 1, .longitude = 1 }, null); const hit = pins.lookup(allocator, "12.94.132.170") orelse return error.TestUnexpectedResult; defer hit.deinit(); @@ -387,7 +422,7 @@ test "lookup: families do not cross-match" { var pins: Pins = .init(allocator); defer pins.deinit(); - try pins.put(try parseCidr("0.0.0.0/0"), "All IPv4", .{ .latitude = 1, .longitude = 1 }); + try pins.put(try parseCidr("0.0.0.0/0"), "All IPv4", .{ .latitude = 1, .longitude = 1 }, null); // An IPv4 /0 must not swallow IPv6 clients. try std.testing.expect(pins.lookup(allocator, "2001:db8::1") == null); @@ -401,8 +436,8 @@ test "put: replaces an existing pin for the same network" { defer pins.deinit(); const cidr = try parseCidr("12.94.132.0/24"); - try pins.put(cidr, "Old", .{ .latitude = 1, .longitude = 1 }); - try pins.put(cidr, "New", .{ .latitude = 2, .longitude = 2 }); + try pins.put(cidr, "Old", .{ .latitude = 1, .longitude = 1 }, null); + try pins.put(cidr, "New", .{ .latitude = 2, .longitude = 2 }, null); try std.testing.expectEqual(@as(usize, 1), pins.entries.items.len); const hit = pins.lookup(allocator, "12.94.132.5") orelse return error.TestUnexpectedResult; @@ -416,7 +451,7 @@ test "remove: reports whether a pin was present" { defer pins.deinit(); const cidr = try parseCidr("12.94.132.0/24"); - try pins.put(cidr, "SF", .{ .latitude = 1, .longitude = 1 }); + try pins.put(cidr, "SF", .{ .latitude = 1, .longitude = 1 }, null); try std.testing.expect(pins.remove(cidr)); try std.testing.expect(!pins.remove(cidr)); @@ -449,8 +484,8 @@ test "save then load round-trips, including commas in the name" { { var pins: Pins = .init(allocator); defer pins.deinit(); - try pins.put(try parseCidr("12.94.132.0/24"), "San Francisco, California, United States", .{ .latitude = 37.7749, .longitude = -122.4194 }); - try pins.put(try parseCidr("2001:db8::/32"), "Test, Place", .{ .latitude = -1.5, .longitude = 2.25 }); + try pins.put(try parseCidr("12.94.132.0/24"), "San Francisco, California, United States", .{ .latitude = 37.7749, .longitude = -122.4194 }, null); + try pins.put(try parseCidr("2001:db8::/32"), "Test, Place", .{ .latitude = -1.5, .longitude = 2.25 }, null); try pins.save(io, path); } @@ -549,7 +584,7 @@ test "save writes SRF that an operator can read" { var pins: Pins = .init(allocator); defer pins.deinit(); - try pins.put(try parseCidr("12.94.132.0/24"), "San Francisco, California", .{ .latitude = 37.7749, .longitude = -122.4194 }); + try pins.put(try parseCidr("12.94.132.0/24"), "San Francisco, California", .{ .latitude = 37.7749, .longitude = -122.4194 }, null); try pins.save(io, path); const content = try std.Io.Dir.cwd().readFileAlloc(io, path, allocator, .limited(64 * 1024)); @@ -563,3 +598,195 @@ test "save writes SRF that an operator can read" { try std.testing.expect(std.mem.indexOf(u8, content, "cidr::12.94.132.0/24") != null); try std.testing.expect(std.mem.indexOf(u8, content, "San Francisco, California") != null); } + +test "lookupCountry: returns the country of the matching pin" { + const allocator = std.testing.allocator; + var pins: Pins = .init(allocator); + defer pins.deinit(); + + try pins.put( + try parseCidr("12.94.132.0/24"), + "San Francisco, California, United States", + .{ .latitude = 37.7749, .longitude = -122.4194 }, + Location.isoFrom("US"), + ); + + const iso = pins.lookupCountry("12.94.132.170") orelse return error.TestUnexpectedResult; + try std.testing.expectEqualStrings("US", &iso); +} + +test "lookupCountry: null for a pin with no recorded country" { + const allocator = std.testing.allocator; + var pins: Pins = .init(allocator); + defer pins.deinit(); + + // Pins written before the country was stored must read as "no opinion" so + // the caller can fall through to the database rather than assume a country. + try pins.put(try parseCidr("12.94.132.0/24"), "Somewhere", .{ .latitude = 1, .longitude = 2 }, null); + + try std.testing.expect(pins.lookupCountry("12.94.132.170") == null); +} + +test "lookupCountry: null when no pin matches" { + const allocator = std.testing.allocator; + var pins: Pins = .init(allocator); + defer pins.deinit(); + + try pins.put(try parseCidr("12.94.132.0/24"), "SF", .{ .latitude = 1, .longitude = 2 }, Location.isoFrom("US")); + + try std.testing.expect(pins.lookupCountry("8.8.8.8") == null); +} + +test "lookupCountry: longest prefix decides the country too" { + const allocator = std.testing.allocator; + var pins: Pins = .init(allocator); + defer pins.deinit(); + + try pins.put(try parseCidr("12.0.0.0/8"), "Broad", .{ .latitude = 1, .longitude = 1 }, Location.isoFrom("GB")); + try pins.put(try parseCidr("12.94.132.0/24"), "Specific", .{ .latitude = 2, .longitude = 2 }, Location.isoFrom("US")); + + const iso = pins.lookupCountry("12.94.132.170") orelse return error.TestUnexpectedResult; + try std.testing.expectEqualStrings("US", &iso); +} + +test "put: replacing a pin also replaces its country" { + const allocator = std.testing.allocator; + var pins: Pins = .init(allocator); + defer pins.deinit(); + + const cidr = try parseCidr("12.94.132.0/24"); + try pins.put(cidr, "London", .{ .latitude = 1, .longitude = 1 }, Location.isoFrom("GB")); + try pins.put(cidr, "Seattle", .{ .latitude = 2, .longitude = 2 }, Location.isoFrom("US")); + + try std.testing.expectEqual(@as(usize, 1), pins.entries.items.len); + const iso = pins.lookupCountry("12.94.132.5") orelse return error.TestUnexpectedResult; + try std.testing.expectEqualStrings("US", &iso); +} + +test "save then load round-trips the country" { + const allocator = std.testing.allocator; + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + const dir_len = try tmp.dir.realPath(io, &path_buf); + const path = try std.fmt.allocPrint(allocator, "{s}/pins", .{path_buf[0..dir_len]}); + defer allocator.free(path); + + { + var pins: Pins = .init(allocator); + defer pins.deinit(); + try pins.put(try parseCidr("12.94.132.0/24"), "Seattle", .{ .latitude = 47.6, .longitude = -122.3 }, Location.isoFrom("US")); + // A pin with no country has to survive the round trip as "no country" + // rather than becoming a bogus one. + try pins.put(try parseCidr("10.0.0.0/8"), "Unknown", .{ .latitude = 1, .longitude = 2 }, null); + try pins.save(io, path); + } + + var loaded = try load(allocator, io, path); + defer loaded.deinit(); + + const us = loaded.lookupCountry("12.94.132.1") orelse return error.TestUnexpectedResult; + try std.testing.expectEqualStrings("US", &us); + try std.testing.expect(loaded.lookupCountry("10.1.2.3") == null); +} + +test "save omits the country field entirely when it is unknown" { + const allocator = std.testing.allocator; + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + const dir_len = try tmp.dir.realPath(io, &path_buf); + const path = try std.fmt.allocPrint(allocator, "{s}/pins", .{path_buf[0..dir_len]}); + defer allocator.free(path); + + var pins: Pins = .init(allocator); + defer pins.deinit(); + try pins.put(try parseCidr("10.0.0.0/8"), "Unknown", .{ .latitude = 1, .longitude = 2 }, null); + try pins.save(io, path); + + const content = try std.Io.Dir.cwd().readFileAlloc(io, path, allocator, .limited(64 * 1024)); + defer allocator.free(content); + + // An unknown country should add nothing to a file an operator reads, rather + // than an empty or "null" field they have to interpret. + try std.testing.expect(std.mem.indexOf(u8, content, "iso") == null); +} + +test "load: a pins file written before the country field still loads" { + const allocator = std.testing.allocator; + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + const dir_len = try tmp.dir.realPath(io, &path_buf); + const path = try std.fmt.allocPrint(allocator, "{s}/pins", .{path_buf[0..dir_len]}); + defer allocator.free(path); + + // Exactly the shape `save` produced before `iso` existed. Operators have + // these on disk, so failing to read them would silently drop every override. + try std.Io.Dir.cwd().writeFile(io, .{ + .sub_path = path, + .data = + \\#!srfv1 + \\#!long + \\cidr::70.102.70.100/32 + \\lat:num:47.6038321 + \\lon:num:-122.330062 + \\name::Seattle, King County, Washington, United States + \\ + , + }); + + var pins = try load(allocator, io, path); + defer pins.deinit(); + + try std.testing.expectEqual(@as(usize, 1), pins.entries.items.len); + const hit = pins.lookup(allocator, "70.102.70.100") orelse return error.TestUnexpectedResult; + defer hit.deinit(); + try std.testing.expectEqualStrings("Seattle, King County, Washington, United States", hit.name); + // No country recorded, so the caller must fall through rather than guess. + try std.testing.expect(hit.iso_country == null); + try std.testing.expect(pins.lookupCountry("70.102.70.100") == null); +} + +test "load: an unparseable country is dropped, keeping the rest of the pin" { + const allocator = std.testing.allocator; + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + var path_buf: [std.fs.max_path_bytes]u8 = undefined; + const dir_len = try tmp.dir.realPath(io, &path_buf); + const path = try std.fmt.allocPrint(allocator, "{s}/pins", .{path_buf[0..dir_len]}); + defer allocator.free(path); + + // Hand-edited files are expected here, so a bad country must not cost the + // operator the location they actually cared about. + try std.Io.Dir.cwd().writeFile(io, .{ + .sub_path = path, + .data = + \\#!srfv1 + \\#!long + \\cidr::10.0.0.0/8 + \\lat:num:1 + \\lon:num:2 + \\name::Somewhere + \\iso::United States + \\ + , + }); + + var pins = try load(allocator, io, path); + defer pins.deinit(); + + try std.testing.expectEqual(@as(usize, 1), pins.entries.items.len); + try std.testing.expect(pins.lookupCountry("10.1.2.3") == null); + const hit = pins.lookup(allocator, "10.1.2.3") orelse return error.TestUnexpectedResult; + defer hit.deinit(); + try std.testing.expectEqualStrings("Somewhere", hit.name); +} diff --git a/src/location/resolver.zig b/src/location/resolver.zig index 0358e13..1896076 100644 --- a/src/location/resolver.zig +++ b/src/location/resolver.zig @@ -11,11 +11,38 @@ pub const Location = struct { name: []const u8, coords: Coordinates, allocator: std.mem.Allocator, + /// ISO 3166-1 alpha-2 country code, upper case, when the producing source + /// knew it. Drives unit selection, which needs the country rather than the + /// display name. + /// + /// Deliberately a fixed-size array rather than a slice: a country code is + /// always two characters, so storing it inline keeps `deinit` a single free + /// and leaves every existing construction site valid via the default. + iso_country: ?[2]u8 = null, pub fn deinit(self: Location) void { self.allocator.free(self.name); } + /// Normalizes a country code from a provider into the stored form. + /// + /// Providers disagree on case -- GeoLite2 and ipwho.is return "US" while + /// Nominatim returns "us" -- so everything is upper-cased on the way in and + /// comparisons downstream can be plain equality. Anything that is not two + /// ASCII letters is rejected rather than stored, so a provider returning "" + /// or "N/A" reads back as "unknown" instead of as a country. + pub fn isoFrom(text: []const u8) ?[2]u8 { + if (text.len != 2) return null; + if (!std.ascii.isAlphabetic(text[0]) or !std.ascii.isAlphabetic(text[1])) return null; + return .{ std.ascii.toUpper(text[0]), std.ascii.toUpper(text[1]) }; + } + + /// Whether this location is in the United States, for unit selection. + pub fn isUS(self: Location) bool { + const iso = self.iso_country orelse return false; + return std.mem.eql(u8, &iso, "US"); + } + /// Build a display name from city, subdivision (state/province), and country /// Returns allocated string that must be freed by caller pub fn buildDisplayName(allocator: std.mem.Allocator, city: []const u8, subdivision: []const u8, country: []const u8, fallback: []const u8) []const u8 { @@ -161,15 +188,18 @@ pub const Resolver = struct { .allocator = self.allocator, .name = try self.allocator.dupe(u8, cached.name), .coords = cached.coords, + .iso_country = cached.iso_country, }; } log.info("Calling nominatim (OpenStreetMap) to resolve place name {s} to coordinates", .{name}); if (@import("builtin").is_test) return error.GeocodingUnavailableInUnitTest; - // Call Nominatim API + // Call Nominatim API. `addressdetails=1` is requested for + // `address.country_code`, which is what lets a pin created from a place + // name record its country for unit selection. const url = try std.fmt.allocPrint( self.allocator, - "https://nominatim.openstreetmap.org/search?q={s}&format=json&limit=1", + "https://nominatim.openstreetmap.org/search?q={s}&format=json&limit=1&addressdetails=1", .{name}, ); defer self.allocator.free(url); @@ -218,6 +248,17 @@ pub const Resolver = struct { const lon = try std.fmt.parseFloat(f64, first.object.get("lon").?.string); log.info("nominatim resolved place name {s} to {}, {}", .{ name, lat, lon }); + // Nominatim nests the country code under `address` and returns it lower + // case; `isoFrom` normalizes. Absent for results that carry no address + // block, which is why this is optional rather than required. + const iso_country: ?[2]u8 = blk: { + const address = first.object.get("address") orelse break :blk null; + if (address != .object) break :blk null; + const code = address.object.get("country_code") orelse break :blk null; + if (code != .string) break :blk null; + break :blk Location.isoFrom(code.string); + }; + // Cache the result try self.geocache.put(name, .{ .name = display_name, @@ -225,6 +266,7 @@ pub const Resolver = struct { .latitude = lat, .longitude = lon, }, + .iso_country = iso_country, }); return .{ @@ -234,6 +276,7 @@ pub const Resolver = struct { .latitude = lat, .longitude = lon, }, + .iso_country = iso_country, }; } @@ -441,3 +484,50 @@ test "buildDisplayName: city only (no state or country)" { defer allocator.free(name); try std.testing.expectEqualStrings("Paris", name); } + +test "isoFrom: upper-cases so providers of either convention agree" { + // Nominatim returns "us", GeoLite2 and ipwho.is return "US". Without + // normalization the same country would compare unequal depending on source. + try std.testing.expectEqualStrings("US", &(Location.isoFrom("us").?)); + try std.testing.expectEqualStrings("US", &(Location.isoFrom("US").?)); + try std.testing.expectEqualStrings("GB", &(Location.isoFrom("gB").?)); +} + +test "isoFrom: rejects anything that is not two letters" { + // Providers return "" for unknown and occasionally a placeholder. Storing + // those would make an unknown country read back as a real one. + try std.testing.expect(Location.isoFrom("") == null); + try std.testing.expect(Location.isoFrom("U") == null); + try std.testing.expect(Location.isoFrom("USA") == null); + try std.testing.expect(Location.isoFrom("12") == null); + try std.testing.expect(Location.isoFrom("N/A") == null); + try std.testing.expect(Location.isoFrom("-") == null); +} + +test "isUS: only true for a known US country code" { + const allocator = std.testing.allocator; + const base: Location = .{ .allocator = allocator, .name = "", .coords = .{ .latitude = 0, .longitude = 0 } }; + + // An unknown country must not read as US: that would flip every client with + // no country data to imperial. + try std.testing.expect(!base.isUS()); + + var us = base; + us.iso_country = Location.isoFrom("us"); + try std.testing.expect(us.isUS()); + + var gb = base; + gb.iso_country = Location.isoFrom("GB"); + try std.testing.expect(!gb.isUS()); +} + +test "Location: iso_country defaults to unknown" { + // The default is what keeps every existing construction site valid, so it is + // worth pinning down rather than leaving implied. + const loc: Location = .{ + .allocator = std.testing.allocator, + .name = "", + .coords = .{ .latitude = 0, .longitude = 0 }, + }; + try std.testing.expect(loc.iso_country == null); +}