Re: [Pound Mailing List] HTTP 1/1 100-continue
Emilio Campos <[email protected]> Mon, 27 Jun 2016 22:13:45 +0200
| Newsgroups | gmane.comp.web.pound.general |
|---|---|
| Message-ID | <[email protected]> |
Hi List, our support department has developed a patch for supporting HTTP header "Expect: 100-continue" and the proper behaviour, we have been testing in our production environment for a week and Pound is managing more than 17 millions of daily connections without any penalty. I wanted to mention that the management of this Header isn't supported in another Privative Load Balancer solutions as Barracuda, the offered solution is disabling the header "Expect: 100-continue", but some privative web applications require this header in a mandatory way, so with this patch the sysadmin can take the decission for enabling / disabling the management of this header with a new directive called "Ignore100continue". *root@localhost # man ./pound.8* * Ignore100continue 0|1* * Ignore Header Expect: 100-continue (default: 1, Ignored). If 0 pound manages Expect: 100-continue headers.* BTW, the patch has been applied in the following joe's source code: https://github.com/goochjj/pound/tree/stage_for_upstream/v2.8a It would be great if this patch could be added in the master branch for future use. Any feedback will be welcomed! Regards! 2016-06-22 21:22 GMT+02:00 Emilio Campos <[email protected]>: > Dear Joe thanks for your comments, I reply you in line. > > > 2016-06-22 14:42 GMT+02:00 Joe Gooch <[email protected]>: > >> I don't know enough about expect continue to know what we should do. >> >> Based on this: >> >> https://support.urbanairship.com/entries/59909909--Expect-100-Continue-Issues-and-Risks >> >> It seems like something I wouldn't ever use. Anytime I've run into it in >> the past it has been to disable it. >> > > Joe I totally agree with you, but there are customer's application that > use it, propietary apps. > > >> >> Based on the above, if pound isn't going to support it, returning code >> 417 Expectation failed is probably the correct thing to do, instead of >> silently dropping the header. (Which would only be a valid course of action >> for GET requests). >> > > It would be great.. right now pound is responding a 500 Internal Server > Error and client thinks the reason is an issue in the load balancer. I > think 417 is a more descriptive error. > > > >> Is that what's happening, or do you have an incoming post request that >> never completes? >> > > No, the connection is closed, in my case the Real Server/Backend is > waiting for a header "Expectation: 100-continue" that never receives and > in the middle of the content transfer from LB to backend and after a few > seconds the Backend closes the active transfer with pound, automatically > the load balancer logs an copy chunk error in syslog and forward to the > client "500 internal server error". > > We are making some tests in Lab, I will update you with results. > > Regards! > > >> >> >> Joe >> >> On Jun 22, 2016, at 4:23 AM, Emilio Campos < >> [email protected]> wrote: >> >> Some advise? >> >> Any information will be appreciated >> >> 2016-06-19 19:54 GMT+02:00 Emilio Campos <[email protected]> >> : >> >>> We are currrently working with pound 2.8a, Joe's version from github and >>> everything works like a charm, but after a few days ago a customer >>> requested us a issue with some transfers. >>> >>> After a review we detected that they use HTTP 1/1 and some large request >>> are sent from the client to server chunked and with header Expect: >>> 100-continue, pound deletes this header and it looks like after a few >>> chunked sent to the backend, the backend sends a unexpected "FIN,ACK" to >>> pound and it closes the communication unexpectedly. >>> >>> I have been reading the code and pound delete this header also I found >>> this in http.c >>> >>> * * we do NOT support the "Expect: 100-continue" headers* >>> * * support may involve severe performance penalties >>> (non-responding back-end, etc)* >>> * * as a stop-gap measure we just skip these headers* >>> >>> Tinkering in google I found a patch for 2.6: >>> >>> >>> *https://bitbucket.org/amplidata/pound/src/f2affa5f867a9718c9dc864e5f1c2e0eddd91b9c/debian/patches/expect_100_continued.patch?at=default&fileviewer=file-view-default >>> <https://bitbucket.org/amplidata/pound/src/f2affa5f867a9718c9dc864e5f1c2e0eddd91b9c/debian/patches/expect_100_continued.patch?at=default&fileviewer=file-view-default> >>> * >>> >>> I would like to know if this could be added in the master branch, we >>> could make tests to support it. >>> >>> -- >>> Load balancer distribution - Open Source Project >>> http://www.zenloadbalancer.com >>> Distribution list (subscribe): >>> zenloadbalancer-support-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org >>> >> >> >> >> -- >> Load balancer distribution - Open Source Project >> http://www.zenloadbalancer.com >> Distribution list (subscribe): >> zenloadbalancer-support-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org >> >> > > > -- > Load balancer distribution - Open Source Project > http://www.zenloadbalancer.com > Distribution list (subscribe): > zenloadbalancer-support-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org > -- Load balancer distribution - Open Source Project http://www.zenloadbalancer.com Distribution list (subscribe): zenloadbalancer-support-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
pound28a-support-100continue-zenloadbalancer.patch
(text/x-patch, 13.2 KB)
--- /opt/pound28a-github/config.c 2015-07-10 22:31:15.000000000 +0200
+++ /opt/pound-2.8a-100cont-zenloadbalancer/config.c 2016-06-27 21:39:51.127468467 +0200
@@ -79,7 +79,7 @@
static regex_t Service, ServiceName, URL, OrURLs, HeadRequire, HeadDeny, BackEnd, Emergency, Priority, HAport, HAportAddr;
static regex_t Redirect, TimeOut, Session, Type, TTL, ID, DynScale;
static regex_t ClientCert, AddHeader, DisableProto, SSLAllowClientRenegotiation, SSLHonorCipherOrder, Ciphers;
-static regex_t CAlist, VerifyList, CRLlist, NoHTTPS11, Grace, Include, ConnTO, IgnoreCase, HTTPS;
+static regex_t CAlist, VerifyList, CRLlist, NoHTTPS11, Grace, Include, ConnTO, IgnoreCase, Ignore100continue, HTTPS;
static regex_t Disabled, Threads, CNName, Anonymise, DHParams, ECDHCurve;
static regex_t ControlGroup, ControlUser, ControlMode;
@@ -864,8 +864,8 @@
parse_sess(res);
} else if(!regexec(&DynScale, lin, 4, matches, 0)) {
res->dynscale = atoi(lin + matches[1].rm_so);
- } else if(!regexec(&IgnoreCase, lin, 4, matches, 0)) {
- ign_case = atoi(lin + matches[1].rm_so);
+ } else if(!regexec(&IgnoreCase, lin, 4, matches, 0)) {
+ ign_case = atoi(lin + matches[1].rm_so);
} else if(!regexec(&Disabled, lin, 4, matches, 0)) {
res->disabled = atoi(lin + matches[1].rm_so);
} else if(!regexec(&End, lin, 4, matches, 0)) {
@@ -1037,9 +1037,9 @@
if(regcomp(&m->pat, lin + matches[1].rm_so, REG_ICASE | REG_NEWLINE | REG_EXTENDED))
conf_err("ForceHTTP10 bad pattern");
} else if(!regexec(&Service, lin, 4, matches, 0)) {
- if(res->services == NULL)
+ if(res->services == NULL) {
res->services = parse_service(NULL);
- else {
+ } else {
for(svc = res->services; svc->next; svc = svc->next)
;
svc->next = parse_service(NULL);
@@ -1644,6 +1644,8 @@
be_to = atoi(lin + matches[1].rm_so);
} else if(!regexec(&ConnTO, lin, 4, matches, 0)) {
be_connto = atoi(lin + matches[1].rm_so);
+ } else if(!regexec(&Ignore100continue, lin, 4, matches, 0)) {
+ ignore_100 = atoi(lin + matches[1].rm_so);
} else if(!regexec(&IgnoreCase, lin, 4, matches, 0)) {
ignore_case = atoi(lin + matches[1].rm_so);
#if OPENSSL_VERSION_NUMBER >= 0x0090800fL
@@ -1822,6 +1824,7 @@
|| regcomp(&IncludeDir, "^[ \t]*IncludeDir[ \t]+\"(.+)\"[ \t]*$", REG_ICASE | REG_NEWLINE | REG_EXTENDED)
|| regcomp(&ConnTO, "^[ \t]*ConnTO[ \t]+([1-9][0-9]*)[ \t]*$", REG_ICASE | REG_NEWLINE | REG_EXTENDED)
|| regcomp(&IgnoreCase, "^[ \t]*IgnoreCase[ \t]+([01])[ \t]*$", REG_ICASE | REG_NEWLINE | REG_EXTENDED)
+ || regcomp(&Ignore100continue, "^[ \t]*Ignore100continue[ \t]+([01])[ \t]*$", REG_ICASE | REG_NEWLINE | REG_EXTENDED)
|| regcomp(&HTTPS, "^[ \t]*HTTPS[ \t]*$", REG_ICASE | REG_NEWLINE | REG_EXTENDED)
|| regcomp(&Disabled, "^[ \t]*Disabled[ \t]+([01])[ \t]*$", REG_ICASE | REG_NEWLINE | REG_EXTENDED)
|| regcomp(&DHParams, "^[ \t]*DHParams[ \t]+\"(.+)\"[ \t]*$", REG_ICASE | REG_NEWLINE | REG_EXTENDED)
@@ -1921,6 +1924,7 @@
alive_to = 30;
daemonize = 1;
grace = 30;
+ ignore_100 = 1;
services = NULL;
listeners = NULL;
@@ -2010,6 +2014,7 @@
regfree(&IncludeDir);
regfree(&ConnTO);
regfree(&IgnoreCase);
+ regfree(&Ignore100continue);
regfree(&HTTPS);
regfree(&Disabled);
regfree(&CNName);
--- /opt/pound28a-github/http.c 2015-07-10 22:31:15.000000000 +0200
+++ /opt/pound-2.8a-100cont-zenloadbalancer/http.c 2016-06-27 21:43:30.431463667 +0200
@@ -520,7 +520,7 @@
void
do_http(thr_arg *arg)
{
- int cl_11, be_11, res, chunked, n, sock, no_cont, skip, conn_closed, force_10, sock_proto, is_rpc;
+ int cl_11, be_11, res, chunked, n, sock, no_cont, skip, conn_closed, force_10, sock_proto, is_rpc, exp_cont, read_cl_body;
LISTENER *lstn;
SERVICE *svc;
BACKEND *backend, *cur_backend, *old_backend;
@@ -706,7 +706,7 @@
}
/* check other headers */
- for(chunked = 0, cont = L_1, n = 1; n < MAXHEADERS && headers[n]; n++) {
+ for(chunked = 0, cont = L_1, n = 1, exp_cont = 0; n < MAXHEADERS && headers[n]; n++) {
/* no overflow - see check_header for details */
switch(check_header(headers[n], buf)) {
case HEADER_HOST:
@@ -742,13 +742,14 @@
}
break;
case HEADER_EXPECT:
- /*
- * we do NOT support the "Expect: 100-continue" headers
- * support may involve severe performance penalties (non-responding back-end, etc)
- * as a stop-gap measure we just skip these headers
- */
- if(!strcasecmp("100-continue", buf))
- headers_ok[n] = 0;
+ // By Zen Load Balancer: Supported "Expect: 100-continue" headers
+ if(!strcasecmp("100-continue", buf)) {
+ if (ignore_100 == 1) {
+ headers_ok[n] = 0;
+ } else {
+ exp_cont = 1;
+ }
+ }
break;
case HEADER_ILLEGAL:
if(lstn->log_level > 0) {
@@ -1171,6 +1172,78 @@
BIO_puts(be, "\r\n");
}
+ /* headers from client were sent to backend; if there was an 'Expect: 100-continue' we wait for the backend
+ * to respond and forward this to the client before handling the body of the request.
+ * If server does not reply with 'Continue' we'll close connections and bail out without reading the request.
+ * Note that since we have no timeouts, we'll keep waiting for the server reply.
+ */
+
+ //headers = NULL; /* headers will be read if exp_cont seen */
+ read_cl_body = 1; /* will be reset to 0 if exp_cont and '100 Continue' not rx'd, to skip */
+
+ if (exp_cont) { /* 'Expect: 100-continue' header seen! */
+ logmsg(LOG_INFO, "Managing connection Expect 100-continue");
+
+ /* flush to the back-end */
+ if(cur_backend->be_type == 0 && BIO_flush(be) != 1) {
+ str_be(buf, MAXBUF - 1, cur_backend);
+ end_req = cur_time();
+ addr2str(caddr, MAXBUF - 1, &from_host, 1);
+ logmsg(LOG_NOTICE, "(%lx) e500 for %s error flush to %s/%s: %s (%.3f sec)",
+ pthread_self(), caddr, buf, request, strerror(errno), (end_req - start_req) / 1000000.0);
+ err_reply(cl, h500, lstn->err500);
+ clean_all();
+ return;
+ }
+
+ if((headers = get_headers(be, cl, lstn)) == NULL) {
+ str_be(buf, MAXBUF - 1, cur_backend);
+ end_req = cur_time();
+ addr2str(caddr, MAXBUF - 1, &from_host, 1);
+ logmsg(LOG_NOTICE, "(%lx) e500 for %s response error read headers from %s/%s: %s (%.3f secs)",
+ pthread_self(), caddr, buf, request, strerror(errno), (end_req - start_req) / 1000000.0);
+ err_reply(cl, h500, lstn->err500);
+ clean_all();
+ return;
+ }
+ strncpy(response, headers[0], MAXBUF); /* get_headers made sure of termination & length */
+ be_11 = (response[7] == '1'); /* just in case this is needed somewhere somehow */
+ if (regexec(&RESP_SKIP, response, 0, NULL, 0)) { /* other reply than '100 Continue' ? */
+ logmsg(LOG_NOTICE, "(%lx) didn't get '100 Continue' but '%s' response from backend", pthread_self(), response);
+ conn_closed = 1; /* force be connection to close; probably overkill but safe */
+ read_cl_body = 0; /* skip reading client body */
+ } else {
+ /* send the response to client */
+ for(n = 0; n < MAXHEADERS && headers[n]; n++) {
+ /* logmsg(LOG_NOTICE, "(%lx) sending '%s' to client", pthread_self(), headers[n]); */
+ if(BIO_printf(cl, "%s\r\n", headers[n]) <= 0) {
+ if(errno) {
+ addr2str(caddr, MAXBUF - 1, &from_host, 1);
+ logmsg(LOG_NOTICE, "(%lx) error write headers to %s: %s", pthread_self(), caddr, strerror(errno));
+ }
+ free_headers(headers);
+ clean_all();
+ return;
+ }
+ }
+ free_headers(headers);
+ headers = NULL;
+
+ /* final CRLF */
+ BIO_puts(cl, "\r\n");
+ if(BIO_flush(cl) != 1) {
+ if(errno) {
+ addr2str(caddr, MAXBUF - 1, &from_host, 1);
+ logmsg(LOG_NOTICE, "(%lx) error flush headers to %s: %s", pthread_self(), caddr, strerror(errno));
+ }
+ clean_all();
+ return;
+ }
+ }
+ }
+
+ if (read_cl_body) { /* read client body if '100 Continue' rx'd or no 'Expect: 100-continue' seen */
+
if(cl_11 && chunked) {
/* had Transfer-encoding: chunked so read/write all the chunks (HTTP/1.1 only) */
if(copy_chunks(cl, be, NULL, cur_backend->be_type, lstn->max_req)) {
@@ -1259,7 +1332,8 @@
BIO_flush(be);
}
}
- }
+ }
+ }
/* flush to the back-end */
if(cur_backend->be_type == 0 && BIO_flush(be) != 1) {
@@ -1413,7 +1487,8 @@
break;
}
- /* get the response */
+ /* we'll arrive directly here if '100 Continue' was expected but not received; headers will be valid then */
+ /* get the response */
for(skip = 1; skip;) {
if((headers = get_headers(be, cl, lstn)) == NULL) {
str_be(buf, MAXBUF - 1, cur_backend);
@@ -1429,11 +1504,17 @@
strncpy(response, headers[0], MAXBUF);
be_11 = (response[7] == '1');
/* responses with code 100 are never passed back to the client */
- skip = !regexec(&RESP_SKIP, response, 0, NULL, 0);
+ if (exp_cont == 1){
+ skip = 0; /* was: !regexec(&RESP_SKIP, response, 0, NULL, 0); to ignore 100_continue */
/* some response codes (1xx, 204, 304) have no content */
+ } else {
+ skip = !regexec(&RESP_SKIP, response, 0, NULL, 0);
+ }
if(!no_cont && !regexec(&RESP_IGN, response, 0, NULL, 0))
no_cont = 1;
+ for(n = 0; n < MAXHEADERS; n++)
+ headers_ok[n] = 1;
for(chunked = 0, cont = -1L, n = 1; n < MAXHEADERS && headers[n]; n++) {
switch(check_header(headers[n], buf)) {
case HEADER_CONNECTION:
@@ -1493,6 +1574,8 @@
/* send the response */
if(!skip)
for(n = 0; n < MAXHEADERS && headers[n]; n++) {
+ if(!headers_ok[n])
+ continue;
if(BIO_printf(cl, "%s\r\n", headers[n]) <= 0) {
if(errno) {
addr2str(caddr, MAXBUF - 1, &from_host, 1);
@@ -1504,6 +1587,9 @@
}
}
free_headers(headers);
+ if(!skip && ssl && svc->sts >= 0)
+ BIO_printf(cl, "Strict-Transport-Security: max-age=%d\r\n", svc->sts);
+
if(!skip && cur_backend->be_type == 0 && svc->becookie && cur_backend->bekey) {
char *cp = buf;
char *ep = buf + sizeof(buf) - 1;
@@ -1553,6 +1639,7 @@
if(!no_cont) {
/* ignore this if request was HEAD or similar */
if(be_11 && chunked) {
+ logmsg(LOG_INFO, "Executing copy_chunks");
/* had Transfer-encoding: chunked so read/write all the chunks (HTTP/1.1 only) */
if(copy_chunks(be, cl, &res_bytes, skip, L0)) {
/* copy_chunks() has its own error messages */
--- /opt/pound28a-github/pound.h 2015-07-10 22:31:15.000000000 +0200
+++ /opt/pound-2.8a-100cont-zenloadbalancer/pound.h 2016-06-27 21:25:48.907486905 +0200
@@ -280,7 +280,10 @@
log_facility, /* log facility to use */
print_log, /* print log messages to stdout/stderr */
grace, /* grace period before shutdown */
- control_sock; /* control socket */
+ control_sock, /* control socket */
+ ignore_100; /* ignore header "Expect: 100-continue"*/
+ /* 1 Ignore header (Default)*/
+ /* 0 Manages header */
extern regex_t HEADER, /* Allowed header */
CHUNK_HEAD, /* chunk header line */
--- /opt/pound28a-github/pound.8 2015-07-10 22:31:15.000000000 +0200
+++ /opt/pound-2.8a-100cont-zenloadbalancer/pound.8 2016-06-27 21:09:45.000000000 +0200
@@ -275,6 +275,10 @@
Ignore case when matching URLs (default: 0). This value can be
overridden for specific services.
.TP
+\fBIgnore100continue\fR 0|1
+Ignore Header Expect: 100-continue (default: 1, Ignored).
+If 0 pound manages Expect: 100-continue headers.
+.TP
\fBDynScale\fR 0|1
Enable or disable the dynamic rescaling code (default: 0). If enabled
.B Pound