* Avoid a utf8 conversion in isDetached

* Fixes #4001

* hit the long url codepath

---------

Co-authored-by: Jarred Sumner <709451+Jarred-Sumner@users.noreply.github.com>
This commit is contained in:
Jarred Sumner
2023-08-06 22:49:10 -07:00
committed by GitHub
parent 0665733b03
commit 00a907c7de
3 changed files with 108 additions and 9 deletions

View File

@@ -3943,7 +3943,8 @@ pub const AnyBlob = union(enum) {
pub fn isDetached(this: *const AnyBlob) bool {
return switch (this.*) {
.Blob => |blob| blob.isDetached(),
else => this.slice().len == 0,
.InternalBlob => this.InternalBlob.bytes.items.len == 0,
.WTFStringImpl => this.WTFStringImpl.length() == 0,
};
}

View File

@@ -322,7 +322,7 @@ pub const Request = struct {
pub fn sizeOfURL(this: *const Request) usize {
if (this.url.length() > 0)
return this.url.length();
return this.url.byteSlice().len;
if (this.uws_request) |req| {
const req_url = req.url();
@@ -369,8 +369,8 @@ pub const Request = struct {
std.debug.assert(this.sizeOfURL() == url_bytelength);
}
if (url_bytelength < 64) {
var buffer: [64]u8 = undefined;
if (url_bytelength < 128) {
var buffer: [128]u8 = undefined;
const url = std.fmt.bufPrint(&buffer, "{s}{any}{s}", .{
this.getProtocol(),
fmt,
@@ -381,21 +381,30 @@ pub const Request = struct {
std.debug.assert(this.sizeOfURL() == url.len);
}
if (bun.String.tryCreateAtom(url)) |atomized| {
this.url = atomized;
return;
var href = bun.JSC.URL.hrefFromString(bun.String.fromBytes(url));
if (!href.isEmpty()) {
if (href.byteSlice().ptr == url.ptr) {
this.url = bun.String.createLatin1(url[0..href.length()]);
href.deref();
} else {
this.url = href;
}
} else {
// TODO: what is the right thing to do for invalid URLS?
this.url = bun.String.create(url);
}
return;
}
if (strings.isAllASCII(host) and strings.isAllASCII(req_url)) {
this.url = bun.String.createUninitializedLatin1(url_bytelength);
var bytes = @constCast(this.url.byteSlice());
const url = std.fmt.bufPrint(bytes, "{s}{any}{s}", .{
_ = std.fmt.bufPrint(bytes, "{s}{any}{s}", .{
this.getProtocol(),
fmt,
req_url,
}) catch @panic("Unexpected error while printing URL");
_ = url;
} else {
// slow path
var temp_url = std.fmt.allocPrint(bun.default_allocator, "{s}{any}{s}", .{
@@ -406,6 +415,13 @@ pub const Request = struct {
defer bun.default_allocator.free(temp_url);
this.url = bun.String.create(temp_url);
}
const href = bun.JSC.URL.hrefFromString(this.url);
// TODO: what is the right thing to do for invalid URLS?
if (!href.isEmpty()) {
this.url = href;
}
return;
}
}

View File

@@ -1,6 +1,88 @@
import { describe, expect, test } from "bun:test";
describe("Server", () => {
test.only("normlizes incoming request URLs", async () => {
const server = Bun.serve({
fetch(request) {
return new Response(request.url, {
headers: {
"Connection": "close",
},
});
},
port: 0,
});
const received: string[] = [];
const expected: string[] = [];
for (let path of [
"/",
"/../",
"/./",
"/foo",
"/foo/",
"/foo/bar",
"/foo/bar/",
"/foo/bar/..",
"/foo/bar/../",
"/foo/bar/../?123",
"/foo/bar/../?123=456",
"/foo/bar/../#123=456",
"/",
"/../",
"/./",
"/foo",
"/foo/",
"/foo/bar",
"/foo/bar/",
"/foo/bar/..",
"/foo/bar/../",
"/foo/bar/../?123",
"/foo/bar/../?123=456",
"/foo/bar/../#123=456",
"/../".repeat(128),
"/./".repeat(128),
"/foo".repeat(128),
"/foo/".repeat(128),
"/foo/bar".repeat(128),
"/foo/bar/".repeat(128),
"/foo/bar/..".repeat(128),
"/foo/bar/../".repeat(128),
"/../".repeat(128),
"/./".repeat(128),
"/foo".repeat(128),
"/foo/".repeat(128),
"/foo/bar".repeat(128),
"/foo/bar/".repeat(128),
"/foo/bar/..".repeat(128),
"/foo/bar/../".repeat(128),
]) {
expected.push(new URL(path, "http://localhost:" + server.port).href);
const { promise, resolve } = Promise.withResolvers();
Bun.connect({
hostname: server.hostname,
port: server.port,
socket: {
async open(socket) {
socket.write(`GET ${path} HTTP/1.1\r\nHost: localhost:${server.port}\r\n\r\n`);
await socket.flush();
},
async data(socket, data) {
const lines = Buffer.from(data).toString("utf8");
received.push(lines.split("\r\n\r\n").at(-1)!);
await socket.end();
resolve();
},
},
});
await promise;
}
server.stop(true);
expect(received).toEqual(expected);
});
test("should not allow Bun.serve without first argument being a object", () => {
expect(() => {
//@ts-ignore