[Ocamlnet-devel] [PATCH] Persistent connections and HTTPS

[Török E. <edw...@et...>]

Hi,

Attached patch implements a workaround to allow persistent connections to work with HTTPS,
avoiding bugs in Https_client#continue. See below for the long explanation.

Persistent HTTP connections work if I enable the aggressive connection cache with Http_client.
However with Https_client I noticed that it always closed and reopened the Ssl connections. 
While this doesn't influence the application's correctness (thanks to the retry mechanisms in Http_client!), it heavily influences its performance: the latency is very bad due to repeated reopened connections, and repeated SSL handshakes ... even on localhost.

Enabling Netlog debugging showed that some Ssl exceptions were thrown each time around read/write/shutdown after the 1st query completed. Further investigation revealed that the problem might be with Https_client#continue:

       method continue fd cb tmo tmo_x host port esys =
-	let mplex =
-	  Uq_ssl.create_ssl_multiplex_controller
-	    ~close_inactive_descr:true
-	    ~preclose:(preclose fd)
-	    ~initial_state:`Client
-	    ~timeout:(tmo, tmo_x)
-	    fd ctx esys in
-	(mplex :> Uq_engines.multiplex_controller)

Problems with this code:
  * creating a new ssl multiplex controller will create a new Ssl.socket (and share just the Ssl.context and fd).
  * the new Ssl.socket doesn't have a defined state (SSL_set_connected_state was not called, and there was no prior handshake either), causing further operations on it to raise errors (if I add some debugging code
to Uq_ssl to print Ssl.get_error_string):
[Thu Sep 19 14:57:29 2013] [debug] [7534:11] Uq_ssl: SSL write error: error:140D0114:SSL routines:SSL_write:uninitialized
[Thu Sep 19 14:57:29 2013] [debug] [7534:11] Uq_ssl: SSL read error: error:140DF114:SSL routines:SSL_read:uninitialized
[Thu Sep 19 14:57:29 2013] [debug] [7534:11] Uq_ssl: SSL shutdown error: error:140E0114:SSL routines:SSL_shutdown:uninitialized
  * the new Ssl.socket is probably missing the state of the handshake (session keys, etc.)

    
The attached workaround implements a very simple workaround: if the esys is still the same
just reuse the previous multiplex controller, I'm guessing this is what the Hashtbl was meant for anyway.

A better solution would be to have something like create_ssl_multiplex_controller_for_existing_session that would take the existing Ssl.socket instead of creating a new one, and then Https_client#continue wouldn't have to drop the connection if esys or tmo changes.

Best regards,
--Edwin