Skip to content
Draft
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
4 changes: 4 additions & 0 deletions packages/bun-uws/src/Http3Request.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "quic.h"
#include "QueryParser.h"
#include "HttpMethod.h"

#include <cctype>
#include <string_view>
Expand Down Expand Up @@ -66,6 +67,9 @@ struct Http3Request {
return {methodLower, n};
}

/* The id of :method, or HTTP_METHOD_NONE for a token that is none of HTTP_METHOD_NAMES */
uint8_t getMethodId() { return methodIdFromWire(method); }

std::string_view getHeader(std::string_view lowerCasedHeader) {
if (lowerCasedHeader == "host") return authority;
for (unsigned int i = 0; i < headerCount; i++) {
Expand Down
2 changes: 1 addition & 1 deletion packages/bun-uws/src/HttpContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,7 @@ 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->getMethod()) == HTTP_METHOD_NONE) [[unlikely]] {
if (!IsNodeHttp && httpRequest->getMethodId() == HTTP_METHOD_NONE) [[unlikely]] {
((HttpResponse<SSL> *) s)->writeStatus("501 Not Implemented")->end({}, true);
} else {
selectedRouter->getUserData() = {(HttpResponse<SSL> *) s, httpRequest};
Expand Down
6 changes: 6 additions & 0 deletions packages/bun-uws/src/HttpParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,12 @@ struct HttpResponseData;
return headers->key;
}

/* The id of the method, or HTTP_METHOD_NONE for a token that is none of HTTP_METHOD_NAMES */
uint8_t getMethodId()
{
return methodIdFromWire(headers->key);
}

/* Finds and decodes the URI component. */
std::string_view getQuery(std::string_view key)
{
Expand Down
23 changes: 3 additions & 20 deletions src/http_types/Method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use enumset::EnumSet;

#[allow(non_camel_case_types)]
#[repr(u8)]
#[derive(enumset::EnumSetType, Debug)]
#[derive(enumset::EnumSetType, strum::FromRepr, Debug)]
pub enum Method {
ACL = 0,
BIND = 1,
Expand Down Expand Up @@ -48,8 +48,7 @@ pub enum Method {
pub type Set = EnumSet<Method>;

bun_core::comptime_string_map! {
/// The wire form is RFC 9110 case-sensitive uppercase, so the per-request
/// hot path hits the uppercase entries; the all-lower entries exist only
/// For method names from JavaScript; the all-lower entries exist only
/// for `new Request("get", …)` JS-side convenience (mixed-case still
/// rejects).
#[inline]
Expand Down Expand Up @@ -200,24 +199,8 @@ impl Method {
)
}

#[inline]
pub fn find(str: &[u8]) -> Option<Method> {
Self::which(str)
}

/// Looks up the method in `METHOD_MAP` (length dispatch + constant-length
/// word compares; no hashing — a `phf::Map` here cost a SipHash13 round per
/// lookup, ≈ 0.6 % self-time in a Bun.serve hello-world profile, called
/// twice per request).
///
/// `#[inline]`: this lookup should be fully
/// inlined into `NodeHTTPResponse.createForJS` (no separate symbol in the
/// release binary). Without the hint LLVM keeps this as a ~600-byte
/// out-of-line call because the full compare tree looks heavy, even though
/// every per-request caller only ever exercises the len=3 `b"GET"` arm —
/// trivially branch-predicted once the length dispatch is visible
/// at the call site. Showed up as 8 self-time samples (0.09 %) in the
/// `server/node-http` bench from the call alone.
/// word compares; no hashing).
#[inline]
pub fn which(str: &[u8]) -> Option<Method> {
METHOD_MAP.get(str).copied()
Expand Down
9 changes: 1 addition & 8 deletions src/jsc/bindings/NodeHTTP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,7 @@ static void assignHeadersFromUWebSocketsForCall(uWS::HttpRequest* request, JSVal
args.append(jsString(vm, WTF::move(fullURL)));
}

// Get the method.
if (methodString.isUndefinedOrNull()) [[unlikely]] {
std::string_view methodView = request->getMethod();
WTF::String methodString = String::fromUTF8ReplacingInvalidSequences({ reinterpret_cast<const Latin1Character*>(methodView.data()), methodView.length() });
args.append(jsString(vm, WTF::move(methodString)));
} else {
args.append(methodString);
}
args.append(methodString);

uint32_t bits = 0;
// llhttp's F_CONNECTION_CLOSE / F_CONNECTION_UPGRADE: a whole list item.
Expand Down
11 changes: 3 additions & 8 deletions src/runtime/bake/DevServer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1768,11 +1768,7 @@ fn on_js_request(dev: &mut DevServer, req: &mut Request, resp: AnyResponse) {
return resp.corked(move || on_outdated_js_corked(resp));
}

dev.on_js_request_with_bundle(
route_bundle_index,
resp,
Method::which(req.method()).unwrap_or(Method::POST),
);
dev.on_js_request_with_bundle(route_bundle_index, resp, req.method());
}

fn on_asset_request(dev: &mut DevServer, req: &mut Request, resp: AnyResponse) {
Expand Down Expand Up @@ -2096,7 +2092,7 @@ impl ReqOrSaved {
match self {
ReqOrSaved::Req(req) => {
// SAFETY: `req` is a uWS `Request*` valid for the handler callback's duration.
Method::which(unsafe { &**req }.method()).unwrap_or(Method::POST)
unsafe { &**req }.method()
}
ReqOrSaved::Aborted => unreachable!(),
}
Expand Down Expand Up @@ -2126,7 +2122,7 @@ impl DevServer {

let method = match &req {
// SAFETY: r is a uws Request ptr valid for the duration of the handler callback
ReqOrSaved::Req(r) => Method::which(unsafe { &**r }.method()).unwrap_or(Method::GET),
ReqOrSaved::Req(r) => unsafe { &**r }.method(),
_ => unreachable!(),
};

Expand Down Expand Up @@ -2164,7 +2160,6 @@ impl DevServer {
unsafe { &mut *r },
resp,
global,
Some(method),
)? {
Some(saved) => saved,
// Abort the deferral on failure.
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/server/DirectoryRoute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl DirectoryRoute {
}

pub(crate) fn on_request(this: ThisPtr<DirectoryRoute>, req: AnyRequest, resp: AnyResponse) {
let method = Method::find(req.method()).unwrap_or(Method::GET);
let method = req.method();
Self::on(this, req, resp, method);
}

Expand Down
2 changes: 1 addition & 1 deletion src/runtime/server/FileRoute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ impl FileRoute {
}

pub(crate) fn on_request(this: ThisPtr<FileRoute>, req: AnyRequest, resp: AnyResponse) {
let method = Method::find(req.method()).unwrap_or(Method::GET);
let method = req.method();
Self::on(this, req, resp, method);
}

Expand Down
7 changes: 1 addition & 6 deletions src/runtime/server/HTMLBundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,13 +273,8 @@ impl Route {
}

// create the PendingResponse, add it to the list
let Some(method) = Method::which(req.method()) else {
resp.write_status(b"405 Method Not Allowed");
resp.end_without_body(true);
return;
};
let pending = PendingResponse {
method,
method: req.method(),
resp,
_route: RefPtr::from_this(this),
is_response_pending: Cell::new(true),
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/server/NodeHTTPResponse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2748,7 +2748,7 @@ pub(crate) unsafe extern "C" fn NodeHTTPResponse__createForJS(
let request_ref = bun_opaque::opaque_deref(request.cast_const());

let vm = bun_vm_mut(global_object);
let method = HttpMethod::which(request_ref.method()).unwrap_or(HttpMethod::OPTIONS);
let method = request_ref.method();
// Like llhttp, the framing decides, not the method. CONNECT has no body: the parser tunnels every byte after its head.
if method != HttpMethod::CONNECT {
let req_len: usize = 'brk: {
Expand Down
20 changes: 2 additions & 18 deletions src/runtime/server/RequestContext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1400,30 +1400,14 @@ where
}
}

#[inline]
fn req_method(r: *mut Req<SSL_ENABLED, MUX>) -> &'static [u8] {
// SAFETY: r is a live uWS/lsquic request handle for the duration of
// the request callback; both surfaces return request-owned slices.
unsafe {
if MUX {
(*r.cast::<bun_uws_sys::h3::Request>()).method()
} else {
(*r.cast::<bun_uws_sys::Request>()).method()
}
}
}

pub(crate) fn create(
this: &mut core::mem::MaybeUninit<Self>,
server: *mut ThisServer,
req: *mut Req<SSL_ENABLED, MUX>,
resp: uws::AnyResponse,
should_deinit_context: Option<DeferDeinitFlag>,
method: Option<Method>,
method: Method,
) {
let resolved_method = method
.or_else(|| Method::which(Self::req_method(req)))
.unwrap_or(Method::GET);
let any_req = Self::any_request(req);
let range = RangeRequest::raw_from_request(&any_req);
let if_range = RangeRequest::if_range_from_request(&any_req, range);
Expand All @@ -1434,7 +1418,7 @@ where
root: Cell::new(slot),
resp: Cell::new(Some(resp)),
req: Cell::new(Some(req)),
method: resolved_method,
method,
server: Cell::new(
NonNull::new(server).map(|p| bun_ptr::BackRef::from_raw_mut(p.as_ptr())),
),
Expand Down
11 changes: 0 additions & 11 deletions src/runtime/server/ServerConfig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,17 +188,6 @@ pub(crate) enum RouteMethod {
Specific(Method),
}

impl RouteMethod {
/// `None` for [`RouteMethod::Any`]: the method is not known until it is
/// read off the request.
pub(crate) fn specific(&self) -> Option<Method> {
match self {
RouteMethod::Any => None,
RouteMethod::Specific(method) => Some(*method),
}
}
}

impl Default for RouteDeclaration {
fn default() -> Self {
Self {
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/server/StaticRoute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ impl StaticRoute {
}

pub(crate) fn on_request(this: ThisPtr<Self>, req: AnyRequest, resp: AnyResponse) {
let method = Method::find(req.method()).unwrap_or(Method::GET);
let method = req.method();
if method == Method::GET {
Self::on_get(this, req, resp);
} else if method == Method::HEAD {
Expand Down
54 changes: 17 additions & 37 deletions src/runtime/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,6 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
resp: *mut uws_sys::NewAppResponse<SSL>,
should_deinit_context: Option<request_context::DeferDeinitFlag>,
create_js_request: CreateJsRequest,
method: Option<bun_http_types::Method::Method>,
) -> Option<PreparedRequest<SSL, DEBUG>> {
jsc::mark_binding!();
// SAFETY: `this`/`resp` are live for the duration of the uWS callback.
Expand All @@ -746,18 +745,10 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
// we avoid needing to worry as much about what memory to free.
// (RFC 9114 §4.2 transfer-encoding check is H3-only — skipped here.)

// Resolve once, reuse for both `has_request_body()` here and the
// forward to `RequestContext::create` below. With `Method::which` now
// a length-gated match (316a83f) the
// second call is cheap, but the resolved value is also what `create`
// wants — passing `None` made it parse a second time.
let method = method.or_else(|| bun_http_types::Method::Method::which(req.method()));
let method = req.method();

let request_body_length: Option<usize> = 'len: {
if method
.unwrap_or(bun_http_types::Method::Method::OPTIONS)
.has_request_body()
{
if method.has_request_body() {
let len: usize = if let Some(cl) = req.header(b"content-length") {
bun_http_types::parse_content_length(cl)
} else {
Expand Down Expand Up @@ -978,7 +969,6 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
resp,
None,
CreateJsRequest::Bake,
None,
) {
Some(p) => p,
None => return,
Expand Down Expand Up @@ -1151,7 +1141,6 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
resp,
Some(bun_ptr::BackRef::new(&should_deinit_context)),
CreateJsRequest::Yes,
None,
) else {
return;
};
Expand Down Expand Up @@ -1205,7 +1194,6 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
resp,
Some(bun_ptr::BackRef::new(&should_deinit_context)),
CreateJsRequest::No,
user_route.route.method.specific(),
) else {
return;
};
Expand Down Expand Up @@ -1312,21 +1300,19 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
// request, so only the first request for a given method pays the FFI hop
// into `Bun__HTTPMethod__toJS`. (`get(..)` falls back to a fresh intern
// if a future method variant ever indexes past the cache.)
let method_string = match bun_http::Method::find(req.method()) {
Some(m) => match this_ref.method_name_cache.get(m as usize) {
Some(slot) => {
let cached = slot.get();
if cached == JSValue::ZERO {
let v = m.to_js(global);
slot.set(v);
v
} else {
cached
}
let method = req.method();
let method_string = match this_ref.method_name_cache.get(method as usize) {
Some(slot) => {
let cached = slot.get();
if cached == JSValue::ZERO {
let v = method.to_js(global);
slot.set(v);
v
} else {
cached
}
None => m.to_js(global),
},
None => JSValue::UNDEFINED,
}
None => method.to_js(global),
};
let callback = this_ref.config.on_node_http_request;
// C++ forwards `any_server` to `NodeHTTPResponse::create`, which
Expand Down Expand Up @@ -4020,20 +4006,14 @@ impl AnyServer {
req: &mut uws::Request,
resp: uws::AnyResponse,
global: &jsc::JSGlobalObject,
method: Option<bun_http::Method>,
) -> jsc::JsResult<Option<SavedRequest>> {
let req: &mut uws_sys::Request = req;
Ok(any_server_dispatch_resp!(self, resp, |s, r| {
// `s` is the live `*mut NewServer` carried in `self.ptr`,
// tagged at construction in `AnyServer::from`.
let Some(p) = NewServer::prepare_js_request_context(
s,
req,
r,
None,
CreateJsRequest::Bake,
method,
) else {
let Some(p) =
NewServer::prepare_js_request_context(s, req, r, None, CreateJsRequest::Bake)
else {
return Ok(None);
};
Some(p.save(global, req, r))
Expand Down
Loading
Loading