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
