[PATCH] KeepAlive support - first try

Hi all,

The attached two patches implement KeepAlive support.

I don't want to commit these just yet until a few people have given it a look-see, as there are a few outstanding problems with it.

Known Bugs:

• Lack of DECHUNK support from downstream server.

If a downstream server sends a response that is chunked, there is no way for the proxy to determine the content length. As a result the connection between the proxy and the downstream server hangs until the downstream server times out it's connection, causing a delay.

There is DECHUNK filter inside Apache - but it depends on a request_rec being available. In the ap_proxy_http_handler() there is only a conn_rec defined, but no request_rec. Any ideas on the correct way to fix this?

• Connection fails after HEAD request.

If a connection is made to a reverse proxy, and a HEAD request is sent, this request is completed successfully. The problem is that the next request (on the same keepalive connection) will fail with a "Document contains no data" on line 593 of proxy_http.c.

If a third request is sent, this request will work - it would seem that the request directly following a HEAD request (and I suspect any request that returns no body) always fails.

Do you need help?

In addition to our free technical library, Pantek provides professional IT services to companies who utilize Open Source software: Learn more about Pantek's Technical Support offerings Place an order for 24/7/365 Technical Support here Learn more about Pantek, who we are, and what we do Chat with a Live Operator Related httpd.apache.org Links announce bugs cvs dev docs gui-dev modproxy-dev packagers testers Request more information about Pantek

I have tried to track this problem down with no success :(

Regards,
Graham

-- 
-----------------------------------------
minfrin@sharp.fm		"There's a moon
					over Bourbon Street
						tonight..."

diff -u3 -r --exclude=CVS ../../pristine/httpd-2.0/include/httpd.h httpd-2.0/include/httpd.h --- ../../pristine/httpd-2.0/include/httpd.h Mon Mar 26 00:08:43 2001 +++ httpd-2.0/include/httpd.h Thu Mar 29 15:29:20 2001
@@ -884,6 +884,10 @@
/** The length of the current request body * @defvar long remain */ long remain; + + /** Connection to a downstream server/proxy */ + conn_rec *proxy; + }; /* Per-vhost config... */

diff -u3 -r --exclude=CVS ../../../../pristine/httpd-proxy/module-2.0/mod_proxy.c proxy/mod_proxy.c --- ../../../../pristine/httpd-proxy/module-2.0/mod_proxy.c Mon Mar 26 00:35:07 2001 +++ proxy/mod_proxy.c Fri Mar 30 14:53:59 2001
@@ -430,7 +430,6 @@
ps->raliases = ap_make_array(p, 10, sizeof(struct proxy_alias)); ps->noproxies = ap_make_array(p, 10, sizeof(struct noproxy_entry)); ps->dirconn = ap_make_array(p, 10, sizeof(struct dirconn_entry)); - ps->nocaches = ap_make_array(p, 10, sizeof(struct nocache_entry)); ps->allowed_connect_ports = ap_make_array(p, 10, sizeof(int)); ps->cache_completion = DEFAULT_CACHE_COMPLETION; ps->domain = NULL;
@@ -455,7 +454,6 @@
ps->raliases = ap_append_arrays(p, base->raliases, overrides->raliases); ps->noproxies = ap_append_arrays(p, base->noproxies, overrides->noproxies); ps->dirconn = ap_append_arrays(p, base->dirconn, overrides->dirconn); - ps->nocaches = ap_append_arrays(p, base->nocaches, overrides->nocaches); ps->allowed_connect_ports = ap_append_arrays(p, base->allowed_connect_ports, overrides->allowed_connect_ports); ps->domain = (overrides->domain == NULL) ? base->domain : overrides->domain;
@@ -549,6 +547,7 @@
struct noproxy_entry *new; struct noproxy_entry *list = (struct noproxy_entry *) conf->noproxies->elts; struct hostent hp; + struct apr_sockaddr_t *addr; int found = 0; int i;
@@ -563,11 +562,12 @@
new->name = arg; /* Don't do name lookups on things that aren't dotted */ if (ap_strchr_c(arg, '.') != NULL && - ap_proxy_host2addr(new->name, &hp) == NULL) - /*@@@FIXME: This copies only the first of (possibly many) IP addrs */ - memcpy(&new->addr, hp.h_addr, sizeof(struct in_addr)); - else - new->addr.s_addr = 0; + apr_sockaddr_info_get(&addr, new->name, APR_UNSPEC, 0, 0, parms->pool)) { + new->addr = addr; + } + else { + new->addr = NULL; + } } return NULL; } diff -u3 -r --exclude=CVS ../../../../pristine/httpd-proxy/module-2.0/mod_proxy.h proxy/mod_proxy.h --- ../../../../pristine/httpd-proxy/module-2.0/mod_proxy.h Fri Mar 23 23:26:26 2001 +++ proxy/mod_proxy.h Fri Mar 30 14:50:16 2001
@@ -163,12 +163,7 @@
struct noproxy_entry { const char *name; - struct in_addr addr; -}; - -struct nocache_entry { - const char *name; - struct in_addr addr; + struct apr_sockaddr_t *addr; }; typedef struct { diff -u3 -r --exclude=CVS ../../../../pristine/httpd-proxy/module-2.0/proxy_connect.c proxy/proxy_connect.c --- ../../../../pristine/httpd-proxy/module-2.0/proxy_connect.c Fri Mar 23 23:28:17 2001 +++ proxy/proxy_connect.c Thu Mar 29 19:31:29 2001
@@ -143,10 +143,12 @@
} /* check if ProxyBlock directive on this host */ - destaddr.s_addr = ap_inet_addr(host); +/* XXX FIXME */ +/* destaddr.s_addr = ap_inet_addr(host); */ for (i = 0; i < conf->noproxies->nelts; i++) { if ((npent[i].name != NULL && ap_strstr_c(host, npent[i].name) != NULL) - || destaddr.s_addr == npent[i].addr.s_addr || npent[i].name[0] == '*') +/* || destaddr.s_addr == npent[i].addr.s_addr */ + || npent[i].name[0] == '*') return ap_proxyerror(r, HTTP_FORBIDDEN, "Connect to remote machine blocked"); } diff -u3 -r --exclude=CVS ../../../../pristine/httpd-proxy/module-2.0/proxy_http.c proxy/proxy_http.c --- ../../../../pristine/httpd-proxy/module-2.0/proxy_http.c Mon Mar 26 00:35:36 2001 +++ proxy/proxy_http.c Fri Mar 30 17:59:35 2001
@@ -177,70 +177,63 @@
* route.) */ int ap_proxy_http_handler(request_rec *r, char *url, - const char *proxyhost, int proxyport) + const char *proxyname, int proxyport) { - apr_pool_t *p = r->pool; - char *desthost; - int destport = 0; - char *destportstr = NULL; + request_rec *rp; + apr_pool_t *p = r->connection->pool; + struct hostent *connecthost; + const char *connectname; + int connectport = 0; + apr_sockaddr_t *uri_addr; + apr_sockaddr_t *connect_addr; char server_portstr[32]; - const char *uri = NULL; apr_socket_t *sock; - int i, len, backasswards; + int i, j, k, len, backasswards, close=0, failed=0; apr_status_t err; apr_array_header_t *headers_in_array; apr_table_entry_t *headers_in; - struct sockaddr_in server; - struct in_addr destaddr; char buffer[HUGE_STRING_LEN]; char *response; char *buf; conn_rec *origin; apr_bucket *e; apr_bucket_brigade *bb = apr_brigade_create(p); + uri_components uri; void *sconf = r->server->module_config; proxy_server_conf *conf = (proxy_server_conf *) ap_get_module_config(sconf, &proxy_module); - struct noproxy_entry *npent = (struct noproxy_entry *) conf->noproxies->elts; - memset(&server, '\0', sizeof(server)); - server.sin_family = AF_INET; - /* We break the URL into host, port, uri */ - { - const char *buf; + /* + * Step One: Determine Who To Connect To + * + * Break up the URL to determine the host to connect to + */ - uri = strstr(url, "://"); - if (uri == NULL) - return HTTP_BAD_REQUEST; - uri += 3; - destport = DEFAULT_HTTP_PORT; - buf = ap_strchr_c(uri, '/'); - if (buf == NULL) { - desthost = apr_pstrdup(p, uri); - uri = "/"; - } - else { - char *q = apr_palloc(p, buf - uri + 1); - memcpy(q, uri, buf - uri); - q[buf - uri] = '\0'; - uri = buf; - desthost = q; - } + /* we break the URL into host, port, uri */ + if (HTTP_OK != ap_parse_uri_components(p, url, &uri)) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "proxy: URI %s cannot be parsed", url); + return HTTP_BAD_REQUEST; } - /* Get the port number - put it in destport and destportstr */ - { - char *buf; - buf = ap_strchr(desthost, ':'); - if (buf != NULL) { - *(buf++) = '\0'; - if (apr_isdigit(*buf)) { - destport = atoi(buf); - destportstr = buf; - } - } + /* do a DNS lookup for the destination host */ + err = apr_sockaddr_info_get(&uri_addr, uri.hostname, APR_UNSPEC, uri.port, 0, p); + + /* are we connecting directly, or via a proxy? */ + if (proxyname) { + connectname = proxyname; + connectport = proxyport; + err = apr_sockaddr_info_get(&connect_addr, proxyname, APR_UNSPEC, proxyport, 0, p); + } + else { + connectname = uri.hostname; + connectport = uri.port; + connect_addr = uri_addr; + url = apr_pstrcat(p, uri.path, uri.query ? "?" : "", + uri.query ? uri.query : "", uri.fragment ? "#" : "", + uri.fragment ? uri.fragment : "", NULL); } /* Get the server port for the Via headers */
@@ -254,73 +247,192 @@
} /* check if ProxyBlock directive on this host */ - destaddr.s_addr = apr_inet_addr(desthost); - for (i = 0; i < conf->noproxies->nelts; i++) { - if ((npent[i].name != NULL - && ap_strstr_c(desthost, npent[i].name) != NULL) - || destaddr.s_addr == npent[i].addr.s_addr - || npent[i].name[0] == '*') + for (j = 0; j < conf->noproxies->nelts; j++) { + struct noproxy_entry *npent = (struct noproxy_entry *) conf->noproxies->elts; + struct apr_sockaddr_t *conf_addr = npent[j].addr; + if ((npent[j].name && ap_strstr_c(uri.hostname, npent[j].name)) + || npent[j].name[0] == '*') { return ap_proxyerror(r, HTTP_FORBIDDEN, - "Connect to remote machine blocked"); + "Connect to remote machine blocked (by name)"); + } + while (conf_addr) { + while (uri_addr) { + char *conf_ip; + char *uri_ip; + apr_sockaddr_ip_get(&conf_ip, conf_addr); + apr_sockaddr_ip_get(&uri_ip, uri_addr); +/* ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server, + "Testing %s and %s", conf_ip, uri_ip); */ + if (!apr_strnatcasecmp(conf_ip, uri_ip)) { + return ap_proxyerror(r, HTTP_FORBIDDEN, + "Connect to remote machine blocked (by IP address)"); + } + uri_addr = uri_addr->next; + } + conf_addr = conf_addr->next; + } } - if ((apr_socket_create(&sock, APR_INET, SOCK_STREAM, p)) != APR_SUCCESS) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, - "proxy: error creating socket"); - return HTTP_INTERNAL_SERVER_ERROR; + + /* + * Step Two: Make the Connection + * + * We have determined who to connect to. Now make the connection, supporting + * a KeepAlive connection. + */ + + /* get all the possible IP addresses for the destname and loop through them + * until we get a successful connection + */ + if (APR_SUCCESS != err) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "proxy: DNS lookup failure for %s", connectname); + return ap_proxyerror(r, HTTP_BAD_GATEWAY, apr_pstrcat(p, + "DNS lookup failure for: ", + connectname, NULL)); } + else { + + /* 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 (r->connection->proxy) { + struct apr_sockaddr_t *remote_addr; + apr_port_t port; + if ((remote_addr = r->connection->proxy->remote_addr) && + (APR_SUCCESS == apr_sockaddr_port_get(&port, remote_addr)) && + (port == connectport) && + (!apr_strnatcasecmp(r->connection->proxy->remote_addr->hostname,connectname))) { + ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server, + "Keepalive addresses match!!! - keep original socket"); + } + else { + ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server, + "Keepalive addresses do not match - close old socket (%s/%s, %d/%d)", connectname, r->connection->proxy->remote_addr->hostname, connectport, port); + apr_socket_close(r->connection->proxy->client_socket); + r->connection->proxy = NULL; + } + } + + /* get a socket - either a keepalive one, or a new one */ + if (r->connection->proxy) { + + /* use previous keepalive socket */ + origin = r->connection->proxy; + sock = origin->client_socket; + origin->keepalives++; + + } + else { + + /* create a new socket */ + if ((apr_socket_create(&sock, APR_INET, SOCK_STREAM, p)) != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, + "proxy: error creating socket"); + return HTTP_INTERNAL_SERVER_ERROR; + } #if !defined(TPF) && !defined(BEOS) - if (conf->recv_buffer_size > 0 && apr_setsocketopt(sock, APR_SO_RCVBUF, - conf->recv_buffer_size)) { - ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, - "setsockopt(SO_RCVBUF): Failed to set ProxyReceiveBufferSize, using default"); - } + if (conf->recv_buffer_size > 0 && apr_setsocketopt(sock, APR_SO_RCVBUF, + conf->recv_buffer_size)) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, + "setsockopt(SO_RCVBUF): Failed to set ProxyReceiveBufferSize, using default"); + } #endif - if (proxyhost != NULL) { - err = ap_proxy_doconnect(sock, (char *)proxyhost, proxyport, r); - } - else { - err = ap_proxy_doconnect(sock, (char *)desthost, destport, r); - } + /* + * At this point we have a list of one or more IP addresses of + * the machine to connect to. If configured, reorder this + * list so that the "best candidate" is first try. "best + * candidate" could mean the least loaded server, the fastest + * responding server, whatever. + * + * For now we do nothing, ie we get DNS round robin. + * XXX FIXME + */ - if (err != APR_SUCCESS) { - if (proxyhost != NULL) - return DECLINED; /* try again another way */ - else - return ap_proxyerror(r, HTTP_BAD_GATEWAY, apr_pstrcat(p, - "Could not connect to remote machine: ", - desthost, NULL)); - } - - origin = ap_new_connection(p, r->server, sock, 0); - if (!origin) { - /* the peer reset the connection already; ap_new_connection() - * closed the socket */ - /* XXX somebody that knows what they're doing add an error path */ - /* XXX how's this? */ - return ap_proxyerror(r, HTTP_BAD_GATEWAY, apr_pstrcat(p, - "Connection reset by peer: ", - desthost, NULL)); + + /* try each IP address until we connect successfully */ + failed = 1; + while (connect_addr) { + + /* make the connection out of the socket */ + err = apr_connect(sock, connect_addr); + + /* if an error occurred, loop round and try again */ + if (err != APR_SUCCESS) { + ap_log_error(APLOG_MARK, APLOG_ERR, err, r->server, + "proxy connect to %pI (%s) failed", connect_addr, connectname); + connect_addr = connect_addr->next; + continue; + } + + /* the socket is now open, create a new connection */ + origin = ap_new_connection(r->connection->pool, r->server, sock, 0); + r->connection->proxy = 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, + "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; + } + + /* handle a permanent error from the above loop */ + if (failed) { + if (proxyname) { + return DECLINED; + } + else { + return HTTP_BAD_GATEWAY; + } + } + } } + + /* + * Step Three: Send the Request + * + * Send the HTTP/1.1 request to the remote server + */ + ap_add_output_filter("CORE", NULL, NULL, origin); /* strip connection listed hop-by-hop headers from the request */ + /* even though in theory a connection: close coming from the client + * should not affect the connection to the server, it's unlikely + * that subsequent client requests will hit this thread/process, so + * we cancel server keepalive if the client does. + */ + close += ap_proxy_liststr(apr_table_get(r->headers_in, "Connection"), "close"); ap_proxy_clear_connection(p, r->headers_in); + if (close) { + apr_table_mergen(r->headers_in, "Connection", "close"); + origin->keepalive = 0; + } - buf = apr_pstrcat(p, r->method, " ", proxyhost ? url : uri, + buf = apr_pstrcat(p, r->method, " ", url, " HTTP/1.1" CRLF, NULL); e = apr_bucket_pool_create(buf, strlen(buf), p); APR_BRIGADE_INSERT_TAIL(bb, e); - if (destportstr != NULL && destport != DEFAULT_HTTP_PORT) { - buf = apr_pstrcat(p, "Host: ", desthost, ":", destportstr, CRLF, NULL); + if (uri.port_str && uri.port != DEFAULT_HTTP_PORT) { + buf = apr_pstrcat(p, "Host: ", uri.hostname, ":", uri.port_str, CRLF, NULL); e = apr_bucket_pool_create(buf, strlen(buf), p); APR_BRIGADE_INSERT_TAIL(bb, e); } else { - buf = apr_pstrcat(p, "Host: ", desthost, CRLF, NULL); + buf = apr_pstrcat(p, "Host: ", uri.hostname, CRLF, NULL); e = apr_bucket_pool_create(buf, strlen(buf), p); APR_BRIGADE_INSERT_TAIL(bb, e); }
@@ -425,11 +537,6 @@
} - /* we don't yet support keepalives - but we will soon, I promise! */ - buf = apr_pstrcat(p, "Connection: close", CRLF, NULL); - e = apr_bucket_pool_create(buf, strlen(buf), p); - APR_BRIGADE_INSERT_TAIL(bb, e); - /* add empty line at the end of the headers */ e = apr_bucket_pool_create(CRLF, strlen(CRLF), p); APR_BRIGADE_INSERT_TAIL(bb, e);
@@ -439,6 +546,7 @@
ap_pass_brigade(origin->output_filters, bb); /* send the request data, if any. */ + /* XXX FIXME: This doesn't handle large block sizes */ if (ap_should_client_block(r)) { while ((i = ap_get_client_block(r, buffer, sizeof buffer)) > 0) { e = apr_bucket_pool_create(buffer, i, p);
@@ -450,8 +558,17 @@
APR_BRIGADE_INSERT_TAIL(bb, e); ap_pass_brigade(origin->output_filters, bb); + + /* + * Step Four: Receive the Response + * + * Get response from the remote server, and pass it up the + * filter chain + */ + ap_add_input_filter("HTTP_IN", NULL, NULL, origin); ap_add_input_filter("CORE_IN", NULL, NULL, origin); +// ap_add_input_filter("DECHUNK", NULL, rp, origin); apr_brigade_destroy(bb); bb = apr_brigade_create(p);
@@ -463,13 +580,15 @@
e = APR_BRIGADE_FIRST(bb); apr_bucket_read(e, (const char **)&response, &len, APR_BLOCK_READ); if (len == -1) { + r->connection->proxy = NULL; apr_socket_close(sock); ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "ap_get_brigade() - proxy receive - Error reading from remote server %s (length %d)", - proxyhost ? proxyhost : desthost, len); + connectname, len); return ap_proxyerror(r, HTTP_BAD_GATEWAY, "Error reading from remote server"); } else if (len == 0) { + r->connection->proxy = NULL; apr_socket_close(sock); return ap_proxyerror(r, HTTP_BAD_GATEWAY, "Document contains no data");
@@ -488,6 +607,7 @@
/* 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); + r->connection->proxy = NULL; return HTTP_BAD_GATEWAY; } backasswards = 0;
@@ -508,11 +628,13 @@
ap_log_error(APLOG_MARK, APLOG_WARNING|APLOG_NOERRNO, 0, r->server, "proxy: Bad HTTP/%d.%d header returned by %s (%s)", major, minor, r->uri, r->method); + close += 1; } else { /* strip connection listed hop-by-hop headers from response */ const char *buf; + close += ap_proxy_liststr(apr_table_get(r->headers_out, "Connection"), "close"); ap_proxy_clear_connection(p, r->headers_out); if ((buf = apr_table_get(r->headers_out, "Content-type"))) { r->content_type = apr_pstrdup(p, buf);
@@ -535,12 +657,19 @@
ap_get_server_name(r), server_portstr) ); } + + /* Cancel KeepAlive if HTTP/1.0 or less */ + if ((major < 1) || (minor < 1)) { + close += 1; + origin->keepalive = 0; + } } else { /* an http/0.9 response */ backasswards = 1; r->status = 200; r->status_line = "200 OK"; + close += 1; } /* munge the Location and URI response headers according to ProxyPassReverse */
@@ -563,21 +692,56 @@
APR_BRIGADE_INSERT_TAIL(bb, e); } + /* get content-length */ + { + const char *buf; + if ((buf = apr_table_get(r->headers_out, "Content-Length"))) { + origin->remain = atol(buf); + } + else { + origin->remain = -1; + } + if (close) { + origin->remain = -1; + } + } + + ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server, + "proxy: start body send"); + /* send body */ - /* HTTP/1.0 tells us to read to EOF, rather than content-length bytes */ if (!r->header_only) { - origin->remain = -1; - while (ap_get_brigade(origin->input_filters, bb, AP_MODE_BLOCKING) == APR_SUCCESS) { - if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) { - ap_pass_brigade(r->output_filters, bb); - break; - } - ap_pass_brigade(r->output_filters, bb); - apr_brigade_destroy(bb); - bb = apr_brigade_create(p); - } + + /* if HTTP/1.0, or keepalives are off, read till EOF */ + while (ap_get_brigade(origin->input_filters, bb, AP_MODE_BLOCKING) == APR_SUCCESS) { + if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(bb))) { + ap_pass_brigade(r->output_filters, bb); + break; + } + ap_pass_brigade(r->output_filters, bb); + apr_brigade_destroy(bb); + bb = apr_brigade_create(p); + } + } + + ap_log_error(APLOG_MARK, APLOG_DEBUG|APLOG_NOERRNO, 0, r->server, + "proxy: end body send"); + + + /* + * Step Five: Clean Up + * + * If there are no KeepAlives, or if the connection has been signalled + * to close, close the socket and clean up + */ + + /* if the connection is < HTTP/1.1, or Connection: close, + * we close the socket, otherwise we leave it open for KeepAlive support + */ + if (close) { + apr_socket_close(sock); + r->connection->proxy = NULL; } - apr_socket_close(sock); return OK; } Received on Fri Mar 30 17:10:50 2001