Reworked the storage of the client socket between keepalive connections

to fix some nasty problems with the socket lasting longer than the
memory pool it was allocated from.
PR:
Obtained from:
Reviewed by:

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@88730 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/modules/proxy/mod_proxy.c b/modules/proxy/mod_proxy.c
index 9e53bf2b77f9393555c17559d89cf0bce1ed5f45..ebf2b2f71fea74ac029713a09dc35623274a54ae 100644 (file)
--- a/modules/proxy/mod_proxy.c
+++ b/modules/proxy/mod_proxy.c
@@ -414,8 +414,12 @@ static int proxy_handler(request_rec *r)
     if (strcasecmp(scheme, "ftp") == 0)
        return ap_proxy_ftp_handler(r, NULL, url);
 #endif
-    else
+    else {
+        ap_log_error(APLOG_MARK, APLOG_DEBUG | APLOG_NOERRNO, 0, r->server,
+                    "Neither CONNECT, HTTP or FTP for %s",
+                    r->uri);
        return HTTP_FORBIDDEN;
+    }
 }
 
 /* -------------------------------------------------------------- */
@@ -431,7 +435,7 @@ static void * create_proxy_config(apr_pool_t *p, server_rec *s)
     ps->noproxies = ap_make_array(p, 10, sizeof(struct noproxy_entry));
     ps->dirconn = ap_make_array(p, 10, sizeof(struct dirconn_entry));
     ps->allowed_connect_ports = ap_make_array(p, 10, sizeof(int));
-/*    pc->origin = ap_make_array(p, 10, sizeof(struct origin_entry));*/
+    ps->client_socket = NULL;
     ps->domain = NULL;
     ps->viaopt = via_off; /* initially backward compatible with 1.3.1 */
     ps->viaopt_set = 0; /* 0 means default */
@@ -455,7 +459,6 @@ static void * merge_proxy_config(apr_pool_t *p, void *basev, void *overridesv)
     ps->noproxies = ap_append_arrays(p, base->noproxies, overrides->noproxies);
     ps->dirconn = ap_append_arrays(p, base->dirconn, overrides->dirconn);
     ps->allowed_connect_ports = ap_append_arrays(p, base->allowed_connect_ports, overrides->allowed_connect_ports);
-/*    ps->origin = base->origin;*/
 
     ps->domain = (overrides->domain == NULL) ? base->domain : overrides->domain;
     ps->viaopt = (overrides->viaopt_set == 0) ? base->viaopt : overrides->viaopt;

diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h
index c321dc5c9959ddb1eb56e4d6738902e059f5c82d..7a9bd7923aeba858b6b93c7ab94ad05f589ef78f 100644 (file)
--- a/modules/proxy/mod_proxy.h
+++ b/modules/proxy/mod_proxy.h
@@ -167,11 +167,6 @@ struct noproxy_entry {
     struct apr_sockaddr_t *addr;
 };
 
-struct origin_entry {
-    conn_rec *origin;
-    struct origin_entry *next;
-};
-
 typedef struct {
     apr_array_header_t *proxies;
     apr_array_header_t *aliases;
@@ -179,8 +174,10 @@ typedef struct {
     apr_array_header_t *noproxies;
     apr_array_header_t *dirconn;
     apr_array_header_t *allowed_connect_ports;
-/*    apr_array_header_t *origin_array;*/
-    conn_rec *origin;
+    long id;
+    const char *connectname;
+    apr_port_t connectport;
+    apr_socket_t *client_socket;
     const char *domain;                /* domain name to use in absence of a domain name in the request */
     int req;                   /* true if proxy requests are enabled */
     char req_set;

diff --git a/modules/proxy/proxy_http.c b/modules/proxy/proxy_http.c
index afa45964888ac296dbe2e2b84e82e9de6588b352..352c9f20d73bef190f40bb4160a1b49b4f0a3c2e 100644 (file)
--- a/modules/proxy/proxy_http.c
+++ b/modules/proxy/proxy_http.c
@@ -180,7 +180,7 @@ int ap_proxy_http_handler(request_rec *r, char *url,
                       const char *proxyname, int proxyport)
 {
     request_rec *rp;
-    apr_pool_t *p = r->connection->pool;
+    apr_pool_t *p = r->pool;
     const char *connectname;
     int connectport = 0;
     apr_sockaddr_t *uri_addr;
@@ -271,38 +271,31 @@ int ap_proxy_http_handler(request_rec *r, char *url,
                              connectname, NULL));
     }
 
-    /* if a KeepAlive socket is already open, check whether it must stay
+    /* if a keepalive socket is already open, check whether it must stay
      * open, or whether it should be closed and a new socket created.
      */
-    if (conf->origin) {
-       struct apr_sockaddr_t *remote_addr;
-       apr_port_t port;
-       if ((remote_addr = conf->origin->remote_addr) &&
-           (APR_SUCCESS == apr_sockaddr_port_get(&port, remote_addr)) &&
-            (port == connectport) &&
-            (!apr_strnatcasecmp(conf->origin->remote_addr->hostname,connectname))) {
+    if (conf->client_socket) {
+       if ((conf->id == r->connection->id) &&
+           (conf->connectport == connectport) &&
+           (conf->connectname) &&
+            (!apr_strnatcasecmp(conf->connectname,connectname))) {
            ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server,
                         "proxy: keepalive address match (keep original socket)");
         }
        else {
             ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server,
-                        "proxy: keepalive address mismatch (close old socket (%s/%s, %d/%d))", connectname, conf->origin->remote_addr->hostname, connectport, port);
-            apr_socket_close(conf->origin->client_socket);
-            conf->origin = NULL;
+                        "proxy: keepalive address mismatch (close old socket (%s/%s, %d/%d))", connectname, conf->connectname, connectport, conf->connectport);
+            apr_socket_close(conf->client_socket);
+            conf->client_socket = NULL;
        }
     }
 
     /* get a socket - either a keepalive one, or a new one */
     new = 1;
-    if (conf->origin) {
+    if (conf->client_socket) {
 
        /* use previous keepalive socket */
-       origin = conf->origin;
-       sock = origin->client_socket;
-       origin->aborted = 0;
-       origin->keepalive = 1;
-       origin->keepalives++;
-       origin->remain = 0;
+       sock = conf->client_socket;
        new = 0;
 
        /* XXX FIXME: If the socket has since closed, change new to 1 so
@@ -311,7 +304,9 @@ int ap_proxy_http_handler(request_rec *r, char *url,
     if (new) {
 
        /* create a new socket */
-       if ((apr_socket_create(&sock, APR_INET, SOCK_STREAM, p)) != APR_SUCCESS) {
+       /* allocate this out of the process pool - if this socket gets lost then the proxy
+        * hangs when the socket is closed...! */
+       if ((apr_socket_create(&sock, APR_INET, SOCK_STREAM, r->server->process->pconf)) != APR_SUCCESS) {
             ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
                         "proxy: error creating socket");
            return HTTP_INTERNAL_SERVER_ERROR;
@@ -325,6 +320,9 @@ int ap_proxy_http_handler(request_rec *r, char *url,
        }
 #endif
 
+       ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server,
+                    "proxy: socket has been created");
+
        /*
         * At this point we have a list of one or more IP addresses of
         * the machine to connect to. If configured, reorder this
@@ -352,22 +350,6 @@ int ap_proxy_http_handler(request_rec *r, char *url,
                continue;
             }
 
-           /* the socket is now open, create a new connection */
-           origin = ap_new_connection(p, r->server, sock, 0);
-           conf->origin = origin;
-           if (!origin) {
-               /* the peer reset the connection already; ap_new_connection() 
-                * closed the socket */
-               ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server,
-                            "proxy: an error occurred creating a new connection to %pI (%s)", connect_addr, connectname);
-               connect_addr = connect_addr->next;
-               continue;
-           }
-
-           /* we use keepalives unless later specified */
-           origin->keepalive = 1;
-           origin->keepalives = 1;
-
            /* if we get here, all is well */
            failed = 0;
            break;
@@ -384,6 +366,30 @@ int ap_proxy_http_handler(request_rec *r, char *url,
        }
     }
 
+    ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server,
+                "proxy: socket is connected");
+
+    /* the socket is now open, create a new connection */
+    origin = ap_new_connection(p, r->server, sock, r->connection->id);
+    if (!origin) {
+       /* the peer reset the connection already; ap_new_connection() 
+        * closed the socket */
+       ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server,
+                    "proxy: an error occurred creating a new connection to %pI (%s)", connect_addr, connectname);
+       apr_socket_close(sock);
+       return HTTP_INTERNAL_SERVER_ERROR;
+    }
+    conf->id = r->connection->id;
+    /* allocate this out of the connection pool - the check on r->connection->id makes
+     * sure that this string does not live past the connection lifetime */
+    conf->connectname = apr_pstrdup(r->connection->pool, connectname);
+    conf->connectport = connectport;
+    conf->client_socket = sock;
+
+    ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server,
+                "proxy: connection complete");
+
+
 
     /*
      * Step Three: Send the Request
@@ -392,11 +398,8 @@ int ap_proxy_http_handler(request_rec *r, char *url,
      */
 
     /* set up the connection filters */
-    origin->input_filters = NULL;
     ap_add_input_filter("HTTP_IN", NULL, NULL, origin);
     ap_add_input_filter("CORE_IN", NULL, NULL, origin);
-
-    origin->output_filters = NULL;
     ap_add_output_filter("CORE", NULL, NULL, origin);
 
 
@@ -577,16 +580,16 @@ int ap_proxy_http_handler(request_rec *r, char *url,
        apr_bucket_read(e, (const char **)&response, &len, APR_BLOCK_READ);
     }
     if (len == -1) {
-       conf->origin = NULL;
        apr_socket_close(sock);
+       conf->client_socket = NULL;
        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
             "proxy: error reading from remote server %s (length %d) using ap_get_brigade()",
             connectname, len);
        return ap_proxyerror(r, HTTP_BAD_GATEWAY,
                             "Error reading from remote server");
     } else if (len == 0) {
-       conf->origin = NULL;
        apr_socket_close(sock);
+       conf->client_socket = NULL;
        return ap_proxyerror(r, HTTP_BAD_GATEWAY,
                             "No response data from server");
     }
@@ -604,7 +607,7 @@ int ap_proxy_http_handler(request_rec *r, char *url,
         /* If not an HTTP/1 message or if the status line was > 8192 bytes */
        if (response[5] != '1' || response[len - 1] != '\n') {
            apr_socket_close(sock);
-           conf->origin = NULL;
+           conf->client_socket = NULL;
            return ap_proxyerror(r, HTTP_BAD_GATEWAY,
                                 apr_pstrcat(p, "Corrupt status line returned by remote server: ", response, NULL));
        }
@@ -754,10 +757,7 @@ int ap_proxy_http_handler(request_rec *r, char *url,
      */
     if (close) {
         apr_socket_close(sock);
-        conf->origin = NULL;
-    }
-    else {
-       origin->keptalive = 1;
+       conf->client_socket = NULL;
     }
 
     return OK;