From 93fe29538791eb9d264e92850ec93d86c87672d7 Mon Sep 17 00:00:00 2001 From: Peter Mondlock Date: Sun, 3 Oct 2021 10:20:33 -0500 Subject: [PATCH 1/3] add a few missing dependencies to dream.opam; allow the dune-project to run cram tests; add a cram test for #118; propose a small fix for #118 --- dream.opam | 4 ++ dune-project | 1 + src/http/error_handler.ml | 70 +++++++++++++++------------ test/cram/dune | 2 + test/cram/http/dune | 6 +++ test/cram/http/econnreset.ml | 34 +++++++++++++ test/cram/http/issue_118_econnreset.t | 18 +++++++ 7 files changed, 103 insertions(+), 32 deletions(-) create mode 100644 test/cram/dune create mode 100644 test/cram/http/dune create mode 100644 test/cram/http/econnreset.ml create mode 100644 test/cram/http/issue_118_econnreset.t diff --git a/dream.opam b/dream.opam index 903f8457..9dc72d81 100644 --- a/dream.opam +++ b/dream.opam @@ -55,6 +55,7 @@ depends: [ "conf-libev" {os != "win32"} "cstruct" {>= "6.0.0"} "dune" {>= "2.7.0"} # --instrument-with. + "emile" "fmt" {>= "0.8.7"} # `Italic. "graphql_parser" "graphql-lwt" @@ -64,9 +65,12 @@ depends: [ "lwt_ssl" "logs" {>= "0.5.0"} "magic-mime" + "mimic" "mirage-clock" "mirage-crypto" {>= "0.8.1"} # AES-256-GCM. "mirage-crypto-rng" {>= "0.8.0"} # Signature of initialize. + "mirage-time" + "mirage-stack" "multipart_form" {>= "0.3.0"} "ocaml" {>= "4.08.0"} "ssl" {>= "0.5.8"} # Ssl.get_negotiated_alpn_protocol. diff --git a/dune-project b/dune-project index 45acd3f0..e1e4354c 100644 --- a/dune-project +++ b/dune-project @@ -1 +1,2 @@ (lang dune 2.7) +(cram enable) diff --git a/src/http/error_handler.ml b/src/http/error_handler.ml index 480f49f9..8eb7c227 100644 --- a/src/http/error_handler.ml +++ b/src/http/error_handler.ml @@ -117,40 +117,46 @@ let customize template (error : Dream.error) = (* First, log the error. *) + let log_handler condition (error : Dream.error) = + let client = + match error.client with + | None -> "" + | Some client -> " (" ^ client ^ ")" + in + + let layer = + match error.layer with + | `TLS -> ["TLS" ^ client] + | `HTTP -> ["HTTP" ^ client] + | `HTTP2 -> ["HTTP/2" ^ client] + | `WebSocket -> ["WebSocket" ^ client] + | `App -> [] + in + + let description, backtrace = + match condition with + | `String string -> string, "" + | `Exn exn -> + let backtrace = Printexc.get_backtrace () in + Printexc.to_string exn, backtrace + in + + let message = String.concat ": " (layer @ [description]) in + + select_log error.severity (fun log -> + log ?request:error.request "%s" message); + backtrace |> Dream__middleware.Log.iter_backtrace (fun line -> + select_log error.severity (fun log -> + log ?request:error.request "%s" line)) + in begin match error.condition with | `Response _ -> () - | `String _ | `Exn _ as condition -> - - let client = - match error.client with - | None -> "" - | Some client -> " (" ^ client ^ ")" - in - - let layer = - match error.layer with - | `TLS -> ["TLS" ^ client] - | `HTTP -> ["HTTP" ^ client] - | `HTTP2 -> ["HTTP/2" ^ client] - | `WebSocket -> ["WebSocket" ^ client] - | `App -> [] - in - - let description, backtrace = - match condition with - | `String string -> string, "" - | `Exn exn -> - let backtrace = Printexc.get_backtrace () in - Printexc.to_string exn, backtrace - in - - let message = String.concat ": " (layer @ [description]) in - - select_log error.severity (fun log -> - log ?request:error.request "%s" message); - backtrace |> Dream__middleware.Log.iter_backtrace (fun line -> - select_log error.severity (fun log -> - log ?request:error.request "%s" line)) + (* TODO is this the right place to handle lower level connection resets? *) + | `Exn (Unix.Unix_error (Unix.ECONNRESET, _, _)) -> + let condition = `String "Connection Reset at Client" in + let severity = `Info in + log_handler condition {error with condition; severity} + | `String _ | `Exn _ as condition -> log_handler condition error end; (* If Dream will not send a response for this error, we are done after diff --git a/test/cram/dune b/test/cram/dune new file mode 100644 index 00000000..9db2463b --- /dev/null +++ b/test/cram/dune @@ -0,0 +1,2 @@ +(cram + (applies_to :whole_subtree)) diff --git a/test/cram/http/dune b/test/cram/http/dune new file mode 100644 index 00000000..5a054089 --- /dev/null +++ b/test/cram/http/dune @@ -0,0 +1,6 @@ +(executable + (name econnreset) + (libraries dream)) + +(cram + (deps %{exe:econnreset.exe})) diff --git a/test/cram/http/econnreset.ml b/test/cram/http/econnreset.ml new file mode 100644 index 00000000..a8e0fbef --- /dev/null +++ b/test/cram/http/econnreset.ml @@ -0,0 +1,34 @@ +(* This file is part of Dream, released under the MIT license. See LICENSE.md + for details, or visit https://github.com/aantron/dream. + + Copyright 2021 Anton Bachin *) + +let serve port = + print_endline "server mode"; + Dream.run ~greeting:false ~port (fun _ -> Unix.sleepf 10.0; Dream.html "Hello") + +let client port = + print_endline "client mode"; + let open Unix in + let fd = socket PF_INET6 SOCK_STREAM 0 in + (* force the client to send a TCP RST packet if it fails during connection *) + setsockopt_optint fd SO_LINGER (Some 0); + let _ = connect fd (ADDR_INET (inet6_addr_loopback ,port)) in + ignore @@ failwith "sending RST"; + shutdown fd SHUTDOWN_ALL + +let () = + let server = ref(false) in + let port = ref(-1) in + let usage = "Test for ECONNRESET errors being reported" in + Arg.parse [ + "-p", Set_int port, "sets the port (required)"; + "-s", Set server, "enables the server on port [port], if not set sends a TCP RST on [port]" + ] (fun _ -> ()) usage; + + let port = !port in + if port > 65535 || port < 1025 then failwith "Port argument (-p) must set and be between 1025-65535"; + + if !server then serve port + else client port + diff --git a/test/cram/http/issue_118_econnreset.t b/test/cram/http/issue_118_econnreset.t new file mode 100644 index 00000000..482909c2 --- /dev/null +++ b/test/cram/http/issue_118_econnreset.t @@ -0,0 +1,18 @@ +Start a Dream server + + $ ./econnreset.exe -s -p 9191 &> test.log & + $ export PID=$! + $ sleep 1 + +Force a connection reset - will log a few errors + + $ ./econnreset.exe -p 9191 + +Does the log contain an error line for the ECONNRESET? An error code of [1] is "good", meaning no line was found. + + $ kill "${PID}" + $ cat test.log | grep 'ERROR' | grep 'ECONNRESET' + +Does the log contain an info line with custom string for the ECONNRESET? + + $ cat test.log | grep 'INFO' | grep 'Connection Reset at Client' | wc -l From e6bdf76ef6f8753b8f8f16ef8890757d6fb6c01c Mon Sep 17 00:00:00 2001 From: Peter Mondlock Date: Sun, 3 Oct 2021 10:42:55 -0500 Subject: [PATCH 2/3] allow econnreset to identify an open port --- test/cram/http/econnreset.ml | 25 ++++++++++++++++++++----- test/cram/http/issue_118_econnreset.t | 5 +++-- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/test/cram/http/econnreset.ml b/test/cram/http/econnreset.ml index a8e0fbef..402d3c11 100644 --- a/test/cram/http/econnreset.ml +++ b/test/cram/http/econnreset.ml @@ -13,21 +13,36 @@ let client port = let fd = socket PF_INET6 SOCK_STREAM 0 in (* force the client to send a TCP RST packet if it fails during connection *) setsockopt_optint fd SO_LINGER (Some 0); - let _ = connect fd (ADDR_INET (inet6_addr_loopback ,port)) in - ignore @@ failwith "sending RST"; - shutdown fd SHUTDOWN_ALL + let _ = connect fd (ADDR_INET (inet6_addr_loopback, port)) in + ignore @@ failwith "sending RST" + +let print_open_port () = + let open Unix in + let fd = socket PF_INET6 SOCK_STREAM 0 in + bind fd (ADDR_INET (inet6_addr_loopback, 0)); + + begin match getsockname fd with + | ADDR_INET (_, port) -> Printf.printf "%d\n" port + | _ -> failwith "Invalid Socket response" + end; + + exit 0 let () = let server = ref(false) in let port = ref(-1) in let usage = "Test for ECONNRESET errors being reported" in Arg.parse [ - "-p", Set_int port, "sets the port (required)"; + "-p", Set_int port, "sets the port to listen on or connect to, if not specified, prints an available TCP port and exits"; "-s", Set server, "enables the server on port [port], if not set sends a TCP RST on [port]" ] (fun _ -> ()) usage; let port = !port in - if port > 65535 || port < 1025 then failwith "Port argument (-p) must set and be between 1025-65535"; + + (* see if we need to print an open port or validate the port *) + if port = -1 then print_open_port () + else if port > 65535 || port < 1025 + then failwith "Port argument (-p) must set and be between 1025-65535"; if !server then serve port else client port diff --git a/test/cram/http/issue_118_econnreset.t b/test/cram/http/issue_118_econnreset.t index 482909c2..bc6800cd 100644 --- a/test/cram/http/issue_118_econnreset.t +++ b/test/cram/http/issue_118_econnreset.t @@ -1,12 +1,13 @@ Start a Dream server - $ ./econnreset.exe -s -p 9191 &> test.log & + $ export PORT=$(./econnreset.exe) + $ ./econnreset.exe -s -p "${PORT}" &> test.log & $ export PID=$! $ sleep 1 Force a connection reset - will log a few errors - $ ./econnreset.exe -p 9191 + $ ./econnreset.exe -p "${PORT}" Does the log contain an error line for the ECONNRESET? An error code of [1] is "good", meaning no line was found. From 78cf84aee8cbc6a94d0bda87a9ec6e6de3182f2b Mon Sep 17 00:00:00 2001 From: Anton Bachin Date: Sun, 12 Nov 2023 19:01:59 +0300 Subject: [PATCH 3/3] Document Dream.run ?socket_path --- src/dream.mli | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/dream.mli b/src/dream.mli index a1fb5609..d9b5e482 100644 --- a/src/dream.mli +++ b/src/dream.mli @@ -2120,6 +2120,8 @@ val run : - [~interface] is the network interface to listen on. Defaults to ["localhost"]. Use ["0.0.0.0"] to listen on all interfaces. - [~port] is the port to listen on. Defaults to [8080]. + - If [~socket_path] is specified, Dream will listen on a Unix domain socket + at the given path, and ignore [~interface] and [~port]. - [~stop] is a promise that causes the server to stop accepting new requests, and {!Dream.run} to return. Requests that have already entered the Web application continue to be processed. The default value is a