httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Jagielski <...@jaguNET.com>
Subject Re: Enable persistence for SSL connections to the backend for reverse proxy
Date Sat, 08 Dec 2007 18:31:58 GMT

On Dec 8, 2007, at 9:38 AM, Ruediger Pluem wrote:
>
> Thoughts, comments?
>

A *real quick* review.... But so far, I see no issues (not
tested yet though :) )

+1

>
> +static apr_status_t socket_cleanup(proxy_conn_rec *conn)
> +{
> +    if (conn->sock) {
> +        apr_socket_close(conn->sock);
> +        conn->sock = NULL;
> +    }
> +    if (conn->connection) {
> +        apr_pool_cleanup_kill(conn->pool, conn, connection_cleanup);
> +        conn->connection = NULL;
> +    }
> +    return APR_SUCCESS;
> +}
> +

Why not simply void? We don't check the return value
and I see no way to return non-success anyway :)

> +PROXY_DECLARE(apr_status_t) ap_proxy_ssl_connection_cleanup 
> (proxy_conn_rec *conn,
> +                                                             
> request_rec *r)
> +{
>

> ...


>
> Index: modules/proxy/mod_proxy_http.c
>
> +    if (is_ssl) {
> +        ap_proxy_ssl_connection_cleanup(backend, r);
> +    }
> +
>      /* Step One: Determine Who To Connect To */
>      if ((status = ap_proxy_determine_connection(p, r, conf,  
> worker, backend,
>

I assume all the below is to allow non HTTP to also honor SSL
connections, otherwise we'd simply place the function in  
mod_proxy_http.c
and not worry about the API issues... +1

>                                     uri, &url, proxyname,
> Index: modules/proxy/mod_proxy.h
> ===================================================================
> --- modules/proxy/mod_proxy.h	(revision 601660)
> +++ modules/proxy/mod_proxy.h	(working copy)
> @@ -483,6 +483,8 @@
>  PROXY_DECLARE(void) ap_proxy_table_unmerge(apr_pool_t *p,  
> apr_table_t *t, char *key);
>  /* DEPRECATED (will be replaced with ap_proxy_connect_backend */
>  PROXY_DECLARE(int) ap_proxy_connect_to_backend(apr_socket_t **,  
> const char *, apr_sockaddr_t *, const char *, proxy_server_conf *,  
> server_rec *, apr_pool_t *);
> +PROXY_DECLARE(apr_status_t) ap_proxy_ssl_connection_cleanup 
> (proxy_conn_rec *conn,
> +                                                             
> request_rec *r);
>  PROXY_DECLARE(int) ap_proxy_ssl_enable(conn_rec *c);
>  PROXY_DECLARE(int) ap_proxy_ssl_disable(conn_rec *c);
>  PROXY_DECLARE(int) ap_proxy_conn_is_https(conn_rec *c);
> Index: include/ap_mmn.h
> ===================================================================
> --- include/ap_mmn.h	(revision 601660)
> +++ include/ap_mmn.h	(working copy)
> @@ -143,6 +143,7 @@
>   * 20071108.2 (2.3.0-dev)  Add st and keep fields to struct  
> util_ldap_connection_t
>   * 20071108.3 (2.3.0-dev)  Add API guarantee for adding connection  
> filters
>   *                         with non-NULL request_rec pointer  
> (ap_add_*_filter*)
> + * 20071108.4 (2.3.0-dev)  Add ap_proxy_ssl_connection_cleanup
>   */
>
>  #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
> @@ -150,7 +151,7 @@
>  #ifndef MODULE_MAGIC_NUMBER_MAJOR
>  #define MODULE_MAGIC_NUMBER_MAJOR 20071108
>  #endif
> -#define MODULE_MAGIC_NUMBER_MINOR 3                    /* 0...n */
> +#define MODULE_MAGIC_NUMBER_MINOR 4                    /* 0...n */
>
>  /**
>   * Determine if the server's current MODULE_MAGIC_NUMBER is at  
> least a


Mime
View raw message