[master] 5b70451 Fixes to and better test-coverage of exception paths in H2 upgrade.

[Poul-Henning Kamp]

commit 5b70451f629cfe80584e8ea9e63221b60256edfa
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Wed Mar 22 08:41:51 2017 +0000

    Fixes to and better test-coverage of exception paths in H2 upgrade.

diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c
index 117690c..60b671c 100644
--- a/bin/varnishd/http2/cache_http2_proto.c
+++ b/bin/varnishd/http2/cache_http2_proto.c
@@ -181,7 +181,6 @@ h2_del_req(struct worker *wrk, struct h2_req *r2)
 	Req_Release(r2->req);
 	if (r)
 		return;
-
 	/* All streams gone, including stream #0, clean up */
 	req = h2->srq;
 	Req_Cleanup(sp, wrk, req);
diff --git a/bin/varnishd/http2/cache_http2_session.c b/bin/varnishd/http2/cache_http2_session.c
index 4c46a57..18b2adf 100644
--- a/bin/varnishd/http2/cache_http2_session.c
+++ b/bin/varnishd/http2/cache_http2_session.c
@@ -192,11 +192,12 @@ h2_b64url_settings(struct h2_sess *h2, struct req *req)
 /**********************************************************************/
 
 static int
-h2_ou_session(const struct worker *wrk, struct h2_sess *h2,
+h2_ou_session(struct worker *wrk, struct h2_sess *h2,
     struct req *req)
 {
 	ssize_t sz;
 	enum htc_status_e hs;
+	struct h2_req *r2;
 
 	if (h2_b64url_settings(h2, req)) {
 		VSLb(h2->vsl, SLT_Debug, "H2: Bad HTTP-Settings");
@@ -220,7 +221,7 @@ h2_ou_session(const struct worker *wrk, struct h2_sess *h2,
 	HTC_RxInit(h2->htc, h2->ws);
 
 	/* Start req thread */
-	(void)h2_new_req(wrk, h2, 1, req);
+	r2 = h2_new_req(wrk, h2, 1, req);
 	req->req_step = R_STP_RECV;
 	req->transport = &H2_transport;
 	req->req_step = R_STP_TRANSPORT;
@@ -233,7 +234,8 @@ h2_ou_session(const struct worker *wrk, struct h2_sess *h2,
 	hs = HTC_RxStuff(h2->htc, H2_prism_complete,
 	    NULL, NULL, NAN, h2->sess->t_idle + cache_param->timeout_idle, 256);
 	if (hs != HTC_S_COMPLETE) {
-		VSLb(h2->vsl, SLT_Debug, "H2: No OU PRISM (hs=%d)", hs);
+		VSLb(h2->vsl, SLT_Debug, "H2: No/Bad OU PRISM (hs=%d)", hs);
+		h2_del_req(wrk, r2);
 		return (0);
 	}
 	XXXAZ(Pool_Task(wrk->pool, &req->task, TASK_QUEUE_REQ));
@@ -285,11 +287,7 @@ h2_new_session(struct worker *wrk, void *arg)
 	h2->req0 = h2_new_req(wrk, h2, 0, NULL);
 
 	if (req->err_code == H2_OU_MARKER && !h2_ou_session(wrk, h2, req)) {
-		CNT_AcctLogCharge(wrk->stats, req);
-		VCL_Rel(&req->vcl);
-		Req_Release(req);
 		h2_del_req(wrk, h2->req0);
-		SES_Delete(h2->sess, SC_RX_JUNK, NAN);
 		return;
 	}
 	assert(HTC_S_COMPLETE == H2_prism_complete(h2->htc));
diff --git a/bin/varnishtest/tests/t02000.vtc b/bin/varnishtest/tests/t02000.vtc
index 2bdfac9..29fe3ba 100644
--- a/bin/varnishtest/tests/t02000.vtc
+++ b/bin/varnishtest/tests/t02000.vtc
@@ -8,9 +8,17 @@ server s1 {
 
 varnish v1 -vcl+backend {} -start
 
-varnish v1 -cliok "param.set feature +http2"
 varnish v1 -cliok "param.set debug +syncvsl"
 
+varnish v1 -cliok "param.set feature -http2"
+
+client c1 {
+	txpri
+	expect_close
+} -run
+
+varnish v1 -cliok "param.set feature +http2"
+
 client c1 {
 	stream 1 {
 		txprio -weight 10 -stream 0
diff --git a/bin/varnishtest/tests/t02001.vtc b/bin/varnishtest/tests/t02001.vtc
index d461f53..a55920a 100644
--- a/bin/varnishtest/tests/t02001.vtc
+++ b/bin/varnishtest/tests/t02001.vtc
@@ -27,16 +27,41 @@ client c1 {
 
 server s1 {
 	rxreq
-	expect req.url == /upgrade
+	expect req.url == /upgrade1
+	expect req.http.host == foo.bar
+	expect req.bodylen == 4
+	txresp -status 401 -bodylen 8
+
+	rxreq
+	expect req.url == /upgrade2
 	expect req.http.host == foo.bar
 	barrier b1 sync
-	txresp -status 401 -bodylen 11
+	txresp -status 402 -bodylen 11
 } -start
 
+delay .5
+
 varnish v1 -cliok "param.set feature +http2"
 
+# We don't support upgrades with body
+
+client c1 {
+	send "POST /upgrade1 HTTP/1.1\r\n"
+	send "Host: foo.bar\r\n"
+	send "Upgrade: h2c\r\n"
+	send "HTTP2-Settings: AAMAAABkAAQAAP__\r\n"
+	send "Content-Length: 4\r\n"
+	send "\r\n"
+	send "FOO\n"
+	rxresp
+	expect resp.status == 401
+	expect resp.bodylen == 8
+} -run
+
+delay .5
+
 client c1 {
-	send "GET /upgrade HTTP/1.1\r\n"
+	send "GET /upgrade2 HTTP/1.1\r\n"
 	send "Host: foo.bar\r\n"
 	send "Upgrade: h2c\r\n"
 	send "HTTP2-Settings: AAMAAABkAAQAAP__\r\n"
@@ -56,7 +81,7 @@ client c1 {
 	barrier b1 sync
 	stream 1 {
 		rxresp
-		expect resp.status == 401
+		expect resp.status == 402
 		expect resp.bodylen == 11
 	} -run
 } -run