c-cube / tiny_httpd

Minimal HTTP server using good old threads + blocking IO, with a small request router.
https://c-cube.github.io/tiny_httpd
76 stars 11 forks source link

wip: add middlewares #27

Closed c-cube closed 2 years ago

c-cube commented 2 years ago

cc @craff , is that enough for a micro framework, you think? It should also be enough to do statistics if you have a DB backend (or just in memory, really).

craff commented 2 years ago

This sounds good to me. I would add a middleware parameter to create too, for middle ware common to all path + path specific middle_ware though.

c-cube commented 2 years ago

Good idea, done.

c-cube commented 2 years ago

I think we could write a few toy middlewares to test a bit the API:

module type STATS = sig
  val n_connections : unit -> int
  val avg_query_time : unit -> float
end

val statistics : unit -> Middleware.t * (module STATS)

val log : ?log:(string->unit) -> unit -> Middleware.t
(* log queries and responses *)

module type STORAGE = sig
  val set : string -> string -> unit
  val get : string -> string option
end

val session : (module STORAGE) -> Middleware.t

what do you think? are there other good examples?

craff commented 2 years ago

Your example is fine. SSL could be another middleware ?

Le 11 décembre 2021 12:19:06 GMT-10:00, Simon Cruanes @.***> a écrit :

I think we could write a few toy middlewares to test a bit the API:

module type STATS = sig
 val n_connections : unit -> int
 val avg_query_time : unit -> float
end

val statistics : unit -> Middleware.t * (module STATS)

val log : ?log:(string->unit) -> unit -> Middleware.t
(* log queries and responses *)

module Storage : sig
 val set : string -> string -> unit
 val get : string -> string option
end

val session : (module STORAGE) -> Middleware.t

what do you think? are there other good examples?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/c-cube/tiny_httpd/pull/27#issuecomment-991796851

c-cube commented 2 years ago

Never thought of that. SSL is tough, I'm not sure I'm ready to tackle that :sweat_smile: . My philosophy so far was that that's nginx's job (or whatever proxy you have)… Would be a nice contribution though.

craff commented 2 years ago

On 21-12-11 17:58:23, Simon Cruanes wrote:

Never thought of that. SSL is tough, I'm not sure I'm ready to tackle that 😅 . My philosophy so far was that that's nginx's job (or whatever proxy you have)… Would be a nice contribution though.

In real it more a kind of joke. But still could be nice to have an ssl layer based on Leroy's cryptokit to be independent from openssl. I don't see any benefit to use an openssl binding over using nginx or apache proxy.

Maybe the managment of cookies, at least reading them, could be a middleware.

Cheers, Christophe

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. *

-- Christophe Raffalli tél: +689 87 23 11 48 web: http://raffalli.eu

craff commented 2 years ago

I has a deeper look at middleware ... need a bit more thinking, but for timing, if we want to compare tiny_httpd accurately to other solutions, the ideal would start the timer after parsing the first line of the request, until the end of the output of the response. With timing as a middleware we miss some part of the treatment in our timing.

c-cube commented 2 years ago

I think it could work reasonably well, like this:

let log_time h = fun req ~resp ->
  let t1 = now() in
  h req ~resp:(
   fun response ->
    resp response;
    let t2 = now() in
    log_somewhere "request took %f seconds" (t2 -. t1))

we do miss a bit of time spent in outer handlers in parsing req, but that's negligible. We would properly time the sending back of the response, at least.

c-cube commented 2 years ago

Tried it in https://github.com/c-cube/tiny_httpd/pull/27/commits/7685505f280b93e11f821533df72ad9c03c738dd#diff-886864fbae9f805b6111e8db0d075ea8172ecabaf15d941e77016dd939d52e4fR7-R23 :grin:

craff commented 2 years ago

I added the time of arrival in the request... and used it in echo. parsing time is not really neglectabled (because it include reception of all request packet but the first). I think it is good like this and did a PR.

craff commented 2 years ago

A small remark: start_time may be used by some people as a timestamp in data bases as it is a bit more accurate that the time of addition to the data base (for people looking at micro-second precision).

c-cube commented 2 years ago

I don't know if Unix.gettimeofday() is reliable enough on the microsecond scale, but do you think that's a problem. I think your patch is a nice contribution, going to merge it.