node:http, node:http2: refuse the malformed requests that Node refuses

packages/bun-uws/src/HttpParser.h

@@ -106,6 +106,8 @@ struct HttpResponseData;
          * body. llhttp reports HPE_INVALID_CONTENT_LENGTH ("Content-Length can't
          * be present with Transfer-Encoding"). node:http compat only. */
         HTTP_PARSER_ERROR_TRAILER_CONTENT_LENGTH = 17,
+        /* A byte in the request target that llhttp rejects (HPE_INVALID_URL). node:http compat only. */
+        HTTP_PARSER_ERROR_INVALID_URL = 18,
     };
 
 
@@ -116,6 +118,7 @@ struct HttpResponseData;
         HTTP_HEADER_PARSER_ERROR_INVALID_METHOD = 3,
         HTTP_HEADER_PARSER_ERROR_REQUEST_HEADER_FIELDS_TOO_LARGE = 4,
         HTTP_HEADER_PARSER_ERROR_PAUSED_H2_UPGRADE = 5,
+        HTTP_HEADER_PARSER_ERROR_INVALID_URL = 6,
     };
 
     struct HttpParserResult {
@@ -639,7 +642,7 @@ struct HttpResponseData;
                 p++;
                 char *valueStart = p;
                 while (true) {
-                    p = tryConsumeFieldValue(p);
+                    p = tryConsumeFieldValue<true>(p);
                     const unsigned char stopByte = (unsigned char) p[0];
                     if (stopByte == '\t') { p++; continue; }
                     /* Same lenient-header-value acceptance as getHeaders. */
@@ -758,6 +761,11 @@ struct HttpResponseData;
             return (((x)-~0ULL/255*(n))&~(x)&~0ULL/255*128);
         }
 
+        /* Nonzero if any byte of x is 0x7f or above. Adding 1 to a byte carries only out of 0xff, which x itself flags. */
+        static inline uint64_t hasDeleteOrAbove(uint64_t x) {
+            return ((x + ~0ULL/255) | x) & (~0ULL/255*128);
+        }
+
         /* RFC 9110 5.6.2. Tokens */
         /* Hyphen is not checked here as it is very common */
         static inline bool isUnlikelyFieldNameByte(unsigned char c)
@@ -890,6 +898,7 @@ struct HttpResponseData;
 
 
         /* Puts method as key, target as value and returns non-null (or nullptr on error). */
+        template <bool IsNodeHttp>
         static inline ConsumeRequestLineResult consumeRequestLine(char *data, char *end, HttpRequest::Header &header, bool useStrictMethodValidation, uint64_t maxHeaderSize) {
             /* Scan until single SP, assume next is / (origin request) */
             char *start = data;
@@ -948,10 +957,23 @@ struct HttpResponseData;
                         return ConsumeRequestLineResult::error(HTTP_HEADER_PARSER_ERROR_REQUEST_HEADER_FIELDS_TOO_LARGE);
                     }
                     if (hasLess(word, 33)) {
-                        while (*(unsigned char *)data > 32) data++;
+                        while (*(unsigned char *)data > 32) {
+                            if constexpr (IsNodeHttp) {
+                                if (*(unsigned char *)data >= 0x7f) [[unlikely]] {
+                                    return ConsumeRequestLineResult::error(HTTP_HEADER_PARSER_ERROR_INVALID_URL);
+                                }
+                            }
+                            data++;
+                        }
                         if(maxHeaderSize && (uintptr_t)(data - start) > maxHeaderSize) {
                             return ConsumeRequestLineResult::error(HTTP_HEADER_PARSER_ERROR_REQUEST_HEADER_FIELDS_TOO_LARGE);
                         }
+                        if constexpr (IsNodeHttp) {
+                            /* A CR at end is the post padding. */
+                            if (data[0] != 32 && !isNewline(data[0])) [[unlikely]] {
+                                return ConsumeRequestLineResult::error(HTTP_HEADER_PARSER_ERROR_INVALID_URL);
+                            }
+                        }
                         /* Now we stand on space */
                         header.value = {start, (size_t) (data - start)};
                         auto nextPosition = data + 11;
@@ -976,6 +998,12 @@ struct HttpResponseData;
                          * post-padding sentinel. Fall through to the version error. */
                         return ConsumeRequestLineResult::error(HTTP_HEADER_PARSER_ERROR_INVALID_HTTP_VERSION);
                     }
+                    if constexpr (IsNodeHttp) {
+                        /* llhttp accepts only 0x21 to 0x7e in a request target, with or without insecureHTTPParser. */
+                        if (hasDeleteOrAbove(word)) [[unlikely]] {
+                            return ConsumeRequestLineResult::error(HTTP_HEADER_PARSER_ERROR_INVALID_URL);
+                        }
+                    }
                 }
             }
 
@@ -1014,27 +1042,37 @@ struct HttpResponseData;
         /* RFC 9110: 5.5 Field Values (TLDR; anything above 31 is allowed; htab (9) is also allowed)
         * Field values are usually constrained to the range of US-ASCII characters [...]
         * Field values containing CR, LF, or NUL characters are invalid and dangerous [...]
-        * Field values containing other CTL characters are also invalid. */
+        * Field values containing other CTL characters are also invalid.
+        * node:http also stops at DEL (127), which llhttp treats like the other CTL characters. */
+        template <bool IsNodeHttp>
         static inline char * tryConsumeFieldValue(char *p) {
             for (; true; p += 8) {
                 uint64_t word;
                 memcpy(&word, p, sizeof(uint64_t));
-                if (hasLess(word, 32)) {
-                    while (*(unsigned char *)p > 31) p++;
-                    return p;
+                if constexpr (IsNodeHttp) {
+                    if (hasLess(word, 32) || hasLess(word ^ (~0ULL/255*127), 1)) {
+                        while (*(unsigned char *)p > 31 && *(unsigned char *)p != 127) p++;
+                        return p;
+                    }
+                } else {
+                    if (hasLess(word, 32)) {
+                        while (*(unsigned char *)p > 31) p++;
+                        return p;
+                    }
                 }
             }
         }
 
         /* The HTTP parser recognizes "\ra" as invalid "\r\n" scan and breaks. maxHeaderFields must not exceed MAX_HEADER_FIELDS: it bounds the writes to headers. */
+        template <bool IsNodeHttp>
         static HttpParserResult getHeaders(char *postPaddedBuffer, char *end, struct HttpRequest::Header *headers, bool &isAncientHTTP, bool &isConnectRequestLine, bool useStrictMethodValidation, bool useInsecureHTTPParser, uint64_t maxHeaderSize, unsigned int maxHeaderFields) {
             char *preliminaryKey, *preliminaryValue, *start = postPaddedBuffer;
 
             /* It is critical for fallback buffering logic that we only return with success
             * if we managed to parse a complete HTTP request (minus data). */
 
             /* The request line is different from the field names / field values */
-            auto requestLineResult = consumeRequestLine(postPaddedBuffer, end, headers[0], useStrictMethodValidation, maxHeaderSize);
+            auto requestLineResult = consumeRequestLine<IsNodeHttp>(postPaddedBuffer, end, headers[0], useStrictMethodValidation, maxHeaderSize);
 
             if (requestLineResult.isErrorOrShortRead()) {
                 /* Error - invalid request line */
@@ -1050,6 +1088,8 @@ struct HttpResponseData;
                         return HttpParserResult::error(HTTP_ERROR_431_REQUEST_HEADER_FIELDS_TOO_LARGE, HTTP_PARSER_ERROR_REQUEST_HEADER_FIELDS_TOO_LARGE);
                     case HTTP_HEADER_PARSER_ERROR_PAUSED_H2_UPGRADE:
                         return HttpParserResult::error(HTTP_ERROR_400_BAD_REQUEST, HTTP_PARSER_ERROR_PAUSED_H2_UPGRADE);
+                    case HTTP_HEADER_PARSER_ERROR_INVALID_URL:
+                        return HttpParserResult::error(HTTP_ERROR_400_BAD_REQUEST, HTTP_PARSER_ERROR_INVALID_URL);
                     default: {
                         /* Short read */
                     }
@@ -1116,7 +1156,7 @@ struct HttpResponseData;
                 preliminaryValue = postPaddedBuffer;
                 /* The goal of this call is to find next "\r\n", or any invalid field value chars, fast */
                 while (true) {
-                    postPaddedBuffer = tryConsumeFieldValue(postPaddedBuffer);
+                    postPaddedBuffer = tryConsumeFieldValue<IsNodeHttp>(postPaddedBuffer);
                     const unsigned char stopByte = (unsigned char) postPaddedBuffer[0];
                     /* If this is not CR then we caught some stinky invalid char on the way */
                     if (stopByte != '\r') {
@@ -1259,7 +1299,7 @@ struct HttpResponseData;
                 return HttpParserResult::success(consumedTotal + length, user);
             }
             bool isConnectRequestLine = false;
-            auto result = getHeaders(data, data + length, req->headers, req->ancientHttp, isConnectRequestLine, useStrictMethodValidation, useInsecureHTTPParser, maxHeaderSize, maxHeaderFields);
+            auto result = getHeaders<IsNodeHttp>(data, data + length, req->headers, req->ancientHttp, isConnectRequestLine, useStrictMethodValidation, useInsecureHTTPParser, maxHeaderSize, maxHeaderFields);
             if(result.isError()) {
                 return result;
             }

src/js/internal/http.ts

@@ -143,9 +143,14 @@ function emitEOFIncomingMessageOuter(self) {
   // none, so skip the native call (and the socket/server option walk) for the
   // common GET/HEAD case.
   if (self[kHandle] !== undefined && !self[noBodySymbol]) {
-    // The lenient (insecureHTTPParser) value bytes must match what the parser
-    // accepted on the wire, or a CTL byte in a trailer value would vanish here.
-    const rawTrailers = self[kHandle].takeRequestTrailers(self.socket?.server?.insecureHTTPParser === true);
+    // The lenient value bytes must match what the parser accepted on the wire,
+    // or a CTL byte in a trailer value would make the trailers vanish here.
+    const server = self.socket?.server;
+    const { calculateLenientFlags, HTTPParser } = require("node:_http_common");
+    const rawTrailers = self[kHandle].takeRequestTrailers(
+      server != null &&
+        calculateLenientFlags(server.httpValidation, server.insecureHTTPParser) !== HTTPParser.kLenientNone,
+    );
     if (rawTrailers !== undefined) {
       self._addHeaderLines(rawTrailers, rawTrailers.length);
     }

src/js/node/_http_server.ts

@@ -1344,6 +1344,7 @@ enum HttpParserError {
   HTTP_PARSER_ERROR_TRAILER_FIELDS_TOO_LARGE = 15,
   HTTP_PARSER_ERROR_CHUNK_TERMINATOR_EXPECTED = 16,
   HTTP_PARSER_ERROR_TRAILER_CONTENT_LENGTH = 17,
+  HTTP_PARSER_ERROR_INVALID_URL = 18,
 }
 // Native callback fired when the HTTP parser rejects incoming bytes. Builds
 // the same error object Node's parser produces and routes it through
@@ -1426,6 +1427,9 @@ function onServerClientError(
     case HttpParserError.HTTP_PARSER_ERROR_CHUNK_TERMINATOR_EXPECTED:
       err = $HPE_STRICT("Parse Error: Expected LF after chunk data");
       break;
+    case HttpParserError.HTTP_PARSER_ERROR_INVALID_URL:
+      err = $HPE_INVALID_URL("Parse Error: Invalid characters in url");
+      break;
     default:
       err = $HPE_INTERNAL("Parse Error");
       break;

src/jsc/bindings/ErrorCode.ts

@@ -358,5 +358,6 @@ const errors: ErrorCodeMapping = [
   ["ERR_INSPECTOR_COMMAND", Error],
   ["ERR_REDIS_SERVER_ERROR", Error, "RedisError"],
   ["ERR_FFI_CC_DISABLED", Error],
+  ["HPE_INVALID_URL", Error],
 ];
 export default errors;