Commit 835abfad authored by Graham Leggett's avatar Graham Leggett
Browse files

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
parent 26ebfa0c

modules/proxy/mod_proxy.c

+6 −3
Original line number Diff line number Diff line
@@ -414,9 +414,13 @@ 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;

    }

}



/* -------------------------------------------------------------- */

/* Setup configurable data */
@@ -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;

modules/proxy/mod_proxy.h

+4 −7
Original line number Diff line number Diff line
@@ -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;

modules/proxy/proxy_http.c

+46 −46
Original line number Diff line number Diff line
@@ -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;