From 9eb254d5f8b207c204d9e925d7c7cb3582144752 Mon Sep 17 00:00:00 2001 From: Rene Schallner Date: Fri, 21 Mar 2025 11:45:13 +0100 Subject: [PATCH] get rid of zap.util.FreeOrNot --- examples/accept/accept.zig | 13 +- examples/bindataformpost/bindataformpost.zig | 31 +-- examples/cookies/cookies.zig | 24 +- examples/hello2/hello2.zig | 13 +- examples/hello_json/hello_json.zig | 1 + examples/http_params/http_params.zig | 28 +-- examples/simple_router/simple_router.zig | 13 +- .../userpass_session_auth.zig | 2 + src/fio.zig | 4 +- src/http_auth.zig | 43 ++-- src/request.zig | 227 +++++++++--------- src/tests/test_http_params.zig | 97 ++++---- src/util.zig | 45 ++-- 13 files changed, 278 insertions(+), 263 deletions(-) diff --git a/examples/accept/accept.zig b/examples/accept/accept.zig index a9e268c..fc9f766 100644 --- a/examples/accept/accept.zig +++ b/examples/accept/accept.zig @@ -66,7 +66,18 @@ pub fn main() !void { }); try listener.listen(); - std.debug.print("Listening on 0.0.0.0:3000\n", .{}); + std.debug.print( + \\ Listening on 0.0.0.0:3000 + \\ + \\ Test me with: + \\ curl --header "Accept: text/plain" localhost:3000 + \\ curl --header "Accept: text/html" localhost:3000 + \\ curl --header "Accept: application/xml" localhost:3000 + \\ curl --header "Accept: application/json" localhost:3000 + \\ curl --header "Accept: application/xhtml+xml" localhost:3000 + \\ + \\ + , .{}); // start worker threads zap.start(.{ diff --git a/examples/bindataformpost/bindataformpost.zig b/examples/bindataformpost/bindataformpost.zig index 9cc0eda..c1d5fe8 100644 --- a/examples/bindataformpost/bindataformpost.zig +++ b/examples/bindataformpost/bindataformpost.zig @@ -24,12 +24,12 @@ const Handler = struct { // // HERE WE HANDLE THE BINARY FILE // - const params = try r.parametersToOwnedList(Handler.alloc, false); + const params = try r.parametersToOwnedList(Handler.alloc); defer params.deinit(); for (params.items) |kv| { if (kv.value) |v| { std.debug.print("\n", .{}); - std.log.info("Param `{s}` in owned list is {any}\n", .{ kv.key.str, v }); + std.log.info("Param `{s}` in owned list is {any}\n", .{ kv.key, v }); switch (v) { // single-file upload zap.Request.HttpParam.Hash_Binfile => |*file| { @@ -55,32 +55,20 @@ const Handler = struct { files.*.deinit(); }, else => { - // might be a string param, we don't care - // let's just get it as string - // always_alloc param = false -> the string will be a slice from the request buffer - // --> no deinit necessary - if (r.getParamStr(Handler.alloc, kv.key.str, false)) |maybe_str| { - const value: []const u8 = if (maybe_str) |s| s.str else "(no value)"; - // above, we didn't defer s.deinit because the string is just a slice from the request buffer - std.log.debug(" {s} = {s}", .{ kv.key.str, value }); - } else |err| { - std.log.err("Error: {any}\n", .{err}); - } + // let's just get it as its raw slice + const value: []const u8 = r.getParamSlice(kv.key) orelse "(no value)"; + std.log.debug(" {s} = {s}", .{ kv.key, value }); }, } } } // check if we received a terminate=true parameter - if (r.getParamStr(Handler.alloc, "terminate", false)) |maybe_str| { - if (maybe_str) |*s| { - std.log.info("?terminate={s}\n", .{s.str}); - if (std.mem.eql(u8, s.str, "true")) { - zap.stop(); - } + if (r.getParamSlice("terminate")) |str| { + std.log.info("?terminate={s}\n", .{str}); + if (std.mem.eql(u8, str, "true")) { + zap.stop(); } - } else |err| { - std.log.err("cannot check for terminate param: {any}\n", .{err}); } try r.sendJson("{ \"ok\": true }"); } @@ -90,6 +78,7 @@ pub fn main() !void { var gpa = std.heap.GeneralPurposeAllocator(.{ .thread_safe = true, }){}; + defer _ = gpa.detectLeaks(); const allocator = gpa.allocator(); Handler.alloc = allocator; diff --git a/examples/cookies/cookies.zig b/examples/cookies/cookies.zig index 16fc9a8..22d8e18 100644 --- a/examples/cookies/cookies.zig +++ b/examples/cookies/cookies.zig @@ -44,12 +44,12 @@ pub fn main() !void { const cookie_count = r.getCookiesCount(); std.log.info("cookie_count: {}", .{cookie_count}); - // iterate over all cookies as strings (always_alloc=false) - var strCookies = r.cookiesToOwnedStrList(alloc, false) catch unreachable; + // iterate over all cookies as strings + var strCookies = r.cookiesToOwnedStrList(alloc) catch unreachable; defer strCookies.deinit(); std.debug.print("\n", .{}); for (strCookies.items) |kv| { - std.log.info("CookieStr `{s}` is `{s}`", .{ kv.key.str, kv.value.str }); + std.log.info("CookieStr `{s}` is `{s}`", .{ kv.key, kv.value }); // we don't need to deinit kv.key and kv.value because we requested always_alloc=false // so they are just slices into the request buffer } @@ -57,26 +57,22 @@ pub fn main() !void { std.debug.print("\n", .{}); // // iterate over all cookies - const cookies = r.cookiesToOwnedList(alloc, false) catch unreachable; + const cookies = r.cookiesToOwnedList(alloc) catch unreachable; defer cookies.deinit(); for (cookies.items) |kv| { - std.log.info("cookie `{s}` is {any}", .{ kv.key.str, kv.value }); + std.log.info("cookie `{s}` is {any}", .{ kv.key, kv.value }); } // let's get cookie "ZIG_ZAP" by name std.debug.print("\n", .{}); - if (r.getCookieStr(alloc, "ZIG_ZAP", false)) |maybe_str| { - if (maybe_str) |*s| { - defer s.deinit(); // unnecessary because always_alloc=false - - std.log.info("Cookie ZIG_ZAP = {s}", .{s.str}); + if (r.getCookieStr(alloc, "ZIG_ZAP")) |maybe_str| { + if (maybe_str) |s| { + defer alloc.free(s); + std.log.info("Cookie ZIG_ZAP = {s}", .{s}); } else { std.log.info("Cookie ZIG_ZAP not found!", .{}); } - } - // since we provided "false" for duplicating strings in the call - // to getCookieStr(), there won't be an allocation error - else |err| { + } else |err| { std.log.err("ERROR!\n", .{}); std.log.err("cannot check for `ZIG_ZAP` cookie: {any}\n", .{err}); } diff --git a/examples/hello2/hello2.zig b/examples/hello2/hello2.zig index 51e8faa..5302c4c 100644 --- a/examples/hello2/hello2.zig +++ b/examples/hello2/hello2.zig @@ -43,7 +43,18 @@ pub fn main() !void { }); try listener.listen(); - std.debug.print("Listening on 0.0.0.0:3000\n", .{}); + std.debug.print( + \\ Listening on 0.0.0.0:3000 + \\ + \\ Test me with: + \\ curl http://localhost:3000 + \\ curl --header "special-header: test" localhost:3000 + \\ + \\ ... or open http://localhost:3000 in the browser + \\ and watch the log output here + \\ + \\ + , .{}); // start worker threads zap.start(.{ diff --git a/examples/hello_json/hello_json.zig b/examples/hello_json/hello_json.zig index 21f9f46..af693ef 100644 --- a/examples/hello_json/hello_json.zig +++ b/examples/hello_json/hello_json.zig @@ -43,6 +43,7 @@ fn setupUserData(a: std.mem.Allocator) !void { pub fn main() !void { const a = std.heap.page_allocator; try setupUserData(a); + defer users.deinit(); var listener = zap.HttpListener.init(.{ .port = 3000, .on_request = on_request, diff --git a/examples/http_params/http_params.zig b/examples/http_params/http_params.zig index 66976f6..4034eaf 100644 --- a/examples/http_params/http_params.zig +++ b/examples/http_params/http_params.zig @@ -27,6 +27,8 @@ pub fn main() !void { var gpa = std.heap.GeneralPurposeAllocator(.{ .thread_safe = true, }){}; + defer _ = gpa.detectLeaks(); + const allocator = gpa.allocator(); const Handler = struct { @@ -69,42 +71,38 @@ pub fn main() !void { // ================================================================ // iterate over all params as strings - var strparams = try r.parametersToOwnedStrList(alloc, false); + var strparams = try r.parametersToOwnedStrList(alloc); defer strparams.deinit(); std.debug.print("\n", .{}); for (strparams.items) |kv| { - std.log.info("ParamStr `{s}` is `{s}`", .{ kv.key.str, kv.value.str }); + std.log.info("ParamStr `{s}` is `{s}`", .{ kv.key, kv.value }); } std.debug.print("\n", .{}); // iterate over all params - const params = try r.parametersToOwnedList(alloc, false); + const params = try r.parametersToOwnedList(alloc); defer params.deinit(); for (params.items) |kv| { - std.log.info("Param `{s}` is {any}", .{ kv.key.str, kv.value }); + std.log.info("Param `{s}` is {any}", .{ kv.key, kv.value }); } // let's get param "one" by name std.debug.print("\n", .{}); - if (r.getParamStr(alloc, "one", false)) |maybe_str| { - if (maybe_str) |*s| { - defer s.deinit(); - - std.log.info("Param one = {s}", .{s.str}); + if (r.getParamStr(alloc, "one")) |maybe_str| { + if (maybe_str) |s| { + defer alloc.free(s); + std.log.info("Param one = {s}", .{s}); } else { std.log.info("Param one not found!", .{}); } - } - // since we provided "false" for duplicating strings in the call - // to getParamStr(), there won't be an allocation error - else |err| { + } else |err| { std.log.err("cannot check for `one` param: {any}\n", .{err}); } // check if we received a terminate=true parameter - if (r.getParamSlice("terminate")) |maybe_str| { - if (std.mem.eql(u8, maybe_str, "true")) { + if (r.getParamSlice("terminate")) |s| { + if (std.mem.eql(u8, s, "true")) { zap.stop(); } } diff --git a/examples/simple_router/simple_router.zig b/examples/simple_router/simple_router.zig index cacbcc3..f91ed2c 100644 --- a/examples/simple_router/simple_router.zig +++ b/examples/simple_router/simple_router.zig @@ -98,8 +98,17 @@ pub fn main() !void { }); try listener.listen(); - std.debug.print("Listening on 0.0.0.0:3000\n", .{}); - + std.debug.print( + \\ Listening on 0.0.0.0:3000 + \\ + \\ Test me with: + \\ curl http://localhost:3000/ + \\ curl http://localhost:3000/geta + \\ curl http://localhost:3000/getb + \\ curl http://localhost:3000/inca + \\ + \\ + , .{}); // start worker threads zap.start(.{ .threads = 2, diff --git a/examples/userpass_session_auth/userpass_session_auth.zig b/examples/userpass_session_auth/userpass_session_auth.zig index c421ddc..fcf7744 100644 --- a/examples/userpass_session_auth/userpass_session_auth.zig +++ b/examples/userpass_session_auth/userpass_session_auth.zig @@ -72,6 +72,7 @@ fn on_request(r: zap.Request) !void { .AuthOK => { // the authenticator says it is ok to proceed as usual std.log.info("Auth OK", .{}); + // dispatch to target path if (r.path) |p| { // used in the login page @@ -124,6 +125,7 @@ pub fn main() !void { // to detect leaks { const allocator = gpa.allocator(); + var listener = zap.HttpListener.init(.{ .port = 3000, .on_request = on_request, diff --git a/src/fio.zig b/src/fio.zig index 7696989..4b6b3cd 100644 --- a/src/fio.zig +++ b/src/fio.zig @@ -13,7 +13,7 @@ pub const fio_url_s = extern struct { pub extern fn fio_url_parse(url: [*c]const u8, length: usize) fio_url_s; /// Negative thread / worker values indicate a fraction of the number of CPU cores. i.e., -2 will normally indicate "half" (1/2) the number of cores. -/// +/// /// If one value is set to zero, it will be the absolute value of the other value. i.e.: if .threads == -2 and .workers == 0, than facil.io will run 2 worker processes with (cores/2) threads per process. pub const struct_fio_start_args = extern struct { /// The number of threads to run in the thread pool. @@ -420,7 +420,9 @@ pub fn fiobj_obj2cstr(o: FIOBJ) callconv(.C) fio_str_info_s { // pub const http_cookie_args_s = opaque {}; pub extern fn http_set_header(h: [*c]http_s, name: FIOBJ, value: FIOBJ) c_int; +/// set header, copying the data pub extern fn http_set_header2(h: [*c]http_s, name: fio_str_info_s, value: fio_str_info_s) c_int; +/// set cookie, taking ownership of data pub extern fn http_set_cookie(h: [*c]http_s, http_cookie_args_s) c_int; pub extern fn http_sendfile(h: [*c]http_s, fd: c_int, length: usize, offset: usize) c_int; pub extern fn http_sendfile2(h: [*c]http_s, prefix: [*c]const u8, prefix_len: usize, encoded: [*c]const u8, encoded_len: usize) c_int; diff --git a/src/http_auth.zig b/src/http_auth.zig index 894b74e..e4b9fa3 100644 --- a/src/http_auth.zig +++ b/src/http_auth.zig @@ -450,15 +450,15 @@ pub fn UserPassSession(comptime Lookup: type, comptime lockedPwLookups: bool) ty r.parseCookies(false); // check for session cookie - if (r.getCookieStr(self.allocator, self.settings.cookieName, false)) |maybe_cookie| { + if (r.getCookieStr(self.allocator, self.settings.cookieName)) |maybe_cookie| { if (maybe_cookie) |cookie| { - defer cookie.deinit(); + defer self.allocator.free(cookie); self.tokenLookupLock.lock(); defer self.tokenLookupLock.unlock(); - if (self.sessionTokens.getKeyPtr(cookie.str)) |keyPtr| { + if (self.sessionTokens.getKeyPtr(cookie)) |keyPtr| { const keySlice = keyPtr.*; // if cookie is a valid session, remove it! - _ = self.sessionTokens.remove(cookie.str); + _ = self.sessionTokens.remove(cookie); // only now can we let go of the cookie str slice that // was used as the key self.allocator.free(keySlice); @@ -486,18 +486,18 @@ pub fn UserPassSession(comptime Lookup: type, comptime lockedPwLookups: bool) ty r.parseCookies(false); // check for session cookie - if (r.getCookieStr(self.allocator, self.settings.cookieName, false)) |maybe_cookie| { + if (r.getCookieStr(self.allocator, self.settings.cookieName)) |maybe_cookie| { if (maybe_cookie) |cookie| { - defer cookie.deinit(); + defer self.allocator.free(cookie); // locked or unlocked token lookup self.tokenLookupLock.lock(); defer self.tokenLookupLock.unlock(); - if (self.sessionTokens.contains(cookie.str)) { + if (self.sessionTokens.contains(cookie)) { // cookie is a valid session! - zap.debug("Auth: COOKIE IS OK!!!!: {s}\n", .{cookie.str}); + zap.debug("Auth: COOKIE IS OK!!!!: {s}\n", .{cookie}); return .AuthOK; } else { - zap.debug("Auth: COOKIE IS BAD!!!!: {s}\n", .{cookie.str}); + zap.debug("Auth: COOKIE IS BAD!!!!: {s}\n", .{cookie}); // this is not necessarily a bad thing. it could be a // stale cookie from a previous session. So let's check // if username and password are being sent and correct. @@ -508,27 +508,26 @@ pub fn UserPassSession(comptime Lookup: type, comptime lockedPwLookups: bool) ty } // get params of username and password - if (r.getParamStr(self.allocator, self.settings.usernameParam, false)) |maybe_username| { - if (maybe_username) |*username| { - defer username.deinit(); - if (r.getParamStr(self.allocator, self.settings.passwordParam, false)) |maybe_pw| { - if (maybe_pw) |*pw| { - defer pw.deinit(); - + if (r.getParamStr(self.allocator, self.settings.usernameParam)) |maybe_username| { + if (maybe_username) |username| { + defer self.allocator.free(username); + if (r.getParamStr(self.allocator, self.settings.passwordParam)) |maybe_pw| { + if (maybe_pw) |pw| { + defer self.allocator.free(pw); // now check const correct_pw_optional = brk: { if (lockedPwLookups) { self.passwordLookupLock.lock(); defer self.passwordLookupLock.unlock(); - break :brk self.lookup.*.get(username.str); + break :brk self.lookup.*.get(username); } else { - break :brk self.lookup.*.get(username.str); + break :brk self.lookup.*.get(username); } }; if (correct_pw_optional) |correct_pw| { - if (std.mem.eql(u8, pw.str, correct_pw)) { + if (std.mem.eql(u8, pw, correct_pw)) { // create session token - if (self.createAndStoreSessionToken(username.str, pw.str)) |token| { + if (self.createAndStoreSessionToken(username, pw)) |token| { defer self.allocator.free(token); // now set the cookie header if (r.setCookie(.{ @@ -549,12 +548,12 @@ pub fn UserPassSession(comptime Lookup: type, comptime lockedPwLookups: bool) ty } } } else |err| { - zap.debug("getParamSt() for password failed in UserPassSession: {any}", .{err}); + zap.debug("getParamStr() for password failed in UserPassSession: {any}", .{err}); return .AuthFailed; } } } else |err| { - zap.debug("getParamSt() for user failed in UserPassSession: {any}", .{err}); + zap.debug("getParamStr() for user failed in UserPassSession: {any}", .{err}); return .AuthFailed; } return .AuthFailed; diff --git a/src/request.zig b/src/request.zig index a0c2a7e..3fcc0ee 100644 --- a/src/request.zig +++ b/src/request.zig @@ -1,4 +1,6 @@ const std = @import("std"); +const Allocator = std.mem.Allocator; + const Log = @import("log.zig"); const http = @import("http.zig"); const fio = @import("fio.zig"); @@ -20,21 +22,18 @@ pub const HttpError = error{ /// Key value pair of strings from HTTP parameters pub const HttpParamStrKV = struct { - key: util.FreeOrNot, - value: util.FreeOrNot, - pub fn deinit(self: *@This()) void { - self.key.deinit(); - self.value.deinit(); - } + key: []const u8, + value: []const u8, }; /// List of key value pairs of Http param strings. pub const HttpParamStrKVList = struct { items: []HttpParamStrKV, - allocator: std.mem.Allocator, + allocator: Allocator, pub fn deinit(self: *@This()) void { - for (self.items) |*item| { - item.deinit(); + for (self.items) |item| { + self.allocator.free(item.key); + self.allocator.free(item.value); } self.allocator.free(self.items); } @@ -43,10 +42,13 @@ pub const HttpParamStrKVList = struct { /// List of key value pairs of Http params (might be of different types). pub const HttpParamKVList = struct { items: []HttpParamKV, - allocator: std.mem.Allocator, + allocator: Allocator, pub fn deinit(self: *const @This()) void { - for (self.items) |*item| { - item.deinit(); + for (self.items) |item| { + self.allocator.free(item.key); + if (item.value) |v| { + v.free(self.allocator); + } } self.allocator.free(self.items); } @@ -70,28 +72,31 @@ pub const HttpParam = union(HttpParamValueType) { Int: isize, Float: f64, /// we don't do writable strings here - String: util.FreeOrNot, + String: []const u8, /// value will always be null Unsupported: ?void, /// we assume hashes are because of file transmissions Hash_Binfile: HttpParamBinaryFile, /// value will always be null Array_Binfile: std.ArrayList(HttpParamBinaryFile), + + pub fn free(self: HttpParam, alloc: Allocator) void { + switch (self) { + .String => |s| alloc.free(s), + .Array_Binfile => |a| { + a.deinit(); + }, + else => { + // nothing to free + }, + } + } }; /// Key value pair of one typed Http param pub const HttpParamKV = struct { - key: util.FreeOrNot, + key: []const u8, value: ?HttpParam, - pub fn deinit(self: *@This()) void { - self.key.deinit(); - if (self.value) |p| { - switch (p) { - .String => |*s| s.deinit(), - else => {}, - } - } - } }; /// Struct representing an uploaded file. @@ -112,7 +117,7 @@ pub const HttpParamBinaryFile = struct { } }; -fn parseBinfilesFrom(a: std.mem.Allocator, o: fio.FIOBJ) !HttpParam { +fn parseBinfilesFrom(a: Allocator, o: fio.FIOBJ) !HttpParam { const key_name = fio.fiobj_str_new("name", 4); const key_data = fio.fiobj_str_new("data", 4); const key_type = fio.fiobj_str_new("type", 4); @@ -225,14 +230,15 @@ fn parseBinfilesFrom(a: std.mem.Allocator, o: fio.FIOBJ) !HttpParam { } /// Parse FIO object into a typed Http param. Supports file uploads. -pub fn Fiobj2HttpParam(a: std.mem.Allocator, o: fio.FIOBJ, dupe_string: bool) !?HttpParam { +/// Allocator is only used for file uploads. +pub fn fiobj2HttpParam(a: Allocator, o: fio.FIOBJ) !?HttpParam { return switch (fio.fiobj_type(o)) { fio.FIOBJ_T_NULL => null, fio.FIOBJ_T_TRUE => .{ .Bool = true }, fio.FIOBJ_T_FALSE => .{ .Bool = false }, fio.FIOBJ_T_NUMBER => .{ .Int = fio.fiobj_obj2num(o) }, fio.FIOBJ_T_FLOAT => .{ .Float = fio.fiobj_obj2float(o) }, - fio.FIOBJ_T_STRING => .{ .String = try util.fio2strAllocOrNot(a, o, dupe_string) }, + fio.FIOBJ_T_STRING => .{ .String = try util.fio2strAlloc(a, o) }, fio.FIOBJ_T_ARRAY => { return .{ .Unsupported = null }; }, @@ -423,6 +429,7 @@ pub fn setContentTypeFromFilename(self: *const Self, filename: []const u8) !void const e = ext[1..]; const obj = fio.http_mimetype_find(@constCast(e.ptr), e.len); + // fio2str is OK since setHeader takes a copy if (util.fio2str(obj)) |mime_str| { try self.setHeader("content-type", mime_str); } @@ -438,6 +445,8 @@ pub fn setContentTypeFromFilename(self: *const Self, filename: []const u8) !void pub fn getHeader(self: *const Self, name: []const u8) ?[]const u8 { const hname = fio.fiobj_str_new(util.toCharPtr(name), name.len); defer fio.fiobj_free_wrapped(hname); + // fio2str is OK since headers are always strings -> slice is returned + // (and not a slice into some threadlocal "global") return util.fio2str(fio.fiobj_hash_get(self.h.*.headers, hname)); } @@ -495,6 +504,8 @@ pub fn getHeaderCommon(self: *const Self, which: HttpHeaderCommon) ?[]const u8 { .upgrade => fio.HTTP_HEADER_UPGRADE, }; const fiobj = zap.fio.fiobj_hash_get(self.h.*.headers, field); + // fio2str is OK since headers are always strings -> slice is returned + // (and not a slice into some threadlocal "global") return zap.util.fio2str(fiobj); } @@ -606,7 +617,7 @@ pub const AcceptItem = struct { const AcceptHeaderList = std.ArrayList(AcceptItem); /// Parses `Accept:` http header into `list`, ordered from highest q factor to lowest -pub fn parseAcceptHeaders(self: *const Self, allocator: std.mem.Allocator) !AcceptHeaderList { +pub fn parseAcceptHeaders(self: *const Self, allocator: Allocator) !AcceptHeaderList { const accept_str = self.getHeaderCommon(.accept) orelse return error.NoAcceptHeader; const comma_count = std.mem.count(u8, accept_str, ","); @@ -681,7 +692,7 @@ pub fn setCookie(self: *const Self, args: CookieArgs) HttpError!void { } /// Returns named cookie. Works like getParamStr(). -pub fn getCookieStr(self: *const Self, a: std.mem.Allocator, name: []const u8, always_alloc: bool) !?util.FreeOrNot { +pub fn getCookieStr(self: *const Self, a: Allocator, name: []const u8) !?[]const u8 { if (self.h.*.cookies == 0) return null; const key = fio.fiobj_str_new(name.ptr, name.len); defer fio.fiobj_free_wrapped(key); @@ -689,7 +700,9 @@ pub fn getCookieStr(self: *const Self, a: std.mem.Allocator, name: []const u8, a if (value == fio.FIOBJ_INVALID) { return null; } - return try util.fio2strAllocOrNot(a, value, always_alloc); + // we are not entirely sure if cookies fiobjs are always strings + // hence. fio2strAlloc + return try util.fio2strAlloc(a, value); } /// Returns the number of cookies after parsing. @@ -708,15 +721,72 @@ pub fn getParamCount(self: *const Self) isize { return fio.fiobj_obj2num(self.h.*.params); } +const CallbackContext_KV = struct { + allocator: Allocator, + params: *std.ArrayList(HttpParamKV), + last_error: ?anyerror = null, + + pub fn callback(fiobj_value: fio.FIOBJ, context_: ?*anyopaque) callconv(.C) c_int { + const ctx: *@This() = @as(*@This(), @ptrCast(@alignCast(context_))); + // this is thread-safe, guaranteed by fio + const fiobj_key: fio.FIOBJ = fio.fiobj_hash_key_in_loop(); + ctx.params.append(.{ + .key = util.fio2strAlloc(ctx.allocator, fiobj_key) catch |err| { + ctx.last_error = err; + return -1; + }, + .value = fiobj2HttpParam(ctx.allocator, fiobj_value) catch |err| { + ctx.last_error = err; + return -1; + }, + }) catch |err| { + // what to do? + // signal the caller that an error occured by returning -1 + // also, set the error + ctx.last_error = err; + return -1; + }; + return 0; + } +}; + +const CallbackContext_StrKV = struct { + allocator: Allocator, + params: *std.ArrayList(HttpParamStrKV), + last_error: ?anyerror = null, + + pub fn callback(fiobj_value: fio.FIOBJ, context_: ?*anyopaque) callconv(.C) c_int { + const ctx: *@This() = @as(*@This(), @ptrCast(@alignCast(context_))); + // this is thread-safe, guaranteed by fio + const fiobj_key: fio.FIOBJ = fio.fiobj_hash_key_in_loop(); + ctx.params.append(.{ + .key = util.fio2strAlloc(ctx.allocator, fiobj_key) catch |err| { + ctx.last_error = err; + return -1; + }, + .value = util.fio2strAlloc(ctx.allocator, fiobj_value) catch |err| { + ctx.last_error = err; + return -1; + }, + }) catch |err| { + // what to do? + // signal the caller that an error occured by returning -1 + // also, set the error + ctx.last_error = err; + return -1; + }; + return 0; + } +}; + /// Same as parametersToOwnedStrList() but for cookies -pub fn cookiesToOwnedStrList(self: *const Self, a: std.mem.Allocator, always_alloc: bool) anyerror!HttpParamStrKVList { +pub fn cookiesToOwnedStrList(self: *const Self, a: Allocator) anyerror!HttpParamStrKVList { var params = try std.ArrayList(HttpParamStrKV).initCapacity(a, @as(usize, @intCast(self.getCookiesCount()))); - var context: _parametersToOwnedStrSliceContext = .{ + var context: CallbackContext_StrKV = .{ .params = ¶ms, .allocator = a, - .always_alloc = always_alloc, }; - const howmany = fio.fiobj_each1(self.h.*.cookies, 0, _each_nextParamStr, &context); + const howmany = fio.fiobj_each1(self.h.*.cookies, 0, CallbackContext_StrKV.callback, &context); if (howmany != self.getCookiesCount()) { return error.HttpIterParams; } @@ -724,10 +794,10 @@ pub fn cookiesToOwnedStrList(self: *const Self, a: std.mem.Allocator, always_all } /// Same as parametersToOwnedList() but for cookies -pub fn cookiesToOwnedList(self: *const Self, a: std.mem.Allocator, dupe_strings: bool) !HttpParamKVList { +pub fn cookiesToOwnedList(self: *const Self, a: Allocator) !HttpParamKVList { var params = try std.ArrayList(HttpParamKV).initCapacity(a, @as(usize, @intCast(self.getCookiesCount()))); - var context: _parametersToOwnedSliceContext = .{ .params = ¶ms, .allocator = a, .dupe_strings = dupe_strings }; - const howmany = fio.fiobj_each1(self.h.*.cookies, 0, _each_nextParam, &context); + var context: CallbackContext_KV = .{ .params = ¶ms, .allocator = a }; + const howmany = fio.fiobj_each1(self.h.*.cookies, 0, CallbackContext_KV.callback, &context); if (howmany != self.getCookiesCount()) { return error.HttpIterParams; } @@ -747,50 +817,21 @@ pub fn cookiesToOwnedList(self: *const Self, a: std.mem.Allocator, dupe_strings: /// /// Requires parseBody() and/or parseQuery() have been called. /// Returned list needs to be deinited. -pub fn parametersToOwnedStrList(self: *const Self, a: std.mem.Allocator, always_alloc: bool) anyerror!HttpParamStrKVList { +pub fn parametersToOwnedStrList(self: *const Self, a: Allocator) anyerror!HttpParamStrKVList { var params = try std.ArrayList(HttpParamStrKV).initCapacity(a, @as(usize, @intCast(self.getParamCount()))); - var context: _parametersToOwnedStrSliceContext = .{ + + var context: CallbackContext_StrKV = .{ .params = ¶ms, .allocator = a, - .always_alloc = always_alloc, }; - const howmany = fio.fiobj_each1(self.h.*.params, 0, _each_nextParamStr, &context); + + const howmany = fio.fiobj_each1(self.h.*.params, 0, CallbackContext_StrKV.callback, &context); if (howmany != self.getParamCount()) { return error.HttpIterParams; } return .{ .items = try params.toOwnedSlice(), .allocator = a }; } -const _parametersToOwnedStrSliceContext = struct { - allocator: std.mem.Allocator, - params: *std.ArrayList(HttpParamStrKV), - last_error: ?anyerror = null, - always_alloc: bool, -}; - -fn _each_nextParamStr(fiobj_value: fio.FIOBJ, context: ?*anyopaque) callconv(.C) c_int { - const ctx: *_parametersToOwnedStrSliceContext = @as(*_parametersToOwnedStrSliceContext, @ptrCast(@alignCast(context))); - // this is thread-safe, guaranteed by fio - const fiobj_key: fio.FIOBJ = fio.fiobj_hash_key_in_loop(); - ctx.params.append(.{ - .key = util.fio2strAllocOrNot(ctx.allocator, fiobj_key, ctx.always_alloc) catch |err| { - ctx.last_error = err; - return -1; - }, - .value = util.fio2strAllocOrNot(ctx.allocator, fiobj_value, ctx.always_alloc) catch |err| { - ctx.last_error = err; - return -1; - }, - }) catch |err| { - // what to do? - // signal the caller that an error occured by returning -1 - // also, set the error - ctx.last_error = err; - return -1; - }; - return 0; -} - /// Returns the query / body parameters as key/value pairs /// Supported param types that will be converted: /// @@ -804,47 +845,19 @@ fn _each_nextParamStr(fiobj_value: fio.FIOBJ, context: ?*anyopaque) callconv(.C) /// /// Requires parseBody() and/or parseQuery() have been called. /// Returned slice needs to be freed. -pub fn parametersToOwnedList(self: *const Self, a: std.mem.Allocator, dupe_strings: bool) !HttpParamKVList { +pub fn parametersToOwnedList(self: *const Self, a: Allocator) !HttpParamKVList { var params = try std.ArrayList(HttpParamKV).initCapacity(a, @as(usize, @intCast(self.getParamCount()))); - var context: _parametersToOwnedSliceContext = .{ .params = ¶ms, .allocator = a, .dupe_strings = dupe_strings }; - const howmany = fio.fiobj_each1(self.h.*.params, 0, _each_nextParam, &context); + + var context: CallbackContext_KV = .{ .params = ¶ms, .allocator = a }; + + const howmany = fio.fiobj_each1(self.h.*.params, 0, CallbackContext_KV.callback, &context); if (howmany != self.getParamCount()) { return error.HttpIterParams; } return .{ .items = try params.toOwnedSlice(), .allocator = a }; } -const _parametersToOwnedSliceContext = struct { - params: *std.ArrayList(HttpParamKV), - last_error: ?anyerror = null, - allocator: std.mem.Allocator, - dupe_strings: bool, -}; - -fn _each_nextParam(fiobj_value: fio.FIOBJ, context: ?*anyopaque) callconv(.C) c_int { - const ctx: *_parametersToOwnedSliceContext = @as(*_parametersToOwnedSliceContext, @ptrCast(@alignCast(context))); - // this is thread-safe, guaranteed by fio - const fiobj_key: fio.FIOBJ = fio.fiobj_hash_key_in_loop(); - ctx.params.append(.{ - .key = util.fio2strAllocOrNot(ctx.allocator, fiobj_key, ctx.dupe_strings) catch |err| { - ctx.last_error = err; - return -1; - }, - .value = Fiobj2HttpParam(ctx.allocator, fiobj_value, ctx.dupe_strings) catch |err| { - ctx.last_error = err; - return -1; - }, - }) catch |err| { - // what to do? - // signal the caller that an error occured by returning -1 - // also, set the error - ctx.last_error = err; - return -1; - }; - return 0; -} - -/// get named parameter as string +/// get named parameter (parsed) as string /// Supported param types that will be converted: /// /// - Bool @@ -856,8 +869,8 @@ fn _each_nextParam(fiobj_value: fio.FIOBJ, context: ?*anyopaque) callconv(.C) c_ /// So, for JSON body payloads: parse the body instead. /// /// Requires parseBody() and/or parseQuery() have been called. -/// The returned string needs to be deinited with .deinit() -pub fn getParamStr(self: *const Self, a: std.mem.Allocator, name: []const u8, always_alloc: bool) !?util.FreeOrNot { +/// The returned string needs to be deallocated. +pub fn getParamStr(self: *const Self, a: Allocator, name: []const u8) !?[]const u8 { if (self.h.*.params == 0) return null; const key = fio.fiobj_str_new(name.ptr, name.len); defer fio.fiobj_free_wrapped(key); @@ -865,7 +878,7 @@ pub fn getParamStr(self: *const Self, a: std.mem.Allocator, name: []const u8, al if (value == fio.FIOBJ_INVALID) { return null; } - return try util.fio2strAllocOrNot(a, value, always_alloc); + return try util.fio2strAlloc(a, value); } /// similar to getParamStr, except it will return the part of the querystring diff --git a/src/tests/test_http_params.zig b/src/tests/test_http_params.zig index 167cbc0..df4ef18 100644 --- a/src/tests/test_http_params.zig +++ b/src/tests/test_http_params.zig @@ -26,7 +26,7 @@ test "http parameters" { var strParams: ?zap.Request.HttpParamStrKVList = null; var params: ?zap.Request.HttpParamKVList = null; - var paramOneStr: ?zap.util.FreeOrNot = null; + var paramOneStr: ?[]const u8 = null; var paramOneSlice: ?[]const u8 = null; var paramSlices: zap.Request.ParamSliceIterator = undefined; @@ -36,20 +36,18 @@ test "http parameters" { param_count = r.getParamCount(); // true -> make copies of temp strings - strParams = r.parametersToOwnedStrList(alloc, true) catch unreachable; + strParams = r.parametersToOwnedStrList(alloc) catch unreachable; // true -> make copies of temp strings - params = r.parametersToOwnedList(alloc, true) catch unreachable; + params = r.parametersToOwnedList(alloc) catch unreachable; - var maybe_str = r.getParamStr(alloc, "one", true) catch unreachable; - if (maybe_str) |*s| { - paramOneStr = s.*; - } + paramOneStr = r.getParamStr(alloc, "one") catch unreachable; + std.debug.print("\n\nparamOneStr = {s} = {*} \n\n", .{ paramOneStr.?, paramOneStr.?.ptr }); - paramOneSlice = blk: { - if (r.getParamSlice("one")) |val| break :blk alloc.dupe(u8, val) catch unreachable; - break :blk null; - }; + // we need to dupe it here because the request object r will get + // invalidated at the end of the function but we need to check + // its correctness later in the test + paramOneSlice = if (r.getParamSlice("one")) |slice| alloc.dupe(u8, slice) catch unreachable else null; paramSlices = r.getParamSlices(); } @@ -84,44 +82,45 @@ test "http parameters" { if (Handler.params) |*p| { p.deinit(); } - if (Handler.paramOneStr) |*p| { - // allocator.free(p); - p.deinit(); + + if (Handler.paramOneStr) |p| { + allocator.free(p); } if (Handler.paramOneSlice) |p| { - Handler.alloc.free(p); + allocator.free(p); } } - try std.testing.expectEqual(Handler.ran, true); - try std.testing.expectEqual(Handler.param_count, 5); + try std.testing.expectEqual(true, Handler.ran); + try std.testing.expectEqual(5, Handler.param_count); try std.testing.expect(Handler.paramOneStr != null); - try std.testing.expectEqualStrings(Handler.paramOneStr.?.str, "1"); + std.debug.print("\n\n=== Handler.paramOneStr = {s} = {*}\n\n", .{ Handler.paramOneStr.?, Handler.paramOneStr.?.ptr }); + try std.testing.expectEqualStrings("1", Handler.paramOneStr.?); try std.testing.expect(Handler.paramOneSlice != null); - try std.testing.expectEqualStrings(Handler.paramOneSlice.?, "1"); + try std.testing.expectEqualStrings("1", Handler.paramOneSlice.?); try std.testing.expect(Handler.strParams != null); for (Handler.strParams.?.items, 0..) |kv, i| { switch (i) { 0 => { - try std.testing.expectEqualStrings(kv.key.str, "one"); - try std.testing.expectEqualStrings(kv.value.str, "1"); + try std.testing.expectEqualStrings("one", kv.key); + try std.testing.expectEqualStrings("1", kv.value); }, 1 => { - try std.testing.expectEqualStrings(kv.key.str, "two"); - try std.testing.expectEqualStrings(kv.value.str, "2"); + try std.testing.expectEqualStrings("two", kv.key); + try std.testing.expectEqualStrings("2", kv.value); }, 2 => { - try std.testing.expectEqualStrings(kv.key.str, "string"); - try std.testing.expectEqualStrings(kv.value.str, "hello world"); + try std.testing.expectEqualStrings("string", kv.key); + try std.testing.expectEqualStrings("hello world", kv.value); }, 3 => { - try std.testing.expectEqualStrings(kv.key.str, "float"); - try std.testing.expectEqualStrings(kv.value.str, "6.28"); + try std.testing.expectEqualStrings("float", kv.key); + try std.testing.expectEqualStrings("6.28", kv.value); }, 4 => { - try std.testing.expectEqualStrings(kv.key.str, "bool"); - try std.testing.expectEqualStrings(kv.value.str, "true"); + try std.testing.expectEqualStrings("bool", kv.key); + try std.testing.expectEqualStrings("true", kv.value); }, else => return error.TooManyArgs, } @@ -131,24 +130,24 @@ test "http parameters" { while (Handler.paramSlices.next()) |param| { switch (pindex) { 0 => { - try std.testing.expectEqualStrings(param.name, "one"); - try std.testing.expectEqualStrings(param.value, "1"); + try std.testing.expectEqualStrings("one", param.name); + try std.testing.expectEqualStrings("1", param.value); }, 1 => { - try std.testing.expectEqualStrings(param.name, "two"); - try std.testing.expectEqualStrings(param.value, "2"); + try std.testing.expectEqualStrings("two", param.name); + try std.testing.expectEqualStrings("2", param.value); }, 2 => { - try std.testing.expectEqualStrings(param.name, "string"); - try std.testing.expectEqualStrings(param.value, "hello+world"); + try std.testing.expectEqualStrings("string", param.name); + try std.testing.expectEqualStrings("hello+world", param.value); }, 3 => { - try std.testing.expectEqualStrings(param.name, "float"); - try std.testing.expectEqualStrings(param.value, "6.28"); + try std.testing.expectEqualStrings("float", param.name); + try std.testing.expectEqualStrings("6.28", param.value); }, 4 => { - try std.testing.expectEqualStrings(param.name, "bool"); - try std.testing.expectEqualStrings(param.value, "true"); + try std.testing.expectEqualStrings("bool", param.name); + try std.testing.expectEqualStrings("true", param.value); }, else => return error.TooManyArgs, } @@ -158,42 +157,42 @@ test "http parameters" { for (Handler.params.?.items, 0..) |kv, i| { switch (i) { 0 => { - try std.testing.expectEqualStrings(kv.key.str, "one"); + try std.testing.expectEqualStrings("one", kv.key); try std.testing.expect(kv.value != null); switch (kv.value.?) { - .Int => |n| try std.testing.expectEqual(n, 1), + .Int => |n| try std.testing.expectEqual(1, n), else => return error.InvalidHttpParamType, } }, 1 => { - try std.testing.expectEqualStrings(kv.key.str, "two"); + try std.testing.expectEqualStrings("two", kv.key); try std.testing.expect(kv.value != null); switch (kv.value.?) { - .Int => |n| try std.testing.expectEqual(n, 2), + .Int => |n| try std.testing.expectEqual(2, n), else => return error.InvalidHttpParamType, } }, 2 => { - try std.testing.expectEqualStrings(kv.key.str, "string"); + try std.testing.expectEqualStrings("string", kv.key); try std.testing.expect(kv.value != null); switch (kv.value.?) { - .String => |s| try std.testing.expectEqualStrings(s.str, "hello world"), + .String => |s| try std.testing.expectEqualStrings("hello world", s), else => return error.InvalidHttpParamType, } }, 3 => { - try std.testing.expectEqualStrings(kv.key.str, "float"); + try std.testing.expectEqualStrings("float", kv.key); try std.testing.expect(kv.value != null); switch (kv.value.?) { - .Float => |f| try std.testing.expectEqual(f, 6.28), + .Float => |f| try std.testing.expectEqual(6.28, f), else => return error.InvalidHttpParamType, } }, 4 => { - try std.testing.expectEqualStrings(kv.key.str, "bool"); + try std.testing.expectEqualStrings("bool", kv.key); try std.testing.expect(kv.value != null); switch (kv.value.?) { - .Bool => |b| try std.testing.expectEqual(b, true), + .Bool => |b| try std.testing.expectEqual(true, b), else => return error.InvalidHttpParamType, } }, diff --git a/src/util.zig b/src/util.zig index 663155c..1719fe9 100644 --- a/src/util.zig +++ b/src/util.zig @@ -6,6 +6,9 @@ const zap = @import("zap.zig"); /// note: since this is called from within request functions, we don't make /// copies. Also, we return temp memory from fio. -> don't hold on to it outside /// of a request function. FIO temp memory strings do not need to be freed. +/// +/// IMPORTANT!!! The "temp" memory can refer to a shared buffer that subsequent +/// calls to this function will **overwrite**!!! pub fn fio2str(o: fio.FIOBJ) ?[]const u8 { if (o == 0) return null; const x: fio.fio_str_info_s = fio.fiobj_obj2cstr(o); @@ -14,39 +17,21 @@ pub fn fio2str(o: fio.FIOBJ) ?[]const u8 { return x.data[0..x.len]; } -/// A "string" type used internally that carries a flag whether its buffer needs -/// to be freed or not - and honors it in `deinit()`. That way, it's always -/// safe to call deinit(). -/// For instance, slices taken directly from the zap.Request need not be freed. -/// But the ad-hoc created string representation of a float parameter must be -/// freed after use. -pub const FreeOrNot = struct { - str: []const u8, - freeme: bool, - allocator: ?std.mem.Allocator = null, - - pub fn deinit(self: *const @This()) void { - if (self.freeme) { - self.allocator.?.free(self.str); - } - } -}; - /// Used internally: convert a FIO object into its string representation. -/// Depending on the type of the object, a buffer will be created. Hence a -/// FreeOrNot type is used as the return type. -pub fn fio2strAllocOrNot(a: std.mem.Allocator, o: fio.FIOBJ, always_alloc: bool) !FreeOrNot { - if (o == 0) return .{ .str = "null", .freeme = false }; - if (o == fio.FIOBJ_INVALID) return .{ .str = "invalid", .freeme = false }; +/// This always allocates, so choose your allocator wisely. +/// Let's never use that +pub fn fio2strAlloc(a: std.mem.Allocator, o: fio.FIOBJ) ![]const u8 { + if (o == 0) return try a.dupe(u8, "null"); + if (o == fio.FIOBJ_INVALID) return try a.dupe(u8, "invalid"); return switch (fio.fiobj_type(o)) { - fio.FIOBJ_T_TRUE => .{ .str = "true", .freeme = false }, - fio.FIOBJ_T_FALSE => .{ .str = "false", .freeme = false }, + fio.FIOBJ_T_TRUE => try a.dupe(u8, "true"), + fio.FIOBJ_T_FALSE => try a.dupe(u8, "false"), // according to fio2str above, the orelse should never happen - fio.FIOBJ_T_NUMBER => .{ .str = try a.dupe(u8, fio2str(o) orelse "null"), .freeme = true, .allocator = a }, - fio.FIOBJ_T_FLOAT => .{ .str = try a.dupe(u8, fio2str(o) orelse "null"), .freeme = true, .allocator = a }, - // the string comes out of the request, so it is safe to not make a copy - fio.FIOBJ_T_STRING => .{ .str = if (always_alloc) try a.dupe(u8, fio2str(o) orelse "") else fio2str(o) orelse "", .freeme = if (always_alloc) true else false, .allocator = a }, - else => .{ .str = "unknown_type", .freeme = false }, + fio.FIOBJ_T_NUMBER => try a.dupe(u8, fio2str(o) orelse "null"), + fio.FIOBJ_T_FLOAT => try a.dupe(u8, fio2str(o) orelse "null"), + // if the string comes out of the request, it is safe to not make a copy + fio.FIOBJ_T_STRING => try a.dupe(u8, fio2str(o) orelse ""), + else => try a.dupe(u8, "unknown_type"), }; }