Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/bun-uws/src/Http3Request.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,7 @@ struct Http3Request {
return getDecodedQueryValue(key, query);
}

/* HttpRequest::getMethod() lowercases in place; we own no writable
* buffer, so write into a per-request scratch instead. */
/* Lower case, like the method names of the HTTP/2 and HTTP/3 routes. The copy goes in a per-request scratch. */
std::string_view getMethod() {
size_t n = method.size() < sizeof(methodLower) ? method.size() : sizeof(methodLower);
for (size_t i = 0; i < n; i++) {
Comment thread
robobun marked this conversation as resolved.
Resolved
Expand Down
4 changes: 2 additions & 2 deletions packages/bun-uws/src/HttpContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -530,11 +530,11 @@ struct HttpContext {
/* Bun.serve routes only the methods of HTTP_METHOD_NAMES. Any other method gets 501 (RFC 9110 15.6.2), and a
* request that no route takes gets 404, as over HTTP/2 and HTTP/3. No handler reads the content of such a request,
* so the connection closes after the answer. node:http's parser is strict: it answers 400 to an unknown method. */
if (!IsNodeHttp && methodIdFromWire(httpRequest->getCaseSensitiveMethod()) == HTTP_METHOD_NONE) [[unlikely]] {
if (!IsNodeHttp && methodIdFromWire(httpRequest->getMethod()) == HTTP_METHOD_NONE) [[unlikely]] {
((HttpResponse<SSL> *) s)->writeStatus("501 Not Implemented")->end({}, true);
} else {
selectedRouter->getUserData() = {(HttpResponse<SSL> *) s, httpRequest};
if (!selectedRouter->route(httpRequest->getCaseSensitiveMethod(), httpRequest->getUrlForRouting())) {
if (!selectedRouter->route(httpRequest->getMethod(), httpRequest->getUrlForRouting())) {
((HttpResponse<SSL> *) s)->writeStatus("404 Not Found")->end({}, true);
}
}
Expand Down
13 changes: 1 addition & 12 deletions packages/bun-uws/src/HttpParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -528,20 +528,9 @@ struct HttpResponseData;
return url;
}

/* Hack: this should be getMethod */
std::string_view getCaseSensitiveMethod()
{
return headers->key;
}

/* The method as the client sent it */
std::string_view getMethod()
{
/* Compatibility hack: lower case method (todo: remove when major version bumps) */
for (unsigned int i = 0; i < headers->key.length(); i++)
{
((char *)headers->key.data())[i] |= 32;
}

return headers->key;
}

Expand Down
38 changes: 38 additions & 0 deletions test/js/bun/http/node-http-pipeline-callbacks.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { expect, test } from "bun:test";
import { once } from "node:events";
import { createServer } from "node:http";
import type { AddressInfo } from "node:net";
import { connect } from "node:net";
Expand Down Expand Up @@ -73,3 +74,40 @@ test("aborting a connection settles every queued pipelined response callback onc
}
}
});

// rawPacket is the read as it arrived. The server used to lowercase the method
// of a request in that buffer when it dispatched the request.
test("'clientError' rawPacket has an earlier request of the same read as the client sent it", async () => {
const payload = "GET /a HTTP/1.1\r\nHost: x\r\n\r\nGET /b HTTP/1.1\r\nHost: x\r\nno colon here\r\n\r\n";
const errored = Promise.withResolvers<{ code: string; rawPacket: string }>();
const requests: string[] = [];
const server = createServer((req, res) => {
requests.push(`${req.method} ${req.url}`);
res.end("ok");
});
server.on("clientError", (err: any, socket) => {
errored.resolve({ code: err.code, rawPacket: err.rawPacket.toString("latin1") });
socket.destroy();
});
await once(server.listen(0, "127.0.0.1"), "listening");
const client = connect((server.address() as AddressInfo).port, "127.0.0.1");
// A connection that ends with no 'clientError' fails the test at once.
client.on("error", errored.reject);
client.on("close", () => errored.reject(new Error("the connection closed with no 'clientError'")));

try {
client.write(payload);

expect({ ...(await errored.promise), requests }).toEqual({
code: "HPE_INVALID_HEADER_TOKEN",
rawPacket: payload,
requests: ["GET /a"],
});
} finally {
client.destroy();
server.closeAllConnections();
if (server.listening) {
await new Promise<void>(resolve => server.close(() => resolve()));
}
}
});
Loading