From 523e9fdcadc97fc2d21507e9c47dca87d33b067f Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Tue, 6 Jan 2026 22:57:39 -0800 Subject: [PATCH] Determine proper client IP prior to rate limiting, then pass it through --- src/http/Server.zig | 64 +++++++++++++++++++++++++++++++++++++------- src/http/handler.zig | 51 +---------------------------------- 2 files changed, 55 insertions(+), 60 deletions(-) diff --git a/src/http/Server.zig b/src/http/Server.zig index cd87ed7..0a63ecf 100644 --- a/src/http/Server.zig +++ b/src/http/Server.zig @@ -69,16 +69,37 @@ pub fn init( } fn handleWeather(ctx: *Context, req: *httpz.Request, res: *httpz.Response) !void { - try rateLimitMiddleware(ctx.rate_limiter, req, res); + var client_ip_buf: [47]u8 = undefined; + const client_ip = try getClientIp(req, &client_ip_buf); + try rateLimitMiddleware(ctx.rate_limiter, client_ip, res); if (res.status == 429) return; - try handler.handleWeather(&ctx.options, req, res); + try handler.handleWeather(&ctx.options, req, res, client_ip); } -fn rateLimitMiddleware(limiter: *RateLimiter, req: *httpz.Request, res: *httpz.Response) !void { - var ip_buf: [45]u8 = undefined; - const ip_str = try std.fmt.bufPrint(&ip_buf, "{f}", .{req.address}); +fn getClientIp(req: *httpz.Request, buf: []u8) ![]const u8 { + // Check X-Forwarded-For header first (for proxies) + if (req.header("x-forwarded-for")) |xff| { + return parseXForwardedFor(xff); + } - if (!limiter.shouldAcceptRequest(ip_str)) { + // Check X-Real-IP header + if (req.header("x-real-ip")) |real_ip| { + return real_ip; + } + + // Fall back to connection address + const req_addr = try std.fmt.bufPrint(buf, "{f}", .{req.address}); + return req_addr[0..std.mem.lastIndexOfScalar(u8, req_addr, ':').?]; +} + +fn parseXForwardedFor(xff: []const u8) []const u8 { + // Take first IP from comma-separated list + var iter = std.mem.splitScalar(u8, xff, ','); + return std.mem.trim(u8, iter.first(), " \t"); +} + +fn rateLimitMiddleware(limiter: *RateLimiter, client_ip: []const u8, res: *httpz.Response) !void { + if (!limiter.shouldAcceptRequest(client_ip)) { res.status = 429; res.body = "Too Many Requests"; } @@ -215,7 +236,9 @@ test "handleWeather: default endpoint uses IP address" { ht.url("/"); ht.header("x-forwarded-for", "73.158.64.1"); - try handler.handleWeather(&harness.opts, ht.req, ht.res); + var client_ip_buf: [47]u8 = undefined; + const client_ip = try getClientIp(ht.req, &client_ip_buf); + try handler.handleWeather(&harness.opts, ht.req, ht.res, client_ip); try ht.expectStatus(200); } @@ -231,7 +254,10 @@ test "handleWeather: x-forwarded-for with multiple IPs" { 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); + var client_ip_buf: [47]u8 = undefined; + const client_ip = try getClientIp(ht.req, &client_ip_buf); + try std.testing.expectEqualStrings("73.158.64.1", client_ip); + try handler.handleWeather(&harness.opts, ht.req, ht.res, client_ip); try ht.expectStatus(200); } @@ -245,11 +271,29 @@ test "handleWeather: client IP only" { 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.req.address = try std.net.Address.parseIp("73.158.64.1", 0); ht.url("/"); - try handler.handleWeather(&harness.opts, ht.req, ht.res); + var client_ip_buf: [47]u8 = undefined; + const client_ip = try getClientIp(ht.req, &client_ip_buf); + try std.testing.expectEqualStrings("73.158.64.1", client_ip); + try handler.handleWeather(&harness.opts, ht.req, ht.res, client_ip); try ht.expectStatus(200); } + +test "parseXForwardedFor extracts first IP" { + try std.testing.expectEqualStrings("192.168.1.1", parseXForwardedFor("192.168.1.1")); + try std.testing.expectEqualStrings("10.0.0.1", parseXForwardedFor("10.0.0.1, 172.16.0.1")); + try std.testing.expectEqualStrings("203.0.113.1", parseXForwardedFor("203.0.113.1, 198.51.100.1, 192.0.2.1")); +} + +test "parseXForwardedFor trims whitespace" { + try std.testing.expectEqualStrings("192.168.1.1", parseXForwardedFor(" 192.168.1.1 ")); + try std.testing.expectEqualStrings("10.0.0.1", parseXForwardedFor(" 10.0.0.1 , 172.16.0.1")); +} + +test "parseXForwardedFor handles empty string" { + try std.testing.expectEqualStrings("", parseXForwardedFor("")); +} diff --git a/src/http/handler.zig b/src/http/handler.zig index cd6e170..019a246 100644 --- a/src/http/handler.zig +++ b/src/http/handler.zig @@ -22,18 +22,8 @@ pub fn handleWeather( opts: *HandleWeatherOptions, req: *httpz.Request, res: *httpz.Response, + client_ip: []const u8, ) !void { - var client_ip_buf: [47]u8 = undefined; - // We need IP possibly for location, and possibly to determine implicit Imperial/Metric - // Let's get it once here - const client_ip = blk: { - const client_ip = getClientIpFromHeaders(req); - if (client_ip.len == 0) { - const full_location = try std.fmt.bufPrint(&client_ip_buf, "{f}", .{req.conn.address}); - break :blk full_location[0..std.mem.lastIndexOf(u8, full_location, ":").?]; - } - break :blk client_ip; - }; // Get location from path parameter or query string const location = req.param("location") orelse blk: { // Check query string for location parameter @@ -75,30 +65,6 @@ pub fn handleWeather( try handleWeatherInternal(opts, req, res, location, client_ip); } -fn getClientIpFromHeaders(req: *httpz.Request) []const u8 { - // Check X-Forwarded-For header first (for proxies) - if (req.header("x-forwarded-for")) |xff| { - return parseXForwardedFor(xff); - } - - // Check X-Real-IP header - if (req.header("x-real-ip")) |real_ip| { - return real_ip; - } - - // Fall back to client connection - return ""; -} - -fn parseXForwardedFor(xff: []const u8) []const u8 { - // Take first IP from comma-separated list - var iter = std.mem.splitScalar(u8, xff, ','); - if (iter.next()) |first_ip| { - return std.mem.trim(u8, first_ip, " \t"); - } - return ""; -} - fn handleWeatherInternal( opts: *HandleWeatherOptions, req: *httpz.Request, @@ -229,21 +195,6 @@ fn determineFormat(params: QueryParams, user_agent: ?[]const u8) Formatted.Forma return .html; } -test "parseXForwardedFor extracts first IP" { - try std.testing.expectEqualStrings("192.168.1.1", parseXForwardedFor("192.168.1.1")); - try std.testing.expectEqualStrings("10.0.0.1", parseXForwardedFor("10.0.0.1, 172.16.0.1")); - try std.testing.expectEqualStrings("203.0.113.1", parseXForwardedFor("203.0.113.1, 198.51.100.1, 192.0.2.1")); -} - -test "parseXForwardedFor trims whitespace" { - try std.testing.expectEqualStrings("192.168.1.1", parseXForwardedFor(" 192.168.1.1 ")); - try std.testing.expectEqualStrings("10.0.0.1", parseXForwardedFor(" 10.0.0.1 , 172.16.0.1")); -} - -test "parseXForwardedFor handles empty string" { - try std.testing.expectEqualStrings("", parseXForwardedFor("")); -} - test "imperial units selection logic" { // This test documents the priority order for unit selection: // 1. Explicit ?u or ?m parameter (highest priority)