From bf1b5e55060f7dc025bde71e09cf6770cdf21862 Mon Sep 17 00:00:00 2001 From: Joe Schafer Date: Sun, 31 May 2026 00:34:45 -0700 Subject: [PATCH] browser/URL: ignore query and fragment slashes in URL resolve Resolve relative request URLs against only the path component of the base URL. Previously, URL.resolve searched for the last slash in the whole base URL after the authority, so slashes inside a query string or hash route could be mistaken for path directory separators. The reported failure was a hash-routed page loaded at http://127.0.0.1:8123/#/login. When that page ran fetch("api/users/login", { method: "POST" }), browser-compatible URL resolution should have requested http://127.0.0.1:8123/api/users/login. Instead, Lightpanda 0.3.1 reported the response URL as http://127.0.0.1:8123/#/api/users/login, and the HTTP server received POST /. That meant the server returned the single-page app shell instead of the JSON login response. The failure broke a RealWorld/Conduit-style SPA benchmark after login because the app used relative API URLs such as api/users/login. Changing the benchmark fixture to root-absolute API URLs, such as /api/users/login, worked around the benchmark failure, but left Lightpanda incompatible with normal browser behavior for relative request URLs on hash-routed pages. The same issue was not limited to fragments. A base URL such as https://example/app/page?next=/foo/bar also contains slashes after the path component. Those slashes must not affect how path-relative inputs such as api/users/login or ../api/users/login are merged with the base path. This change bounds the directory calculation at the first ? or # in the base URL and searches for the last path slash only inside that path component. Root-absolute inputs still keep only the scheme and authority, query-only inputs still replace the query on the current path, and fragment-only inputs still replace the fragment on the current path. This matches the behavior of the WHATWG URL algorithm, Chromium's relative URL canonicalization, Node's WHATWG URL implementation, Go's net/url ResolveReference, and Python's urllib.parse.urljoin for the cases covered here. All of those implementations split the base URL into components before merging a path-relative reference, so slashes in the query or fragment do not change the base directory. The URL resolver tests now cover the original hash-route repro, slashes inside query strings and fragments, host-only bases with query or fragment components, dot-segment paths, query-only references, and fragment-only references. The fetch Web API tests also move a page to /#/login and POST fetch("xhr"), expecting the request URL to resolve to http://127.0.0.1:9582/xhr rather than the hash route. Verified with: mise x zig@0.15.2 -- zig fmt --check ./*.zig ./**/*.zig mise x zig@0.15.2 rust@stable -- make ZIG=zig test --- src/browser/URL.zig | 67 ++++++++++++++++++++- src/browser/tests/net/fetch_hash_route.html | 28 +++++++++ src/browser/webapi/net/Fetch.zig | 1 + 3 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 src/browser/tests/net/fetch_hash_route.html diff --git a/src/browser/URL.zig b/src/browser/URL.zig index 620e63a2..891ae860 100644 --- a/src/browser/URL.zig +++ b/src/browser/URL.zig @@ -121,7 +121,8 @@ pub fn resolve(allocator: Allocator, base: [:0]const u8, source_path: anytype, o const scheme_end = std.mem.indexOf(u8, base, "://"); const authority_start = if (scheme_end) |end| end + 3 else 0; - const path_start = std.mem.indexOfScalarPos(u8, base, authority_start, '/') orelse base.len; + const path_start = std.mem.indexOfAnyPos(u8, base, authority_start, "/?#") orelse base.len; + const path_end = std.mem.indexOfAnyPos(u8, base, path_start, "?#") orelse base.len; if (path[0] == '/') { const result = try std.mem.joinZ(allocator, "", &.{ base[0..path_start], path }); @@ -129,8 +130,8 @@ pub fn resolve(allocator: Allocator, base: [:0]const u8, source_path: anytype, o } var normalized_base: []const u8 = base[0..path_start]; - if (path_start < base.len) { - if (std.mem.lastIndexOfScalar(u8, base[path_start + 1 ..], '/')) |pos| { + if (path_start < path_end) { + if (std.mem.lastIndexOfScalar(u8, base[path_start + 1 .. path_end], '/')) |pos| { normalized_base = base[0 .. path_start + 1 + pos]; } } @@ -973,6 +974,66 @@ test "URL: resolve" { .path = "something.js", .expected = "https://example/xyz/abc/something.js", }, + .{ + .base = "http://127.0.0.1:8123/#/login", + .path = "api/users/login", + .expected = "http://127.0.0.1:8123/api/users/login", + }, + .{ + .base = "https://example/app/page?next=/foo/bar", + .path = "api/users/login", + .expected = "https://example/app/api/users/login", + }, + .{ + .base = "https://example/app/page#/foo/bar", + .path = "api/users/login", + .expected = "https://example/app/api/users/login", + }, + .{ + .base = "https://example?next=/foo/bar", + .path = "api/users/login", + .expected = "https://example/api/users/login", + }, + .{ + .base = "https://example#/foo/bar", + .path = "api/users/login", + .expected = "https://example/api/users/login", + }, + .{ + .base = "https://example?next=/foo/bar", + .path = "/api/users/login", + .expected = "https://example/api/users/login", + }, + .{ + .base = "https://example/app/page?next=/foo/bar", + .path = "../api/users/login", + .expected = "https://example/api/users/login", + }, + .{ + .base = "https://example/app/page#/foo/bar", + .path = "../api/users/login", + .expected = "https://example/api/users/login", + }, + .{ + .base = "https://example/app/dir/?next=/foo/bar", + .path = "../api/users/login", + .expected = "https://example/app/api/users/login", + }, + .{ + .base = "https://example/app/dir/#/foo/bar", + .path = "../api/users/login", + .expected = "https://example/app/api/users/login", + }, + .{ + .base = "https://example/app/page?next=/foo/bar", + .path = "?q=/api/users/login", + .expected = "https://example/app/page?q=/api/users/login", + }, + .{ + .base = "https://example/app/page#/foo/bar", + .path = "#/api/users/login", + .expected = "https://example/app/page#/api/users/login", + }, .{ .base = "https://example/xyz/abc/123", .path = "/something.js", diff --git a/src/browser/tests/net/fetch_hash_route.html b/src/browser/tests/net/fetch_hash_route.html new file mode 100644 index 00000000..8a794550 --- /dev/null +++ b/src/browser/tests/net/fetch_hash_route.html @@ -0,0 +1,28 @@ + + + + diff --git a/src/browser/webapi/net/Fetch.zig b/src/browser/webapi/net/Fetch.zig index 1f69ef0e..74b3351c 100644 --- a/src/browser/webapi/net/Fetch.zig +++ b/src/browser/webapi/net/Fetch.zig @@ -270,4 +270,5 @@ fn httpShutdownCallback(ctx: *anyopaque) void { const testing = @import("../../../testing.zig"); test "WebApi: fetch" { try testing.htmlRunner("net/fetch.html", .{}); + try testing.htmlRunner("net/fetch_hash_route.html", .{}); }