LibHTTP+WebDriver+WebServer: Return error from HTTP request parser

Author: https://github.com/kalenikaliaksandr
Commit: https://github.com/SerenityOS/serenity/commit/9220cdc285
Pull-request: https://github.com/SerenityOS/serenity/pull/18002
Reviewed-by: https://github.com/awesomekling

Meta/Lagom/Fuzzers/FuzzHttpRequest.cpp

@@ -11,7 +11,7 @@
 extern "C" int LLVMFuzzerTestOneInput(uint8_t const* data, size_t size)
 {
     auto request_wrapper = HTTP::HttpRequest::from_raw_request(ReadonlyBytes { data, size });
-    if (!request_wrapper.has_value())
+    if (!request_wrapper.is_error())
         return 0;
 
     auto& request = request_wrapper.value();

Userland/Libraries/LibHTTP/HttpRequest.cpp

@@ -75,7 +75,7 @@ ErrorOr<ByteBuffer> HttpRequest::to_raw_request() const
     return builder.to_byte_buffer();
 }
 
-Optional<HttpRequest> HttpRequest::from_raw_request(ReadonlyBytes raw_request)
+ErrorOr<HttpRequest, HttpRequest::ParseError> HttpRequest::from_raw_request(ReadonlyBytes raw_request)
 {
     enum class State {
         InMethod,
@@ -118,7 +118,7 @@ Optional<HttpRequest> HttpRequest::from_raw_request(ReadonlyBytes raw_request)
     while (index < raw_request.size()) {
         // FIXME: Figure out what the appropriate limitations should be.
         if (buffer.size() > 65536)
-            return {};
+            return ParseError::RequestTooLarge;
         switch (state) {
         case State::InMethod:
             if (peek() == ' ') {
@@ -178,9 +178,10 @@ Optional<HttpRequest> HttpRequest::from_raw_request(ReadonlyBytes raw_request)
             if (index == raw_request.size()) {
                 // End of data, so store the body
                 auto maybe_body = ByteBuffer::copy(buffer);
-                // FIXME: Propagate this error somehow.
-                if (maybe_body.is_error())
-                    return {};
+                if (maybe_body.is_error()) {
+                    VERIFY(maybe_body.error().code() == ENOMEM);
+                    return ParseError::OutOfMemory;
+                }
                 body = maybe_body.release_value();
                 buffer.clear();
             }
@@ -208,7 +209,7 @@ Optional<HttpRequest> HttpRequest::from_raw_request(ReadonlyBytes raw_request)
     else if (method == "PUT")
         request.set_method(HTTP::HttpRequest::Method::PUT);
     else
-        return {};
+        return ParseError::UnsupportedMethod;
 
     request.m_headers = move(headers);
     auto url_parts = resource.split_limit('?', 2, SplitBehavior::KeepEmpty);

Userland/Libraries/LibHTTP/HttpRequest.h

@@ -18,6 +18,26 @@ namespace HTTP {
 
 class HttpRequest {
 public:
+    enum class ParseError {
+        RequestTooLarge,
+        OutOfMemory,
+        UnsupportedMethod
+    };
+
+    static StringView parse_error_to_string(ParseError error)
+    {
+        switch (error) {
+        case ParseError::RequestTooLarge:
+            return "Request too large"sv;
+        case ParseError::OutOfMemory:
+            return "Out of memory"sv;
+        case ParseError::UnsupportedMethod:
+            return "Unsupported method"sv;
+        default:
+            VERIFY_NOT_REACHED();
+        }
+    }
+
     enum Method {
         Invalid,
         HEAD,
@@ -61,7 +81,7 @@ public:
 
     void set_headers(HashMap<DeprecatedString, DeprecatedString> const&);
 
-    static Optional<HttpRequest> from_raw_request(ReadonlyBytes);
+    static ErrorOr<HttpRequest, HttpRequest::ParseError> from_raw_request(ReadonlyBytes);
     static Optional<Header> get_http_basic_authentication_header(URL const&);
     static Optional<BasicAuthenticationCredentials> parse_http_basic_authentication_header(DeprecatedString const&);
 

Userland/Libraries/LibWeb/WebDriver/Client.cpp

@@ -182,6 +182,9 @@ Client::Client(NonnullOwnPtr<Core::BufferedTCPSocket> socket, Core::Object* pare
                 [](AK::Error const& error) {
                     warnln("Internal error: {}", error);
                 },
+                [](HTTP::HttpRequest::ParseError const& error) {
+                    warnln("HTTP request parsing error: {}", HTTP::HttpRequest::parse_error_to_string(error));
+                },
                 [this](WebDriver::Error const& error) {
                     if (send_error_response(error).is_error())
                         warnln("Could not send error response");
@@ -221,9 +224,7 @@ ErrorOr<void, Client::WrappedError> Client::on_ready_to_read()
             break;
     }
 
-    m_request = HTTP::HttpRequest::from_raw_request(TRY(builder.to_byte_buffer()));
-    if (!m_request.has_value())
-        return {};
+    m_request = TRY(HTTP::HttpRequest::from_raw_request(TRY(builder.to_byte_buffer())));
 
     auto body = TRY(read_body_as_json());
     TRY(handle_request(move(body)));

Userland/Libraries/LibWeb/WebDriver/Client.h

@@ -109,7 +109,7 @@ protected:
     Client(NonnullOwnPtr<Core::BufferedTCPSocket>, Core::Object* parent);
 
 private:
-    using WrappedError = Variant<AK::Error, WebDriver::Error>;
+    using WrappedError = Variant<AK::Error, HTTP::HttpRequest::ParseError, WebDriver::Error>;
 
     void die();
     ErrorOr<void, WrappedError> on_ready_to_read();

Userland/Services/WebServer/Client.cpp

@@ -97,7 +97,7 @@ void Client::start()
 ErrorOr<bool> Client::handle_request(ReadonlyBytes raw_request)
 {
     auto request_or_error = HTTP::HttpRequest::from_raw_request(raw_request);
-    if (!request_or_error.has_value())
+    if (request_or_error.is_error())
         return false;
     auto& request = request_or_error.value();
     auto resource_decoded = URL::percent_decode(request.resource());