[4.1] 61efcba Inspired by Martin I have stared hard at http_conn for a few days.
Poul-Henning Kamp
phk at FreeBSD.org
Fri Sep 4 15:54:54 CEST 2015
commit 61efcba8d11bfe6a9f105c98830dd771a968883d
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Wed Aug 12 12:35:35 2015 +0000
Inspired by Martin I have stared hard at http_conn for a few days.
This commit cleans up a number of weeds and loose ends:
The complete_function should not muck about with the pipelining or WS
reservation.
Only when we have dissected the headers for good do we know how long
the headers were.
Make the HTTP1_Dissect* functions have consistent parameters.
diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 4c57c4e..d73001b 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -838,8 +838,8 @@ enum sess_close http_DoConnection(struct http *hp);
/* cache_http1_proto.c */
htc_complete_f HTTP1_Complete;
-uint16_t HTTP1_DissectRequest(struct http_conn *htc, struct http *hp);
-uint16_t HTTP1_DissectResponse(struct http *sp, struct http_conn *htc);
+uint16_t HTTP1_DissectRequest(struct http_conn *, struct http *);
+uint16_t HTTP1_DissectResponse(struct http_conn *, struct http *);
unsigned HTTP1_Write(const struct worker *w, const struct http *hp, const int*);
#define HTTPH(a, b, c) extern char b[];
diff --git a/bin/varnishd/cache/cache_session.c b/bin/varnishd/cache/cache_session.c
index 7476419..21f405f 100644
--- a/bin/varnishd/cache/cache_session.c
+++ b/bin/varnishd/cache/cache_session.c
@@ -219,7 +219,7 @@ SES_Rx(struct http_conn *htc, double tmo)
* Receive a request/packet/whatever, with timeouts
*
* t0 is when we start
- * *t1 becomes time of first non-idle rx
+ * *t1 becomes time of first non-idle rx
* *t2 becomes time of complete rx
* ti is when we return IDLE if nothing has arrived
* tn is when we timeout on non-complete
@@ -266,6 +266,7 @@ SES_RxStuff(struct http_conn *htc, htc_complete_f *func, double t0,
return (HTC_S_JUNK);
}
if (hs == HTC_S_COMPLETE) {
+ WS_ReleaseP(htc->ws, htc->rxbuf_e);
/* Got it, run with it */
if (t1 != NULL && isnan(*t1))
*t1 = now;
diff --git a/bin/varnishd/http1/cache_http1_fetch.c b/bin/varnishd/http1/cache_http1_fetch.c
index 2a2099b..ca1df5a 100644
--- a/bin/varnishd/http1/cache_http1_fetch.c
+++ b/bin/varnishd/http1/cache_http1_fetch.c
@@ -138,7 +138,7 @@ V1F_FetchRespHdr(struct busyobj *bo)
struct http *hp;
enum htc_status_e hs;
- int first;
+ int first, i;
struct http_conn *htc;
CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
@@ -188,11 +188,13 @@ V1F_FetchRespHdr(struct busyobj *bo)
htc->between_bytes_timeout);
}
} while (hs != HTC_S_COMPLETE);
- bo->acct.beresp_hdrbytes += htc->rxbuf_e - htc->rxbuf_b;
+ WS_ReleaseP(htc->ws, htc->rxbuf_e);
hp = bo->beresp;
- if (HTTP1_DissectResponse(hp, htc)) {
+ i = HTTP1_DissectResponse(htc, hp);
+ bo->acct.beresp_hdrbytes += htc->rxbuf_e - htc->rxbuf_b;
+ if (i) {
VSLb(bo->vsl, SLT_FetchError, "http format error");
htc->doclose = SC_RX_JUNK;
return (-1);
diff --git a/bin/varnishd/http1/cache_http1_fsm.c b/bin/varnishd/http1/cache_http1_fsm.c
index e66fa96..6acf78c 100644
--- a/bin/varnishd/http1/cache_http1_fsm.c
+++ b/bin/varnishd/http1/cache_http1_fsm.c
@@ -159,6 +159,7 @@ HTTP1_Session(struct worker *wrk, struct req *req)
{
enum htc_status_e hs;
struct sess *sp;
+ int i;
CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
@@ -234,9 +235,6 @@ HTTP1_Session(struct worker *wrk, struct req *req)
if (hs != HTC_S_COMPLETE)
WRONG("htc_status (nonbad)");
- req->acct.req_hdrbytes +=
- req->htc->rxbuf_e - req->htc->rxbuf_b;
-
sp->sess_step = S_STP_H1WORKING;
break;
case S_STP_H1BUSY:
@@ -255,7 +253,10 @@ HTTP1_Session(struct worker *wrk, struct req *req)
sp->sess_step = S_STP_H1PROC;
break;
case S_STP_H1WORKING:
- if (http1_dissect(wrk, req)) {
+ i = http1_dissect(wrk, req);
+ req->acct.req_hdrbytes +=
+ req->htc->rxbuf_e - req->htc->rxbuf_b;
+ if (i) {
SES_Close(req->sp, req->doclose);
sp->sess_step = S_STP_H1CLEANUP;
break;
@@ -277,16 +278,16 @@ HTTP1_Session(struct worker *wrk, struct req *req)
return;
SES_RxReInit(req->htc);
if (HTTP1_Complete(req->htc) == HTC_S_COMPLETE) {
+ WS_ReleaseP(req->htc->ws, req->htc->rxbuf_e);
AZ(req->vsl->wid);
req->t_first = req->t_req = sp->t_idle;
wrk->stats->sess_pipeline++;
- req->acct.req_hdrbytes +=
- req->htc->rxbuf_e - req->htc->rxbuf_b;
sp->sess_step = S_STP_H1WORKING;
} else {
if (req->htc->rxbuf_e != req->htc->rxbuf_b)
wrk->stats->sess_readahead++;
sp->sess_step = S_STP_H1NEWREQ;
+
}
break;
default:
diff --git a/bin/varnishd/http1/cache_http1_proto.c b/bin/varnishd/http1/cache_http1_proto.c
index 933c232..77b8f5b 100644
--- a/bin/varnishd/http1/cache_http1_proto.c
+++ b/bin/varnishd/http1/cache_http1_proto.c
@@ -83,6 +83,10 @@ HTTP1_Complete(struct http_conn *htc)
*htc->rxbuf_e = '\0';
return (HTC_S_EMPTY);
}
+ /*
+ * Here we just look for NL[CR]NL to see that reception
+ * is completed. More stringent validation happens later.
+ */
while (1) {
p = strchr(p, '\n');
if (p == NULL)
@@ -93,13 +97,6 @@ HTTP1_Complete(struct http_conn *htc)
if (*p == '\n')
break;
}
- p++;
- WS_ReleaseP(htc->ws, htc->rxbuf_e);
- if (p < htc->rxbuf_e) {
- htc->pipeline_b = p;
- htc->pipeline_e = htc->rxbuf_e;
- htc->rxbuf_e = p;
- }
return (HTC_S_COMPLETE);
}
@@ -109,7 +106,7 @@ HTTP1_Complete(struct http_conn *htc)
*/
static uint16_t
-http1_dissect_hdrs(struct http *hp, char *p, const struct http_conn *htc)
+http1_dissect_hdrs(struct http *hp, char *p, struct http_conn *htc)
{
char *q, *r;
@@ -122,6 +119,8 @@ http1_dissect_hdrs(struct http *hp, char *p, const struct http_conn *htc)
/* Find end of next header */
q = r = p;
+ if (vct_iscrlf(p))
+ break;
while (r < htc->rxbuf_e) {
if (!vct_iscrlf(r)) {
r++;
@@ -132,6 +131,8 @@ http1_dissect_hdrs(struct http *hp, char *p, const struct http_conn *htc)
r += vct_skipcrlf(r);
if (r >= htc->rxbuf_e)
break;
+ if (vct_iscrlf(r))
+ break;
/* If line does not continue: got it. */
if (!vct_issp(*r))
break;
@@ -184,6 +185,13 @@ http1_dissect_hdrs(struct http *hp, char *p, const struct http_conn *htc)
return (400);
}
}
+ if (p < htc->rxbuf_e)
+ p += vct_skipcrlf(p);
+ if (p < htc->rxbuf_e) {
+ htc->pipeline_b = p;
+ htc->pipeline_e = htc->rxbuf_e;
+ htc->rxbuf_e = p;
+ }
return (0);
}
@@ -192,7 +200,7 @@ http1_dissect_hdrs(struct http *hp, char *p, const struct http_conn *htc)
*/
static uint16_t
-http1_splitline(struct http *hp, const struct http_conn *htc, const int *hf)
+http1_splitline(struct http *hp, struct http_conn *htc, const int *hf)
{
char *p;
int i;
@@ -390,7 +398,7 @@ HTTP1_DissectRequest(struct http_conn *htc, struct http *hp)
/*--------------------------------------------------------------------*/
uint16_t
-HTTP1_DissectResponse(struct http *hp, struct http_conn *htc)
+HTTP1_DissectResponse(struct http_conn *htc, struct http *hp)
{
uint16_t retval = 0;
const char *p;
diff --git a/bin/varnishd/proxy/cache_proxy_proto.c b/bin/varnishd/proxy/cache_proxy_proto.c
index f3c9caa..113cea1 100644
--- a/bin/varnishd/proxy/cache_proxy_proto.c
+++ b/bin/varnishd/proxy/cache_proxy_proto.c
@@ -376,7 +376,6 @@ VPX_Proto_Sess(struct worker *wrk, void *priv)
req->htc->pipeline_b = NULL;
else
req->htc->pipeline_e = req->htc->rxbuf_e;
- WS_Release(req->htc->ws, 0);
SES_RxReInit(req->htc);
req->sp->sess_step = S_STP_H1NEWREQ;
wrk->task.func = SES_Proto_Req;
More information about the varnish-commit
mailing list