From 456057a2c054cf8ff5d1e1ed2e004d531405bff5 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Mon, 5 Jan 2026 22:56:32 -0800 Subject: [PATCH] working IP address implementation and unit tests around GeoIP/full handler flow --- src/http/Server.zig | 164 +++++++++++++++++++++++++++++++++++ src/http/handler.zig | 32 ++++--- src/location/GeoIp.zig | 117 ++++++++++++++++++------- src/location/Ip2location.zig | 3 +- src/location/resolver.zig | 47 +++++++++- src/weather/Mock.zig | 3 +- 6 files changed, 316 insertions(+), 50 deletions(-) diff --git a/src/http/Server.zig b/src/http/Server.zig index 9644391..5737f6e 100644 --- a/src/http/Server.zig +++ b/src/http/Server.zig @@ -1,5 +1,6 @@ const std = @import("std"); const httpz = @import("httpz"); +const build_options = @import("build_options"); const handler = @import("handler.zig"); const RateLimiter = @import("RateLimiter.zig"); @@ -91,3 +92,166 @@ pub fn listen(self: *Server) !void { pub fn deinit(self: *Server) void { self.httpz_server.deinit(); } + +const MockHarness = struct { + allocator: std.mem.Allocator, + config: Config, + geoip: *GeoIp, + geocache: *GeoCache, + resolver: *Resolver, + mock: *Mock, + cache: *Cache, + opts: handler.HandleWeatherOptions, + + const Mock = @import("../weather/Mock.zig"); + const GeoCache = @import("../location/GeoCache.zig"); + const GeoIp = @import("../location/GeoIp.zig"); + const Resolver = @import("../location/resolver.zig").Resolver; + const Config = @import("../Config.zig"); + const Cache = @import("../cache/Cache.zig"); + + pub fn init(allocator: std.mem.Allocator) !MockHarness { + const config = try Config.load(allocator); + errdefer config.deinit(allocator); + + if (build_options.download_geoip) { + const GeoLite2 = @import("../location/GeoLite2.zig"); + try GeoLite2.ensureDatabase(allocator, config.geolite_path); + } + + const geoip = try allocator.create(GeoIp); + errdefer allocator.destroy(geoip); + geoip.* = GeoIp.init(allocator, config.geolite_path, null, null) catch { + allocator.destroy(geoip); + return error.SkipZigTest; + }; + errdefer geoip.deinit(); + + var geocache = try allocator.create(GeoCache); + errdefer allocator.destroy(geocache); + geocache.* = try GeoCache.init(allocator, null); + errdefer geocache.deinit(); + + const resolver = try allocator.create(Resolver); + errdefer allocator.destroy(resolver); + resolver.* = Resolver.init(allocator, geoip, geocache, null); + + const mock = try allocator.create(Mock); + errdefer allocator.destroy(mock); + mock.* = try Mock.init(allocator); + errdefer mock.deinit(); + + // Set a simple parse function that returns minimal weather data + mock.parse_fn = struct { + fn parse(_: *anyopaque, alloc: std.mem.Allocator, _: []const u8) anyerror!@import("../weather/types.zig").WeatherData { + const types = @import("../weather/types.zig"); + return types.WeatherData{ + .allocator = alloc, + .location = try alloc.dupe(u8, "Test"), + .display_name = null, + .coords = .{ .latitude = 0, .longitude = 0 }, + .current = .{ + .temp_c = 20, + .feels_like_c = 20, + .condition = try alloc.dupe(u8, "Clear"), + .weather_code = .clear, + .humidity = 50, + .wind_kph = 5, + .wind_deg = 0, + .pressure_mb = 1013, + .precip_mm = 0, + .visibility_km = 10, + }, + .forecast = &.{}, + }; + } + }.parse; + + // Add wildcard response for tests + try mock.responses.put(try allocator.dupe(u8, "*"), try allocator.dupe(u8, "{}")); + + var cache = try Cache.init(allocator, .{ + .max_entries = 100, + .cache_dir = config.cache_dir, + }); + errdefer cache.deinit(); + + return .{ + .allocator = allocator, + .config = config, + .geoip = geoip, + .geocache = geocache, + .resolver = resolver, + .mock = mock, + .cache = cache, + .opts = .{ + .provider = mock.provider(cache), + .resolver = resolver, + .geoip = geoip, + }, + }; + } + + pub fn deinit(self: *MockHarness) void { + self.cache.deinit(); + self.mock.deinit(); + self.allocator.destroy(self.mock); + self.geocache.deinit(); + self.allocator.destroy(self.geocache); + self.allocator.destroy(self.resolver); + self.geoip.deinit(); + self.allocator.destroy(self.geoip); + self.config.deinit(self.allocator); + } +}; + +test "handleWeather: default endpoint uses IP address" { + const allocator = std.testing.allocator; + + var harness = try MockHarness.init(allocator); + defer harness.deinit(); + + var ht = httpz.testing.init(.{}); + defer ht.deinit(); + + ht.url("/"); + ht.header("x-forwarded-for", "73.158.64.1"); + + try handler.handleWeather(&harness.opts, ht.req, ht.res); + + try ht.expectStatus(200); +} + +test "handleWeather: x-forwarded-for with multiple IPs" { + const allocator = std.testing.allocator; + var harness = try MockHarness.init(allocator); + defer harness.deinit(); + + var ht = httpz.testing.init(.{}); + defer ht.deinit(); + + ht.url("/"); + ht.header("x-forwarded-for", "73.158.64.1, 8.8.8.8, 192.168.1.1"); + + try handler.handleWeather(&harness.opts, ht.req, ht.res); + + try ht.expectStatus(200); +} +test "handleWeather: client IP only" { + const allocator = std.testing.allocator; + + var harness = try MockHarness.init(allocator); + defer harness.deinit(); + + var ht = httpz.testing.init(.{}); + defer ht.deinit(); + + // Set connection address to a valid IP that will be in GeoIP database + ht.conn.address = try std.net.Address.parseIp("73.158.64.1", 0); + + ht.url("/"); + + try handler.handleWeather(&harness.opts, ht.req, ht.res); + + try ht.expectStatus(200); +} diff --git a/src/http/handler.zig b/src/http/handler.zig index e47789b..7f68725 100644 --- a/src/http/handler.zig +++ b/src/http/handler.zig @@ -10,6 +10,8 @@ const v2 = @import("../render/v2.zig"); const custom = @import("../render/custom.zig"); const help = @import("help.zig"); +const log = std.log.scoped(.handler); + pub const HandleWeatherOptions = struct { provider: WeatherProvider, resolver: *Resolver, @@ -47,6 +49,7 @@ pub fn handleWeather( } else break :blk client_ip; // no location, just use client ip instead }; + log.debug("location = {s}, client_ip = {s}", .{ location, client_ip }); if (location.len == 0) { res.content_type = .TEXT; res.body = "Sorry, we are unable to determine your location at this time. Try with / or /?location=\n"; @@ -113,20 +116,22 @@ fn handleWeatherInternal( const location = opts.resolver.resolve(location_query) catch |err| { switch (err) { error.LocationNotFound => { + log.debug("Location not found for query {s}", .{location_query}); res.status = 404; - res.body = "Location not found\n"; + res.body = "Location not found (location query resolution failure)\n"; return; }, else => return err, } }; + defer location.deinit(); // Fetch weather using coordinates var weather = opts.provider.fetch(req_alloc, location.coords) catch |err| { switch (err) { error.LocationNotFound => { res.status = 404; - res.body = "Location not found\n"; + res.body = "Location not found (provider fetch failure)\n"; return; }, else => return err, @@ -147,14 +152,13 @@ fn handleWeatherInternal( // Determine if imperial units should be used // Priority: explicit ?u or ?m > lang=us > US IP > default metric const use_imperial = blk: { - if (params.units) |u| { + if (params.units) |u| break :blk u == .uscs; - } - if (params.lang) |lang| { - if (std.mem.eql(u8, lang, "us")) { + + if (params.lang) |lang| + if (std.mem.eql(u8, lang, "us")) break :blk true; - } - } + if (client_ip.len > 0 and opts.geoip.isUSIp(client_ip)) break :blk true; break :blk false; @@ -164,13 +168,13 @@ fn handleWeatherInternal( if (std.mem.eql(u8, fmt, "j1")) { res.content_type = .JSON; break :blk try json.render(req_alloc, weather); - } else if (std.mem.eql(u8, fmt, "v2")) { - break :blk try v2.render(req_alloc, weather, use_imperial); - } else if (std.mem.startsWith(u8, fmt, "%")) { - break :blk try custom.render(req_alloc, weather, fmt, use_imperial); - } else { - break :blk try line.render(req_alloc, weather, fmt, use_imperial); } + if (std.mem.eql(u8, fmt, "v2")) + break :blk try v2.render(req_alloc, weather, use_imperial); + if (std.mem.startsWith(u8, fmt, "%")) + break :blk try custom.render(req_alloc, weather, fmt, use_imperial); + // fall back to line if we don't understant the format parameter + break :blk try line.render(req_alloc, weather, fmt, use_imperial); } else try formatted.render(req_alloc, weather, .{ .use_imperial = use_imperial }); // Add coordinates header using response allocator diff --git a/src/location/GeoIp.zig b/src/location/GeoIp.zig index 71838b4..ecc65f8 100644 --- a/src/location/GeoIp.zig +++ b/src/location/GeoIp.zig @@ -7,8 +7,9 @@ const c = @cImport({ }); const GeoIP = @This(); +const log = std.log.scoped(.geoip); -mmdb: c.MMDB_s, +mmdb: *c.MMDB_s, ip2location_client: ?*Ip2location, ip2location_cache: ?*Ip2location.Cache, allocator: std.mem.Allocator, @@ -17,11 +18,13 @@ pub fn init(allocator: std.mem.Allocator, db_path: []const u8, api_key: ?[]const const path_z = try std.heap.c_allocator.dupeZ(u8, db_path); defer std.heap.c_allocator.free(path_z); - // SAFETY: The C API will initialize this on the next line - var mmdb: c.MMDB_s = undefined; - const status = c.MMDB_open(path_z.ptr, c.MMDB_MODE_MMAP, &mmdb); - if (status != c.MMDB_SUCCESS) + const mmdb = try allocator.create(c.MMDB_s); + errdefer allocator.destroy(mmdb); + + const status = c.MMDB_open(path_z.ptr, c.MMDB_MODE_MMAP, mmdb); + if (status != c.MMDB_SUCCESS) { return error.CannotOpenDatabase; + } var client: ?*Ip2location = null; var cache: ?*Ip2location.Cache = null; @@ -50,7 +53,8 @@ pub fn init(allocator: std.mem.Allocator, db_path: []const u8, api_key: ?[]const } pub fn deinit(self: *GeoIP) void { - c.MMDB_close(&self.mmdb); + c.MMDB_close(self.mmdb); + self.allocator.destroy(self.mmdb); if (self.ip2location_client) |client| { client.deinit(); self.allocator.destroy(client); @@ -61,18 +65,18 @@ pub fn deinit(self: *GeoIP) void { } } -pub fn lookup(self: *GeoIP, ip: []const u8) !?Coordinates { +pub fn lookup(self: *GeoIP, ip: []const u8) ?Coordinates { // Try MaxMind first - const result = lookupInternal(&self.mmdb, ip) catch return null; + const result = lookupInternal(self.mmdb, ip) catch return null; - if (result.found_entry) { - return try self.extractCoordinates(result); - } + log.debug("lookup geoip db for ip {s}. Found: {}", .{ ip, result.found_entry }); + if (result.found_entry) + if (self.extractCoordinates(ip, result)) |coords| + return coords; // Fallback to IP2Location if configured - if (self.ip2location_client) |client| { + if (self.ip2location_client) |client| return client.lookupWithCache(ip, self.ip2location_cache); - } return null; } @@ -85,45 +89,69 @@ fn lookupInternal(mmdb: *c.MMDB_s, ip: []const u8) !c.MMDB_lookup_result_s { var mmdb_error: c_int = 0; const result = c.MMDB_lookup_string(mmdb, ip_z.ptr, &gai_error, &mmdb_error); - if (gai_error != 0 or mmdb_error != 0) return error.MMDBLookupError; + if (mmdb_error != 0) { + log.warn("got error on MMDB_lookup_string for ip {s}. gai = {d}, mmdb_error = {d}", .{ ip, gai_error, mmdb_error }); + return error.MMDBLookupError; + } return result; } pub fn isUSIp(self: *GeoIP, ip: []const u8) bool { - const result = lookupInternal(&self.mmdb, ip) catch return false; + var result = lookupInternal(self.mmdb, ip) catch return false; if (!result.found_entry) return false; - var entry_mut = result.entry; - const null_term: [*:0]const u8 = @ptrCast(&[_]u8{0}); // 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(&entry_mut, &country_data, "country\x00", "iso_code\x00", null_term); + const status = c.MMDB_get_value(&result.entry, &country_data, "country", "iso_code", @as([*c]const u8, null)); - if (status != c.MMDB_SUCCESS or !country_data.has_data) + 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; + } - const country_code = std.mem.span(country_data.unnamed_0.utf8_string); + const country_code = country_data.unnamed_0.utf8_string[0..country_data.data_size]; return std.mem.eql(u8, country_code, "US"); } -fn extractCoordinates(self: *GeoIP, result: c.MMDB_lookup_result_s) !Coordinates { +fn extractCoordinates(self: *GeoIP, ip: []const u8, result: c.MMDB_lookup_result_s) ?Coordinates { _ = self; - var entry_mut = result.entry; - // SAFETY: The C API will initialize below + if (!result.found_entry) return null; + + var entry_copy = result.entry; + + // SAFETY: latitude_data set by MMDB_get_value var latitude_data: c.MMDB_entry_data_s = undefined; - // SAFETY: The C API will initialize below + const lat_status = c.MMDB_get_value(&entry_copy, &latitude_data, "location", "latitude", @as([*c]const u8, null)); + + if (lat_status != c.MMDB_SUCCESS or !latitude_data.has_data) { + log.info("lookup found result, but no latitude available in data for ip {s}. MMDB_get_value returned {d}", .{ ip, lat_status }); + return null; + } + + // SAFETY: longitude_data set by MMDB_get_value var longitude_data: c.MMDB_entry_data_s = undefined; + const lon_status = c.MMDB_get_value(&entry_copy, &longitude_data, "location", "longitude", @as([*c]const u8, null)); - const lat_status = c.MMDB_get_value(&entry_mut, &latitude_data, "location", "latitude", @as([*:0]const u8, @ptrCast(&[_]u8{0}))); - const lon_status = c.MMDB_get_value(&entry_mut, &longitude_data, "location", "longitude", @as([*:0]const u8, @ptrCast(&[_]u8{0}))); + if (lon_status != c.MMDB_SUCCESS or !longitude_data.has_data) { + log.info("lookup found result, but no longitude available in data for ip {s}. MMDB_get_value returned {d}", .{ ip, lon_status }); + return null; + } - if (lat_status != c.MMDB_SUCCESS or lon_status != c.MMDB_SUCCESS or !latitude_data.has_data or !longitude_data.has_data) - return error.CoordinatesNotFound; + var coords = [_]f64{ latitude_data.unnamed_0.double_value, longitude_data.unnamed_0.double_value }; + + // Depending on how this is compiled, the byteswap may or may not be necessary + // original c, compiled with zig, statically linked: byteSwap + // pre=built, dynamically linked, do not byte swap + // I'm not sure precisely what causes this + std.mem.byteSwapAllElements(f64, &coords); + + const latitude = coords[0]; + const longitude = coords[1]; return .{ - .latitude = latitude_data.unnamed_0.double_value, - .longitude = longitude_data.unnamed_0.double_value, + .latitude = latitude, + .longitude = longitude, }; } @@ -156,10 +184,35 @@ test "isUSIP detects US IPs" { defer geoip.deinit(); // Test that the function doesn't crash with various IPs - _ = geoip.isUSIp("8.8.8.8"); - _ = geoip.isUSIp("1.1.1.1"); + try std.testing.expect(geoip.isUSIp("73.158.64.1")); // Test invalid IP returns false const invalid = geoip.isUSIp("invalid"); try std.testing.expect(!invalid); } +test "lookup works" { + const allocator = std.testing.allocator; + const Config = @import("../Config.zig"); + const config = try Config.load(allocator); + defer config.deinit(allocator); + const build_options = @import("build_options"); + const db_path = config.geolite_path; + + if (build_options.download_geoip) { + const GeoLite2 = @import("GeoLite2.zig"); + try GeoLite2.ensureDatabase(std.testing.allocator, db_path); + } + + var geoip = GeoIP.init(std.testing.allocator, db_path, null, null) catch { + return error.SkipZigTest; + }; + defer geoip.deinit(); + + // Test that the function doesn't crash with various IPs + const maybe_coords = geoip.lookup("73.158.64.1"); + + try std.testing.expect(maybe_coords != null); + + const coords = maybe_coords.?; + try std.testing.expectEqual(@as(f64, 37.5958), coords.latitude); +} diff --git a/src/location/Ip2location.zig b/src/location/Ip2location.zig index 54bd002..3fd691d 100644 --- a/src/location/Ip2location.zig +++ b/src/location/Ip2location.zig @@ -23,7 +23,7 @@ pub fn deinit(self: *Self) void { self.allocator.free(self.api_key); } -pub fn lookupWithCache(self: *Self, ip_str: []const u8, cache: ?*Cache) !?Coordinates { +pub fn lookupWithCache(self: *Self, ip_str: []const u8, cache: ?*Cache) ?Coordinates { // Parse IP to u128 for cache lookup const addr = std.net.Address.parseIp(ip_str, 0) catch return null; const ip_u128: u128 = switch (addr.any.family) { @@ -59,6 +59,7 @@ pub fn lookupWithCache(self: *Self, ip_str: []const u8, cache: ?*Cache) !?Coordi pub fn lookup(self: *Self, ip_str: []const u8) !Coordinates { log.info("Fetching geolocation for IP {s}", .{ip_str}); + if (@import("builtin").is_test) return error.LookupUnavailableInUnitTest; // Build URL: https://api.ip2location.io/?key=XXX&ip=1.2.3.4 const url = try std.fmt.allocPrint( self.allocator, diff --git a/src/location/resolver.zig b/src/location/resolver.zig index 02dcf53..789247c 100644 --- a/src/location/resolver.zig +++ b/src/location/resolver.zig @@ -9,6 +9,11 @@ const log = std.log.scoped(.resolver); pub const Location = struct { name: []const u8, coords: Coordinates, + allocator: std.mem.Allocator, + + pub fn deinit(self: Location) void { + self.allocator.free(self.name); + } }; pub const LocationType = enum { @@ -37,6 +42,7 @@ pub const Resolver = struct { pub fn resolve(self: *Resolver, query: []const u8) !Location { const location_type = detectType(query); + log.debug("location type: {}", .{location_type}); return switch (location_type) { .ip_address => try self.resolveIP(query), .domain_name => try self.resolveDomain(query[1..]), // Skip '@' @@ -57,8 +63,9 @@ pub const Resolver = struct { fn resolveIP(self: *Resolver, ip: []const u8) !Location { if (self.geoip) |geoip| { - if (try geoip.lookup(ip)) |coords| { + if (geoip.lookup(ip)) |coords| { return .{ + .allocator = self.allocator, .name = try self.allocator.dupe(u8, ip), .coords = coords, }; @@ -91,12 +98,14 @@ pub const Resolver = struct { // Check cache first if (self.geocache.get(name)) |cached| { return Location{ + .allocator = self.allocator, .name = try self.allocator.dupe(u8, cached.name), .coords = cached.coords, }; } log.info("Calling nominatim (OpenStreetMap) to resolve place name {s} to coordinates", .{name}); + if (@import("builtin").is_test) return error.GeocodingUnavailableInUnitTest; // Call Nominatim API const url = try std.fmt.allocPrint( self.allocator, @@ -158,7 +167,8 @@ pub const Resolver = struct { }, }); - return Location{ + return .{ + .allocator = self.allocator, .name = try self.allocator.dupe(u8, display_name), .coords = .{ .latitude = lat, @@ -178,6 +188,7 @@ pub const Resolver = struct { if (airports.lookup(&upper_code)) |airport| { return Location{ + .allocator = self.allocator, .name = try self.allocator.dupe(u8, airport.name), .coords = airport.coords, }; @@ -230,3 +241,35 @@ test "resolver init" { try std.testing.expect(resolver.geoip == null); try std.testing.expect(resolver.airports == null); } + +test "resolve IP address with GeoIP" { + const allocator = std.testing.allocator; + const Config = @import("../Config.zig"); + const config = try Config.load(allocator); + defer config.deinit(allocator); + + const build_options = @import("build_options"); + if (build_options.download_geoip) { + const GeoLite2 = @import("GeoLite2.zig"); + try GeoLite2.ensureDatabase(allocator, config.geolite_path); + } + + var geoip = GeoIp.init(allocator, config.geolite_path, null, null) catch { + return error.SkipZigTest; + }; + defer geoip.deinit(); + + var geocache = try GeoCache.init(allocator, null); + defer geocache.deinit(); + + var resolver = Resolver.init(allocator, &geoip, &geocache, null); + + // Use IP that's known to have coordinates in GeoLite2 database + const test_ip = "73.158.64.1"; + + const location = try resolver.resolve(test_ip); + defer location.deinit(); + + try std.testing.expectEqualStrings(test_ip, location.name); + try std.testing.expect(location.coords.latitude != 0 or location.coords.longitude != 0); +} diff --git a/src/weather/Mock.zig b/src/weather/Mock.zig index 6467344..a38b172 100644 --- a/src/weather/Mock.zig +++ b/src/weather/Mock.zig @@ -40,7 +40,8 @@ fn fetchRaw(ptr: *anyopaque, allocator: std.mem.Allocator, coords: Coordinates) const key = try std.fmt.allocPrint(allocator, "{d:.4},{d:.4}", .{ coords.latitude, coords.longitude }); defer allocator.free(key); - const raw = self.responses.get(key) orelse return error.LocationNotFound; + // Try exact match first, then wildcard + const raw = self.responses.get(key) orelse self.responses.get("*") orelse return error.LocationNotFound; return try allocator.dupe(u8, raw); }