[master] ef420233e Tweak test cases to work with or disable accept_filter

Tue Dec 31 10:05:10 UTC 2019

commit ef420233e61c38e8f446a48a6da8d6aa7d8b5eef
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Tue Dec 31 10:45:50 2019 +0100

    Tweak test cases to work with or disable accept_filter
    
    The test cases disabling the accept_filter parameter are those involving
    GET requests with a body that are meant to be cached, and one test case
    covering OOB data. The rest uses the POST method since the httpready
    filter will only let syntactically correct GET or HEAD methods pass to
    the application.
    
    Apparently FreeBSD's httpready filter considers that a GET request with
    a body is syntactically incorrect although this is not specifically said
    in the manual. The HTTP specification doesn't forbid such requests and
    they are hard to justify considering the semantics of GET and HEAD, but
    there are products in the wild relying on that.
    
    On the other hand GET\r\n\r\n isn't considered malformed, see r01881.vtc.
    
    We don't need the dataready filter on FreeBSD for listen addresses that
    expect a PROXY protocol header, the httpready filter will effectively
    work like that if the request doesn't look like a GET or a HEAD.

diff --git a/bin/varnishtest/tests/b00037.vtc b/bin/varnishtest/tests/b00037.vtc
index 7112cff95..1b7c31235 100644
--- a/bin/varnishtest/tests/b00037.vtc
+++ b/bin/varnishtest/tests/b00037.vtc
@@ -1,12 +1,6 @@
 varnishtest "Error on multiple Host headers"
 
-server s1 {
-	rxreq
-	txresp
-} -start
-
-varnish v1 -vcl+backend {
-} -start
+varnish v1 -vcl {backend be none;} -start
 
 client c1 {
 	txreq -hdr "Host: foo" -hdr "Host: bar"
@@ -17,7 +11,7 @@ client c1 {
 varnish v1 -expect client_req_400 == 1
 
 client c1 {
-	txreq -hdr "Content-Length: 12" -bodylen 12
+	txreq -method POST -hdr "Content-Length: 12" -bodylen 12
 	rxresp
 	expect resp.status == 400
 } -run
diff --git a/bin/varnishtest/tests/b00046.vtc b/bin/varnishtest/tests/b00046.vtc
index 2c60f6630..abf5663d5 100644
--- a/bin/varnishtest/tests/b00046.vtc
+++ b/bin/varnishtest/tests/b00046.vtc
@@ -12,6 +12,7 @@ server s1 {
 	send_urgent " "
 } -start
 
+varnish v1 -cliok "param.set accept_filter off"
 varnish v1 -vcl+backend {} -start
 
 client c1 {
diff --git a/bin/varnishtest/tests/b00069.vtc b/bin/varnishtest/tests/b00069.vtc
index 2167c9483..9d4145dc9 100644
--- a/bin/varnishtest/tests/b00069.vtc
+++ b/bin/varnishtest/tests/b00069.vtc
@@ -2,20 +2,20 @@ varnishtest "HTTP/1 parsing checks"
 
 # Some tricky requests that have been known to cause parsing errors in the past.
 
-server s1 {
+server s1 -repeat 3 {
 	rxreq
 	txresp
 } -start
 
-varnish v1 -vcl+backend {
-} -start
+varnish v1 -vcl+backend "" -start
 
 # This test checks a bug that was dependent on the contents of the buffer left behind
 # by the previous request
 client c1 {
-	send "GET / HTTP/1.1\r\nHost: asdf.com\r\nFoo: baar\r\n\r\n\r\n\r\n\r\n"
+	send "POST / HTTP/1.1\r\nHost: asdf.com\r\nFoo: baar\r\n\r\n\r\n\r\n\r\n"
 	rxresp
-	send "GET / HTTP/1.1\r\nHost: asdf.com\r\nAsdf: b\n \r\n\r\nSj\r"
+	expect resp.status == 200
+	send "POST / HTTP/1.1\r\nHost: asdf.com\r\nAsdf: b\n \r\n\r\nSj\r"
 	rxresp
 	expect resp.status == 200
 } -run
@@ -23,7 +23,7 @@ client c1 {
 # This tests that the line continuation handling doesn't clear out the end of headers
 # [CR]LF
 client c1 {
-	send "GET / HTTP/1.1\r\nHost: asdf.com\r\nAsdf: b\n \r\n\r\nSj"
+	send "POST / HTTP/1.1\r\nHost: asdf.com\r\nAsdf: b\n \r\n\r\nSj"
 	rxresp
 	expect resp.status == 200
 } -run
diff --git a/bin/varnishtest/tests/l00002.vtc b/bin/varnishtest/tests/l00002.vtc
index 82296bc66..7d6a03cf7 100644
--- a/bin/varnishtest/tests/l00002.vtc
+++ b/bin/varnishtest/tests/l00002.vtc
@@ -23,11 +23,11 @@ varnish v1 -vcl+backend {
 } -start
 
 # Request (1001):
-# GET /1 HTTP/1.1\r\n		 17 bytes
+# POST /1 HTTP/1.1\r\n		 18 bytes
 # Host: foo\r\n			 11 bytes
 # Content-Length: 4\r\n		 19 bytes
 # \r\n				  2 bytes
-# Total:			 49 bytes
+# Total:			 50 bytes
 # Response:
 # HTTP/1.1 200 OK\r\n		 17 bytes
 # Content-Length: 1000\r\n	 22 bytes
@@ -72,7 +72,7 @@ varnish v1 -vcl+backend {
 # Total:			 28 bytes
 logexpect l1 -v v1 -g session {
 	expect * 1001	Begin	"^req .* rxreq"
-	expect * =	ReqAcct	"^49 4 53 87 1000 1087$"
+	expect * =	ReqAcct	"^50 4 54 87 1000 1087$"
 	expect 0 =	End
 	expect * 1003	Begin	"^req .* rxreq"
 	expect * =	ReqAcct "^30 0 30 87 2000 2087$"
@@ -87,7 +87,7 @@ logexpect l1 -v v1 -g session {
 
 # Request 1001
 client c1 {
-	txreq -url "/1" -hdr "Host: foo" -body "asdf"
+	txreq -method POST -url "/1" -hdr "Host: foo" -body "asdf"
 	rxresp
 	expect resp.status == 200
 
@@ -104,7 +104,7 @@ client c1 {
 
 logexpect l1 -wait
 
-varnish v1 -expect s_req_hdrbytes == 116
+varnish v1 -expect s_req_hdrbytes == 117
 varnish v1 -expect s_req_bodybytes == 4
 varnish v1 -expect s_resp_hdrbytes == 289
 varnish v1 -expect s_resp_bodybytes == 5000
diff --git a/bin/varnishtest/tests/r01881.vtc b/bin/varnishtest/tests/r01881.vtc
index 5c3ab4dde..7a4977bbb 100644
--- a/bin/varnishtest/tests/r01881.vtc
+++ b/bin/varnishtest/tests/r01881.vtc
@@ -17,7 +17,7 @@ varnish v1 -vcl+backend {
 } -start
 
 client c1 {
-	txreq -body "foobar"
+	txreq -method POST -body "foobar"
 	rxresp
 	expect resp.status == 200
 } -run
diff --git a/bin/varnishtest/tests/r01914.vtc b/bin/varnishtest/tests/r01914.vtc
index 2414daddb..753051893 100644
--- a/bin/varnishtest/tests/r01914.vtc
+++ b/bin/varnishtest/tests/r01914.vtc
@@ -13,6 +13,7 @@ varnish v1 \
     -arg "-s default,1MB" \
     -arg "-s default,1MB" \
     -arg "-s Transient=default" \
+    -arg "-p accept_filter=off" \
     -syntax 4.0 \
     -vcl+backend {
 	import std;
diff --git a/bin/varnishtest/tests/r02105.vtc b/bin/varnishtest/tests/r02105.vtc
index 5834c1197..4dd2c9a28 100644
--- a/bin/varnishtest/tests/r02105.vtc
+++ b/bin/varnishtest/tests/r02105.vtc
@@ -9,6 +9,7 @@ server s1 {
 	txresp
 } -start
 
+varnish v1 -cliok "param.set accept_filter off"
 varnish v1 -vcl+backend {
 	sub vcl_backend_response {
 		set beresp.ttl = 0.5s;
diff --git a/bin/varnishtest/tests/r02266.vtc b/bin/varnishtest/tests/r02266.vtc
index 5fed4d676..c0f4c9d18 100644
--- a/bin/varnishtest/tests/r02266.vtc
+++ b/bin/varnishtest/tests/r02266.vtc
@@ -16,6 +16,7 @@ server s1 {
 	send "line2\n"
 } -start
 
+varnish v1 -cliok "param.set accept_filter off"
 varnish v1 -vcl+backend {
 	sub vcl_backend_response {
 		set beresp.do_stream = false;
diff --git a/include/tbl/params.h b/include/tbl/params.h
index 61fa9a03a..c2742057d 100644
--- a/include/tbl/params.h
+++ b/include/tbl/params.h
@@ -49,7 +49,11 @@ PARAM(
 	/* flags */	XYZZY,
 	/* s-text */
 	"Enable kernel accept-filters. This may require a kernel module to "
-	"be loaded to have an effect when enabled.",
+	"be loaded to have an effect when enabled.\n\n"
+	"Enabling accept_filter may prevent some requests to reach Varnish "
+	"in the first place. Malformed requests may go unnoticed and not "
+	"increase the client_req_400 counter. GET or HEAD requests with a "
+	"body may be blocked altogether.",
 	/* l-text */	NULL,
 	/* func */	NULL
 )
