Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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: 2 additions & 1 deletion packages/bun-uws/src/App.h
Original file line number Diff line number Diff line change
Expand Up @@ -767,9 +767,10 @@ struct TemplatedApp {
return std::move(*this);
}

TemplatedApp &&setFlags(bool requireHostHeader, bool useStrictMethodValidation) {
TemplatedApp &&setFlags(bool requireHostHeader, bool useStrictMethodValidation, bool insecureHTTPParser) {
httpContext->getSocketContextData()->flags.requireHostHeader = requireHostHeader;
httpContext->getSocketContextData()->flags.useStrictMethodValidation = useStrictMethodValidation;
httpContext->getSocketContextData()->flags.insecureHTTPParser = insecureHTTPParser;
return std::move(*this);
}

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 @@ -278,7 +278,7 @@ struct HttpContext {

/* The return value is entirely up to us to interpret. The HttpParser cares only for whether the returned value is DIFFERENT from passed user */

auto result = httpResponseData->consumePostPadded(httpContextData->maxHeaderSize, httpResponseData->isConnectRequest, httpContextData->flags.requireHostHeader,httpContextData->flags.useStrictMethodValidation, data, (unsigned int) length, s, proxyParser, [httpContextData](void *s, HttpRequest *httpRequest) -> void * {
auto result = httpResponseData->consumePostPadded(httpContextData->maxHeaderSize, httpResponseData->isConnectRequest, httpContextData->flags.requireHostHeader,httpContextData->flags.useStrictMethodValidation, httpContextData->flags.insecureHTTPParser, data, (unsigned int) length, s, proxyParser, [httpContextData](void *s, HttpRequest *httpRequest) -> void * {


/* For every request we reset the timeout and hang until user makes action */
Expand Down
1 change: 1 addition & 0 deletions packages/bun-uws/src/HttpContextData.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ struct HttpFlags {
bool requireHostHeader: 1 = true;
bool isAuthorized: 1 = false;
bool useStrictMethodValidation: 1 = false;
bool insecureHTTPParser: 1 = false;
};

template <bool SSL>
Expand Down
46 changes: 38 additions & 8 deletions packages/bun-uws/src/HttpParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,7 @@
}

/* End is only used for the proxy parser. The HTTP parser recognizes "\ra" as invalid "\r\n" scan and breaks. */
static HttpParserResult getHeaders(char *postPaddedBuffer, char *end, struct HttpRequest::Header *headers, void *reserved, bool &isAncientHTTP, bool &isConnectRequest, bool useStrictMethodValidation, uint64_t maxHeaderSize) {
static HttpParserResult getHeaders(char *postPaddedBuffer, char *end, struct HttpRequest::Header *headers, void *reserved, bool &isAncientHTTP, bool &isConnectRequest, bool useStrictMethodValidation, bool insecureHTTPParser, uint64_t maxHeaderSize) {
char *preliminaryKey, *preliminaryValue, *start = postPaddedBuffer;
#ifdef UWS_WITH_PROXY
/* ProxyParser is passed as reserved parameter */
Expand Down Expand Up @@ -785,6 +785,7 @@
postPaddedBuffer++;

preliminaryValue = postPaddedBuffer;
bool hasObsFold = false;
/* The goal of this call is to find next "\r\n", or any invalid field value chars, fast */
while (true) {
postPaddedBuffer = tryConsumeFieldValue(postPaddedBuffer);
Expand All @@ -798,6 +799,15 @@
/* Error - invalid chars in field value */
return HttpParserResult::error(HTTP_ERROR_400_BAD_REQUEST, HTTP_PARSER_ERROR_INVALID_HEADER_TOKEN);
}
/* RFC 7230 obs-fold: CRLF followed by SP/HTAB continues the preceding
* header field value. Only honored when the lenient parser is enabled,
* matching Node.js's insecureHTTPParser option (llhttp LENIENT_HEADERS). */
if (insecureHTTPParser && postPaddedBuffer + 2 < end && postPaddedBuffer[1] == '\n' &&
(postPaddedBuffer[2] == ' ' || postPaddedBuffer[2] == '\t')) [[unlikely]] {
hasObsFold = true;
postPaddedBuffer += 2;
continue;
}
break;
}
if(maxHeaderSize && (uintptr_t)(postPaddedBuffer - headerStart) > maxHeaderSize) {
Expand All @@ -810,7 +820,20 @@
* This way we can have this one single check to see if we found \r\n WITHIN our allowed search space. */
if (postPaddedBuffer[1] == '\n') {
/* Store this header, it is valid */
headers->value = std::string_view(preliminaryValue, (size_t) (postPaddedBuffer - preliminaryValue));
if (hasObsFold) [[unlikely]] {
/* Compact the value in place, dropping the CRLF pairs skipped above,
* then SP-fill the vacated tail so a shortRead re-parse over these
* bytes is idempotent (trimmed as trailing whitespace). */
char *w = preliminaryValue;
for (char *r = preliminaryValue; r < postPaddedBuffer; r++) {
if (*r == '\r') { r++; continue; }
*w++ = *r;
}
headers->value = std::string_view(preliminaryValue, (size_t) (w - preliminaryValue));
Comment thread
robobun marked this conversation as resolved.
while (w < postPaddedBuffer) *w++ = ' ';

Check failure on line 833 in packages/bun-uws/src/HttpParser.h

View check run for this annotation

Claude / Claude Code Review

SP-fill idempotence fix incomplete for multi-fold headers split at intermediate CRLF

The SP-fill idempotence fix in 57e71d39 is incomplete: when a header has ≥2 obs-fold continuations and the packet boundary lands exactly at an *intermediate* fold's CRLF (so `postPaddedBuffer + 2 < end` fails but `end - postPaddedBuffer == 2` still passes the shortRead guard), compaction runs and SP-fills the tail, but the next packet begins with SP/HTAB — so on re-parse the SP-fill bytes land *inside* the value instead of as trailing whitespace. `X-A: one\r\n two\r\n` + ` three\r\n…` yields `"o
Comment thread
robobun marked this conversation as resolved.
} else {
headers->value = std::string_view(preliminaryValue, (size_t) (postPaddedBuffer - preliminaryValue));
}
postPaddedBuffer += 2;
/* Trim trailing whitespace (SP, HTAB) per RFC 9110 Section 5.5 */
while (headers->value.length() && isHTTPHeaderValueWhitespace(headers->value.back())) {
Expand Down Expand Up @@ -857,7 +880,7 @@

/* This is the only caller of getHeaders and is thus the deepest part of the parser. */
template <bool ConsumeMinimally>
HttpParserResult fenceAndConsumePostPadded(uint64_t maxHeaderSize, bool& isConnectRequest, bool requireHostHeader, bool useStrictMethodValidation, char *data, unsigned int length, void *user, void *reserved, HttpRequest *req, MoveOnlyFunction<void *(void *, HttpRequest *)> &requestHandler, MoveOnlyFunction<void *(void *, std::string_view, bool)> &dataHandler) {
HttpParserResult fenceAndConsumePostPadded(uint64_t maxHeaderSize, bool& isConnectRequest, bool requireHostHeader, bool useStrictMethodValidation, bool insecureHTTPParser, char *data, unsigned int length, void *user, void *reserved, HttpRequest *req, MoveOnlyFunction<void *(void *, HttpRequest *)> &requestHandler, MoveOnlyFunction<void *(void *, std::string_view, bool)> &dataHandler) {

/* How much data we CONSUMED (to throw away) */
unsigned int consumedTotal = 0;
Expand All @@ -868,7 +891,7 @@
data[length + 1] = 'a'; /* Anything that is not \n, to trigger "invalid request" */
req->ancientHttp = false;
for (;length;) {
auto result = getHeaders(data, data + length, req->headers, reserved, req->ancientHttp, isConnectRequest, useStrictMethodValidation, maxHeaderSize);
auto result = getHeaders(data, data + length, req->headers, reserved, req->ancientHttp, isConnectRequest, useStrictMethodValidation, insecureHTTPParser, maxHeaderSize);
if(result.isError()) {
return result;
}
Expand Down Expand Up @@ -931,7 +954,14 @@
/* Check Transfer-Encoding header validity and conflicts */
HttpRequest::TransferEncoding transferEncoding = req->getTransferEncoding();

transferEncoding.invalid = transferEncoding.invalid || (transferEncoding.has && (contentLengthStringLen || !transferEncoding.chunked));
if (insecureHTTPParser) [[unlikely]] {
/* Lenient parsing (Node.js insecureHTTPParser): tolerate Content-Length alongside
* Transfer-Encoding and out-of-order chunked; chunked still wins the framing.
* TE without chunked is still rejected since request body framing is undefined. */
transferEncoding.invalid = transferEncoding.has && !transferEncoding.chunked;
} else {
transferEncoding.invalid = transferEncoding.invalid || (transferEncoding.has && (contentLengthStringLen || !transferEncoding.chunked));
}

if (transferEncoding.invalid) [[unlikely]] {
/* Invalid Transfer-Encoding (multiple headers or chunked not last - request smuggling attempt) */
Expand Down Expand Up @@ -1039,7 +1069,7 @@
}

public:
HttpParserResult consumePostPadded(uint64_t maxHeaderSize, bool& isConnectRequest, bool requireHostHeader, bool useStrictMethodValidation, char *data, unsigned int length, void *user, void *reserved, MoveOnlyFunction<void *(void *, HttpRequest *)> &&requestHandler, MoveOnlyFunction<void *(void *, std::string_view, bool)> &&dataHandler) {
HttpParserResult consumePostPadded(uint64_t maxHeaderSize, bool& isConnectRequest, bool requireHostHeader, bool useStrictMethodValidation, bool insecureHTTPParser, char *data, unsigned int length, void *user, void *reserved, MoveOnlyFunction<void *(void *, HttpRequest *)> &&requestHandler, MoveOnlyFunction<void *(void *, std::string_view, bool)> &&dataHandler) {
/* This resets BloomFilter by construction, but later we also reset it again.
* Optimize this to skip resetting twice (req could be made global) */
HttpRequest req;
Expand Down Expand Up @@ -1093,7 +1123,7 @@
fallback.append(data, maxCopyDistance);

// break here on break
HttpParserResult consumed = fenceAndConsumePostPadded<true>(maxHeaderSize, isConnectRequest, requireHostHeader, useStrictMethodValidation, fallback.data(), (unsigned int) fallback.length(), user, reserved, &req, requestHandler, dataHandler);
HttpParserResult consumed = fenceAndConsumePostPadded<true>(maxHeaderSize, isConnectRequest, requireHostHeader, useStrictMethodValidation, insecureHTTPParser, fallback.data(), (unsigned int) fallback.length(), user, reserved, &req, requestHandler, dataHandler);
/* Return data will be different than user if we are upgraded to WebSocket or have an error */
if (consumed.returnedData != user) {
return consumed;
Expand Down Expand Up @@ -1156,7 +1186,7 @@
}
}

HttpParserResult consumed = fenceAndConsumePostPadded<false>(maxHeaderSize, isConnectRequest, requireHostHeader, useStrictMethodValidation, data, length, user, reserved, &req, requestHandler, dataHandler);
HttpParserResult consumed = fenceAndConsumePostPadded<false>(maxHeaderSize, isConnectRequest, requireHostHeader, useStrictMethodValidation, insecureHTTPParser, data, length, user, reserved, &req, requestHandler, dataHandler);
/* Return data will be different than user if we are upgraded to WebSocket or have an error */
if (consumed.returnedData != user) {
return consumed;
Expand Down
1 change: 1 addition & 0 deletions src/js/internal/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const {
server: any,
requireHostHeader: boolean,
useStrictMethodValidation: boolean,
insecureHTTPParser: boolean,
maxHeaderSize: number,
onClientError: (ssl: boolean, socket: any, errorCode: number, rawPacket: ArrayBuffer) => undefined,
) => void;
Expand Down
2 changes: 2 additions & 0 deletions src/js/node/_http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const {
_checkInvalidHeaderChar: checkInvalidHeaderChar,
chunkExpression,
continueExpression,
isLenient,
validateHeaderName,
validateHeaderValue,
} = require("node:_http_common");
Expand Down Expand Up @@ -908,6 +909,7 @@ Server.prototype[kRealListen] = function (tls, port, host, socketPath, reusePort
this[serverSymbol],
this.requireHostHeader,
true,
this.insecureHTTPParser === undefined ? isLenient() : this.insecureHTTPParser,
typeof this.maxHeaderSize !== "undefined" ? this.maxHeaderSize : getMaxHTTPHeaderSize(),
onServerClientError.bind(this),
);
Expand Down
11 changes: 6 additions & 5 deletions src/jsc/bindings/NodeHTTP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ extern "C" void Request__setInternalEventCallback(void*, EncodedJSValue, JSC::JS
extern "C" void Request__setTimeout(void*, EncodedJSValue, JSC::JSGlobalObject*);
extern "C" bool NodeHTTPResponse__setTimeout(void*, EncodedJSValue, JSC::JSGlobalObject*);
extern "C" void Server__setIdleTimeout(EncodedJSValue, EncodedJSValue, JSC::JSGlobalObject*);
extern "C" EncodedJSValue Server__setAppFlags(JSC::JSGlobalObject*, EncodedJSValue, bool require_host_header, bool use_strict_method_validation);
extern "C" EncodedJSValue Server__setAppFlags(JSC::JSGlobalObject*, EncodedJSValue, bool require_host_header, bool use_strict_method_validation, bool insecure_http_parser);
extern "C" EncodedJSValue Server__setOnClientError(JSC::JSGlobalObject*, EncodedJSValue, EncodedJSValue);
extern "C" EncodedJSValue Server__setMaxHTTPHeaderSize(JSC::JSGlobalObject*, EncodedJSValue, uint64_t);

Expand Down Expand Up @@ -1013,18 +1013,19 @@ JSC_DEFINE_HOST_FUNCTION(jsHTTPSetCustomOptions, (JSGlobalObject * globalObject,
{
auto& vm = JSC::getVM(globalObject);
auto scope = DECLARE_THROW_SCOPE(vm);
ASSERT(callFrame->argumentCount() == 5);
ASSERT(callFrame->argumentCount() == 6);
// This is an internal binding.
JSValue serverValue = callFrame->uncheckedArgument(0);
JSValue requireHostHeader = callFrame->uncheckedArgument(1);
JSValue useStrictMethodValidation = callFrame->uncheckedArgument(2);
JSValue maxHeaderSize = callFrame->uncheckedArgument(3);
JSValue callback = callFrame->uncheckedArgument(4);
JSValue insecureHTTPParser = callFrame->uncheckedArgument(3);
JSValue maxHeaderSize = callFrame->uncheckedArgument(4);
JSValue callback = callFrame->uncheckedArgument(5);

double maxHeaderSizeNumber = maxHeaderSize.toNumber(globalObject);
RETURN_IF_EXCEPTION(scope, {});

Server__setAppFlags(globalObject, JSValue::encode(serverValue), requireHostHeader.toBoolean(globalObject), useStrictMethodValidation.toBoolean(globalObject));
Server__setAppFlags(globalObject, JSValue::encode(serverValue), requireHostHeader.toBoolean(globalObject), useStrictMethodValidation.toBoolean(globalObject), insecureHTTPParser.toBoolean(globalObject));
RETURN_IF_EXCEPTION(scope, {});

Server__setMaxHTTPHeaderSize(globalObject, JSValue::encode(serverValue), maxHeaderSizeNumber);
Expand Down
14 changes: 11 additions & 3 deletions src/runtime/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1460,11 +1460,19 @@ impl<const SSL: bool, const DEBUG: bool> NewServer<SSL, DEBUG> {
self.config.idle_timeout = seconds.min(255) as u8;
}

pub fn set_flags(&mut self, require_host_header: bool, use_strict_method_validation: bool) {
pub fn set_flags(
&mut self,
require_host_header: bool,
use_strict_method_validation: bool,
insecure_http_parser: bool,
) {
if let Some(app) = self.app {
// S012: `NewApp<SSL>` is a ZST opaque — safe `*mut → &mut` deref.
bun_opaque::opaque_deref_mut(app)
.set_flags(require_host_header, use_strict_method_validation);
bun_opaque::opaque_deref_mut(app).set_flags(
require_host_header,
use_strict_method_validation,
insecure_http_parser,
);
}
}

Expand Down
42 changes: 24 additions & 18 deletions src/runtime/server/server_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3688,31 +3688,35 @@ pub(super) fn server_set_app_flags_(
server: JSValue,
require_host_header: bool,
use_strict_method_validation: bool,
insecure_http_parser: bool,
) -> JsResult<JSValue> {
if !server.is_object() {
return Err(global.throw(format_args!(
"Failed to set requireHostHeader: The 'this' value is not a Server."
"Failed to set server flags: The 'this' value is not a Server."
)));
}

if let Some(this) = server.as_::<HTTPServer>() {
// SAFETY: `as_` returned a non-null `*mut` to a live JS-wrapped server.
unsafe { &mut *this }.set_flags(require_host_header, use_strict_method_validation);
} else if let Some(this) = server.as_::<HTTPSServer>() {
// SAFETY: `as_` returned a non-null `*mut` to a live JS-wrapped server.
unsafe { &mut *this }.set_flags(require_host_header, use_strict_method_validation);
} else if let Some(this) = server.as_::<DebugHTTPServer>() {
// SAFETY: `as_` returned a non-null `*mut` to a live JS-wrapped server.
unsafe { &mut *this }.set_flags(require_host_header, use_strict_method_validation);
} else if let Some(this) = server.as_::<DebugHTTPSServer>() {
// SAFETY: `as_` returned a non-null `*mut` to a live JS-wrapped server.
unsafe { &mut *this }.set_flags(require_host_header, use_strict_method_validation);
} else {
return Err(global.throw(format_args!(
"Failed to set timeout: The 'this' value is not a Server."
)));
macro_rules! handle {
($ty:ty) => {
if let Some(this) = server.as_::<$ty>() {
// SAFETY: `as_` returned a non-null `*mut` to a live JS-wrapped server.
unsafe { &mut *this }.set_flags(
require_host_header,
use_strict_method_validation,
insecure_http_parser,
);
return Ok(JSValue::UNDEFINED);
}
};
}
Ok(JSValue::UNDEFINED)
handle!(HTTPServer);
handle!(HTTPSServer);
handle!(DebugHTTPServer);
handle!(DebugHTTPSServer);

Err(global.throw(format_args!(
"Failed to set server flags: The 'this' value is not a Server."
)))
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

pub(super) fn server_set_max_http_header_size_(
Expand Down Expand Up @@ -3762,6 +3766,7 @@ extern "C" fn server_set_app_flags_shim(
server: JSValue,
require_host_header: bool,
use_strict_method_validation: bool,
insecure_http_parser: bool,
) -> JSValue {
host_fn::to_js_host_fn_result(
global,
Expand All @@ -3770,6 +3775,7 @@ extern "C" fn server_set_app_flags_shim(
server,
require_host_header,
use_strict_method_validation,
insecure_http_parser,
),
)
}
Expand Down
9 changes: 8 additions & 1 deletion src/uws_sys/App.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,18 @@ impl<const SSL: bool> App<SSL> {
unsafe { c::uws_app_destroy(Self::SSL_FLAG, this.cast::<uws_app_t>()) }
}

pub fn set_flags(&mut self, require_host_header: bool, use_strict_method_validation: bool) {
pub fn set_flags(
&mut self,
require_host_header: bool,
use_strict_method_validation: bool,
insecure_http_parser: bool,
) {
c::uws_app_set_flags(
Self::SSL_FLAG,
self.as_raw(),
require_host_header,
use_strict_method_validation,
insecure_http_parser,
)
}

Expand Down Expand Up @@ -513,6 +519,7 @@ pub mod c {
app: &mut uws_app_t,
require_host_header: bool,
use_strict_method_validation: bool,
insecure_http_parser: bool,
);
pub(crate) safe fn uws_app_set_max_http_header_size(
ssl: i32,
Expand Down
6 changes: 3 additions & 3 deletions src/uws_sys/libuwsockets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -541,13 +541,13 @@ extern "C"
uwsApp->setMaxHTTPHeaderSize(max_header_size);
}
}
void uws_app_set_flags(int ssl, uws_app_t *app, bool require_host_header, bool use_strict_method_validation) {
void uws_app_set_flags(int ssl, uws_app_t *app, bool require_host_header, bool use_strict_method_validation, bool insecure_http_parser) {
if (ssl) {
uWS::SSLApp *uwsApp = (uWS::SSLApp *)app;
uwsApp->setFlags(require_host_header, use_strict_method_validation);
uwsApp->setFlags(require_host_header, use_strict_method_validation, insecure_http_parser);
} else {
uWS::App *uwsApp = (uWS::App *)app;
uwsApp->setFlags(require_host_header, use_strict_method_validation);
uwsApp->setFlags(require_host_header, use_strict_method_validation, insecure_http_parser);
}
}

Expand Down
Loading
Loading