Minor improvements to request params and headers

Fully disambiguate request params from body and query string and clean
up convoluted code, reduce allocs. Implement
`jetzig.http.Request.queryParams` which always returns only the parsed
query string, never the request body.

Deprecate `headers.getFirstValue` in favour of `headers.get`, implement
`headers.getAll`.
This commit is contained in:
Bob Farrell 2024-04-13 19:24:40 +01:00
parent 4de9f4d248
commit 0b968c3b44
5 changed files with 127 additions and 83 deletions

View File

@ -7,8 +7,8 @@
.hash = "12207d49df326e0c180a90fa65d9993898e0a0ffd8e79616b4b81f81769261858856",
},
.zmpl = .{
.url = "https://github.com/jetzig-framework/zmpl/archive/09c04d91d18ec7fd82ac444b1e7e609380274098.tar.gz",
.hash = "12205356d83fa54a6e2c81cd5dc461fa0a1d042af379c7308aea0bf3fdb9d80a709c",
.url = "https://github.com/jetzig-framework/zmpl/archive/6f9496a02dcdc2a309b2c1bbaec2affc55f063ae.tar.gz",
.hash = "122014e63f5421060573216929af3a4c4c373934213ed727c77af927f787cb2f4113",
},
.args = .{
.url = "https://github.com/MasterQ32/zig-args/archive/01d72b9a0128c474aeeb9019edd48605fa6d95f7.tar.gz",

View File

@ -253,14 +253,11 @@ fn generateRoutesForView(self: *Self, views_dir: std.fs.Dir, path: []const u8) !
if (static_params) |capture| {
if (capture.get(static_route.name)) |params| {
for (params.array.array.items) |item| { // XXX: Use public interface for Data.Array here ?
var json_buf = std.ArrayList(u8).init(self.allocator);
defer json_buf.deinit();
const json_writer = json_buf.writer();
try item.toJson(json_writer, false, 0);
const json = try item.toJson();
var encoded_buf = std.ArrayList(u8).init(self.allocator);
defer encoded_buf.deinit();
const writer = encoded_buf.writer();
try std.json.encodeJsonString(json_buf.items, .{}, writer);
try std.json.encodeJsonString(json, .{}, writer);
try static_route.params.append(try self.allocator.dupe(u8, encoded_buf.items));
}
}

View File

@ -4,36 +4,55 @@ const jetzig = @import("../../jetzig.zig");
allocator: std.mem.Allocator,
headers: HeadersArray,
const Self = @This();
const Headers = @This();
pub const max_headers = 25;
const HeadersArray = std.ArrayListUnmanaged(std.http.Header);
pub fn init(allocator: std.mem.Allocator) Self {
pub fn init(allocator: std.mem.Allocator) Headers {
return .{
.allocator = allocator,
.headers = HeadersArray.initCapacity(allocator, max_headers) catch @panic("OOM"),
};
}
pub fn deinit(self: *Self) void {
pub fn deinit(self: *Headers) void {
self.headers.deinit(self.allocator);
}
// Gets the first value for a given header identified by `name`. Names are case insensitive.
pub fn getFirstValue(self: *Self, name: []const u8) ?[]const u8 {
/// Gets the first value for a given header identified by `name`. Names are case insensitive.
pub fn get(self: Headers, name: []const u8) ?[]const u8 {
for (self.headers.items) |header| {
if (jetzig.util.equalStringsCaseInsensitive(name, header.name)) return header.value;
}
return null;
}
/// Gets the first value for a given header identified by `name`. Names are case insensitive.
pub fn getAll(self: Headers, name: []const u8) []const []const u8 {
var headers = std.ArrayList([]const u8).init(self.allocator);
for (self.headers.items) |header| {
if (jetzig.util.equalStringsCaseInsensitive(name, header.name)) {
headers.append(header.value) catch @panic("OOM");
}
}
return headers.toOwnedSlice() catch @panic("OOM");
}
// Deprecated
pub fn getFirstValue(self: *Headers, name: []const u8) ?[]const u8 {
return self.get(name);
}
/// Appends `name` and `value` to headers.
pub fn append(self: *Self, name: []const u8, value: []const u8) !void {
pub fn append(self: *Headers, name: []const u8, value: []const u8) !void {
if (self.headers.items.len >= 25) return error.JetzigTooManyHeaders;
self.headers.appendAssumeCapacity(.{ .name = name, .value = value });
}
/// Removes **all** header entries matching `name`. Names are case-insensitive.
pub fn remove(self: *Self, name: []const u8) void {
pub fn remove(self: *Headers, name: []const u8) void {
if (self.headers.items.len == 0) return;
var index: usize = self.headers.items.len;
@ -47,13 +66,13 @@ pub fn remove(self: *Self, name: []const u8) void {
}
/// Returns an iterator which implements `next()` returning each name/value of the stored headers.
pub fn iterator(self: *Self) Iterator {
pub fn iterator(self: Headers) Iterator {
return Iterator{ .headers = self.headers };
}
/// Returns an array of `std.http.Header`, can be used to set response headers directly.
/// Caller owns memory.
pub fn stdHeaders(self: *Self) !std.ArrayListUnmanaged(std.http.Header) {
pub fn stdHeaders(self: *Headers) !std.ArrayListUnmanaged(std.http.Header) {
var array = try std.ArrayListUnmanaged(std.http.Header).initCapacity(self.allocator, max_headers);
var it = self.iterator();
@ -87,24 +106,45 @@ const Iterator = struct {
test "append" {
const allocator = std.testing.allocator;
var headers = Self.init(allocator);
var headers = Headers.init(allocator);
defer headers.deinit();
try headers.append("foo", "bar");
try std.testing.expectEqualStrings(headers.getFirstValue("foo").?, "bar");
}
test "getFirstValue with multiple headers (bugfix regression test)" {
test "get with multiple headers (bugfix regression test)" {
const allocator = std.testing.allocator;
var headers = Self.init(allocator);
var headers = Headers.init(allocator);
defer headers.deinit();
try headers.append("foo", "bar");
try headers.append("bar", "baz");
try std.testing.expectEqualStrings(headers.getFirstValue("bar").?, "baz");
try std.testing.expectEqualStrings(headers.get("bar").?, "baz");
}
test "getAll" {
const allocator = std.testing.allocator;
var headers = Headers.init(allocator);
defer headers.deinit();
try headers.append("foo", "bar");
try headers.append("foo", "baz");
try headers.append("bar", "qux");
const all = headers.getAll("foo");
defer allocator.free(all);
try std.testing.expectEqualSlices([]const u8, all, &[_][]const u8{ "bar", "baz" });
}
test "append too many headers" {
const allocator = std.testing.allocator;
var headers = Headers.init(allocator);
defer headers.deinit();
for (0..25) |_| try headers.append("foo", "bar");
try std.testing.expectError(error.JetzigTooManyHeaders, headers.append("foo", "bar"));
}
test "case-insensitive matching" {
const allocator = std.testing.allocator;
var headers = Self.init(allocator);
var headers = Headers.init(allocator);
defer headers.deinit();
try headers.append("Content-Type", "bar");
try std.testing.expectEqualStrings(headers.getFirstValue("content-type").?, "bar");
@ -112,7 +152,7 @@ test "case-insensitive matching" {
test "iterator" {
const allocator = std.testing.allocator;
var headers = Self.init(allocator);
var headers = Headers.init(allocator);
defer headers.deinit();
try headers.append("foo", "bar");
@ -129,7 +169,7 @@ test "iterator" {
test "remove" {
const allocator = std.testing.allocator;
var headers = Self.init(allocator);
var headers = Headers.init(allocator);
defer headers.deinit();
try headers.append("foo", "baz");
try headers.append("foo", "qux");
@ -141,7 +181,7 @@ test "remove" {
test "stdHeaders" {
const allocator = std.testing.allocator;
var headers = Self.init(allocator);
var headers = Headers.init(allocator);
defer headers.deinit();
try headers.append("foo", "bar");

View File

@ -33,18 +33,22 @@ pub fn parse(self: *Query) !void {
while (pairs_it.next()) |pair| {
var key_value_it = std.mem.splitScalar(u8, pair, '=');
var count: u2 = 0;
var key: []const u8 = undefined;
var maybe_key: ?[]const u8 = null;
var value: ?[]const u8 = null;
while (key_value_it.next()) |key_or_value| {
switch (count) {
0 => key = key_or_value,
0 => maybe_key = key_or_value,
1 => value = key_or_value,
else => return error.JetzigQueryParseError,
}
count += 1;
}
try self.query_items.append(.{ .key = key, .value = value });
if (maybe_key) |key| {
if (key.len > 0) {
try self.query_items.append(.{ .key = key, .value = value });
}
}
}
var params = try self.data.object();
@ -120,8 +124,8 @@ test "simple query string" {
defer query.deinit();
try query.parse();
try std.testing.expectEqualStrings((try data.get("foo")).string.value, "bar");
try std.testing.expectEqualStrings((try data.get("baz")).string.value, "qux");
try std.testing.expectEqualStrings((data.get("foo")).?.string.value, "bar");
try std.testing.expectEqualStrings((data.get("baz")).?.string.value, "qux");
}
test "query string with array values" {
@ -134,7 +138,7 @@ test "query string with array values" {
try query.parse();
const value = try data.get("foo");
const value = data.get("foo").?;
switch (value.*) {
.array => |array| {
try std.testing.expectEqualStrings(array.get(0).?.string.value, "bar");
@ -154,7 +158,7 @@ test "query string with mapping values" {
try query.parse();
const value = try data.get("foo");
const value = data.get("foo").?;
switch (value.*) {
.object => |object| {
try std.testing.expectEqualStrings(object.get("bar").?.string.value, "baz");
@ -174,13 +178,13 @@ test "query string with param without value" {
try query.parse();
const foo = try data.get("foo");
const foo = data.get("foo").?;
try switch (foo.*) {
.Null => {},
else => std.testing.expect(false),
};
const bar = try data.get("bar");
const bar = data.get("bar").?;
try switch (bar.*) {
.Null => {},
else => std.testing.expect(false),

View File

@ -2,7 +2,7 @@ const std = @import("std");
const jetzig = @import("../../jetzig.zig");
const Self = @This();
const Request = @This();
const default_content_type = "text/html";
pub const Method = enum { DELETE, GET, PATCH, POST, HEAD, PUT, CONNECT, OPTIONS, TRACE };
@ -18,8 +18,8 @@ std_http_request: std.http.Server.Request,
response: *jetzig.http.Response,
status_code: jetzig.http.status_codes.StatusCode = undefined,
response_data: *jetzig.data.Data,
query_data: *jetzig.data.Data,
query: *jetzig.http.Query,
query_params: ?*jetzig.http.Query = null,
query_body: ?*jetzig.http.Query = null,
cookies: *jetzig.http.Cookies = undefined,
session: *jetzig.http.Session = undefined,
body: []const u8 = undefined,
@ -38,7 +38,7 @@ pub fn init(
start_time: i128,
std_http_request: std.http.Server.Request,
response: *jetzig.http.Response,
) !Self {
) !Request {
const method = switch (std_http_request.head.method) {
.DELETE => Method.DELETE,
.GET => Method.GET,
@ -55,11 +55,6 @@ pub fn init(
const response_data = try allocator.create(jetzig.data.Data);
response_data.* = jetzig.data.Data.init(allocator);
const query_data = try allocator.create(jetzig.data.Data);
query_data.* = jetzig.data.Data.init(allocator);
const query = try allocator.create(jetzig.http.Query);
return .{
.allocator = allocator,
.path = jetzig.http.Path.init(std_http_request.head.target),
@ -68,14 +63,12 @@ pub fn init(
.server = server,
.response = response,
.response_data = response_data,
.query_data = query_data,
.query = query,
.std_http_request = std_http_request,
.start_time = start_time,
};
}
pub fn deinit(self: *Self) void {
pub fn deinit(self: *Request) void {
// self.session.deinit();
self.allocator.destroy(self.cookies);
self.allocator.destroy(self.session);
@ -83,7 +76,7 @@ pub fn deinit(self: *Self) void {
}
/// Process request, read body if present, parse headers (TODO)
pub fn process(self: *Self) !void {
pub fn process(self: *Request) !void {
var headers_it = self.std_http_request.iterateHeaders();
var cookie: ?[]const u8 = null;
@ -117,7 +110,7 @@ pub fn process(self: *Self) !void {
}
/// Set response headers, write response payload, and finalize the response.
pub fn respond(self: *Self) !void {
pub fn respond(self: *Request) !void {
if (!self.processed) unreachable;
var cookie_it = self.cookies.headerIterator();
@ -143,7 +136,7 @@ pub fn respond(self: *Self) !void {
/// Render a response. This function can only be called once per request (repeat calls will
/// trigger an error).
pub fn render(self: *Self, status_code: jetzig.http.status_codes.StatusCode) jetzig.views.View {
pub fn render(self: *Request, status_code: jetzig.http.status_codes.StatusCode) jetzig.views.View {
if (self.rendered) self.rendered_multiple = true;
self.rendered = true;
@ -160,7 +153,7 @@ pub fn render(self: *Self, status_code: jetzig.http.status_codes.StatusCode) jet
/// ```
/// The second argument must be `moved_permanently` or `found`.
pub fn redirect(
self: *Self,
self: *Request,
location: []const u8,
redirect_status: enum { moved_permanently, found },
) jetzig.views.View {
@ -188,7 +181,7 @@ pub fn redirect(
/// * `Accept` header (`application/json` or `text/html`)
/// * `Content-Type` header (`application/json` or `text/html`)
/// * Fall back to default: HTML
pub fn requestFormat(self: *Self) jetzig.http.Request.Format {
pub fn requestFormat(self: *Request) jetzig.http.Request.Format {
return self.extensionFormat() orelse
self.acceptHeaderFormat() orelse
self.contentTypeHeaderFormat() orelse
@ -197,7 +190,7 @@ pub fn requestFormat(self: *Self) jetzig.http.Request.Format {
/// Set the layout for the current request/response. Use this to override a `pub const layout`
/// declaration in a view, either in middleware or in a view function itself.
pub fn setLayout(self: *Self, layout: ?[]const u8) void {
pub fn setLayout(self: *Request, layout: ?[]const u8) void {
if (layout) |layout_name| {
self.layout = layout_name;
self.layout_disabled = false;
@ -208,7 +201,7 @@ pub fn setLayout(self: *Self, layout: ?[]const u8) void {
/// Derive a layout name from the current request if defined, otherwise from the route (if
/// defined).
pub fn getLayout(self: *Self, route: *jetzig.views.Route) ?[]const u8 {
pub fn getLayout(self: *Request, route: *jetzig.views.Route) ?[]const u8 {
if (self.layout_disabled) return null;
if (self.layout) |capture| return capture;
if (route.layout) |capture| return capture;
@ -218,7 +211,7 @@ pub fn getLayout(self: *Self, route: *jetzig.views.Route) ?[]const u8 {
/// Shortcut for `request.headers.getFirstValue`. Returns the first matching value for a given
/// header name or `null` if not found. Header names are case-insensitive.
pub fn getHeader(self: *Self, key: []const u8) ?[]const u8 {
pub fn getHeader(self: *Request, key: []const u8) ?[]const u8 {
return self.headers.getFirstValue(key);
}
@ -227,7 +220,7 @@ pub fn getHeader(self: *Self, key: []const u8) ?[]const u8 {
/// Note that query parameters are supported for JSON requests if no request body is present,
/// otherwise the parsed JSON request body will take precedence and query parameters will be
/// ignored.
pub fn params(self: *Self) !*jetzig.data.Value {
pub fn params(self: *Request) !*jetzig.data.Value {
if (!self.processed) unreachable;
switch (self.requestFormat()) {
@ -244,35 +237,45 @@ pub fn params(self: *Self) !*jetzig.data.Value {
};
return data.value.?;
},
.HTML, .UNKNOWN => return self.queryParams(),
.HTML, .UNKNOWN => return self.parseQuery(),
}
}
fn queryParams(self: *Self) !*jetzig.data.Value {
if (!try self.parseQueryString()) {
self.query.data = try self.allocator.create(jetzig.data.Data);
self.query.data.* = jetzig.data.Data.init(self.allocator);
_ = try self.query.data.object();
}
return self.query.data.value.?;
/// Return a `*Value` representing request parameters. This function **always** returns the
/// parsed query string and never the request body.
pub fn queryParams(self: *Request) !*jetzig.data.Value {
if (self.query_params) |parsed| return parsed.data.value.?;
const data = try self.allocator.create(jetzig.data.Data);
data.* = jetzig.data.Data.init(self.allocator);
self.query_params = try self.allocator.create(jetzig.http.Query);
self.query_params.?.* = jetzig.http.Query.init(
self.allocator,
self.path.query orelse "",
data,
);
try self.query_params.?.parse();
return self.query_params.?.data.value.?;
}
fn parseQueryString(self: *Self) !bool {
const data: ?[]const u8 = if (self.body.len > 0) self.body else self.path.query;
if (data) |query| {
self.query.* = jetzig.http.Query.init(
self.allocator,
query,
self.query_data,
);
try self.query.parse();
return true;
}
// Parses request body as params if present, otherwise delegates to `queryParams`.
fn parseQuery(self: *Request) !*jetzig.data.Value {
if (self.body.len == 0) return try self.queryParams();
if (self.query_body) |parsed| return parsed.data.value.?;
return false;
const data = try self.allocator.create(jetzig.data.Data);
data.* = jetzig.data.Data.init(self.allocator);
self.query_body = try self.allocator.create(jetzig.http.Query);
self.query_body.?.* = jetzig.http.Query.init(
self.allocator,
self.body,
data,
);
try self.query_body.?.parse();
return self.query_body.?.data.value.?;
}
fn extensionFormat(self: *Self) ?jetzig.http.Request.Format {
fn extensionFormat(self: *Request) ?jetzig.http.Request.Format {
const extension = self.path.extension orelse return null;
if (std.mem.eql(u8, extension, ".html")) {
@ -284,7 +287,7 @@ fn extensionFormat(self: *Self) ?jetzig.http.Request.Format {
}
}
pub fn acceptHeaderFormat(self: *Self) ?jetzig.http.Request.Format {
pub fn acceptHeaderFormat(self: *Request) ?jetzig.http.Request.Format {
const acceptHeader = self.getHeader("Accept");
if (acceptHeader) |item| {
@ -295,7 +298,7 @@ pub fn acceptHeaderFormat(self: *Self) ?jetzig.http.Request.Format {
return null;
}
pub fn contentTypeHeaderFormat(self: *Self) ?jetzig.http.Request.Format {
pub fn contentTypeHeaderFormat(self: *Request) ?jetzig.http.Request.Format {
const acceptHeader = self.getHeader("content-type");
if (acceptHeader) |item| {
@ -306,7 +309,7 @@ pub fn contentTypeHeaderFormat(self: *Self) ?jetzig.http.Request.Format {
return null;
}
pub fn hash(self: *Self) ![]const u8 {
pub fn hash(self: *Request) ![]const u8 {
return try std.fmt.allocPrint(
self.allocator,
"{s}-{s}-{s}",
@ -314,7 +317,7 @@ pub fn hash(self: *Self) ![]const u8 {
);
}
pub fn fmtMethod(self: *const Self, colorized: bool) []const u8 {
pub fn fmtMethod(self: *const Request, colorized: bool) []const u8 {
if (!colorized) return @tagName(self.method);
return switch (self.method) {
@ -331,7 +334,7 @@ pub fn fmtMethod(self: *const Self, colorized: bool) []const u8 {
/// Format a status code appropriately for the current request format.
/// e.g. `.HTML` => `404 Not Found`
/// `.JSON` => `{ "message": "Not Found", "status": "404" }`
pub fn formatStatus(self: *Self, status_code: jetzig.http.StatusCode) ![]const u8 {
pub fn formatStatus(self: *Request, status_code: jetzig.http.StatusCode) ![]const u8 {
const status = jetzig.http.status_codes.get(status_code);
return switch (self.requestFormat()) {
@ -344,7 +347,7 @@ pub fn formatStatus(self: *Self, status_code: jetzig.http.StatusCode) ![]const u
}
pub fn setResponse(
self: *Self,
self: *Request,
rendered_view: jetzig.http.Server.RenderedView,
options: struct { content_type: ?[]const u8 = null },
) void {
@ -357,7 +360,7 @@ pub fn setResponse(
}
// Determine if a given route matches the current request.
pub fn match(self: *Self, route: jetzig.views.Route) !bool {
pub fn match(self: *Request, route: jetzig.views.Route) !bool {
return switch (self.method) {
.GET => switch (route.action) {
.index => self.isMatch(.exact, route),
@ -384,7 +387,7 @@ pub fn match(self: *Self, route: jetzig.views.Route) !bool {
};
}
fn isMatch(self: *Self, match_type: enum { exact, resource_id }, route: jetzig.views.Route) bool {
fn isMatch(self: *Request, match_type: enum { exact, resource_id }, route: jetzig.views.Route) bool {
const path = switch (match_type) {
.exact => self.path.base_path,
.resource_id => self.path.directory,