BUG/MINOR: http-ana: restore the transaction status after early hints

While a 103 response is being built by consecutive early-hint rules, the
transaction status is set to 103 so that the next rules append their headers
to the same response. Since commit 4c3d3d2a6 ("BUG/MINOR: http-act: Properly
generate 103 responses when several rules are used"), once the response was
finished, or on error, the status was reset to 0.

But the initial status of a transaction is -1, and some code relies on it to
know whether a status was set. This is the case of the deny handling in
http_req_get_intercept_rule(), which only uses the default 403 status if the
status is still -1. So a Lua action returning act.DENY after an early-hint
rule makes haproxy answered with a 500 instead of a 403, and logged a null
status.

Let's restore -1 instead in http_early_hint_end() and on the error paths of
the early-hint action and of the cache early hints. A Lua reg-test is added.

This must be backported to all supported versions. The cache part only
exists in 3.5.

diff --git a/reg-tests/lua/early_hint_deny.lua b/reg-tests/lua/early_hint_deny.lua
new file mode 100644 (file)
index 0000000..277ae13
--- /dev/null
+++ b/reg-tests/lua/early_hint_deny.lua
@@ -0,0 +1,3 @@
+core.register_action("deny", { "http-req" }, function(txn)
+    return act.DENY
+end)

diff --git a/reg-tests/lua/early_hint_deny.vtc b/reg-tests/lua/early_hint_deny.vtc
new file mode 100644 (file)
index 0000000..40fd62b
--- /dev/null
+++ b/reg-tests/lua/early_hint_deny.vtc
@@ -0,0 +1,52 @@
+varnishtest "Lua: deny action evaluated after an early-hint rule"
+feature cmd "$HAPROXY_PROGRAM -cc 'feature(LUA)'"
+feature ignore_unknown_macro
+
+# A Lua action returning act.DENY does not set any status, the deny is then
+# answered with the default 403. The status of the transaction is used while
+# a 103 response is being built by early-hint rules, and it must be restored
+# afterwards, otherwise the deny is answered with a 500 and logged with a
+# null status.
+
+syslog Slg1 -level info {
+    recv
+    expect ~ "[^:\\[ ]\\[${h1_pid}\\]: .* fe fe/<NOSRV> .* 403 .* \"GET /plain HTTP/1.1\""
+    recv
+    expect ~ "[^:\\[ ]\\[${h1_pid}\\]: .* fe fe/<NOSRV> .* 403 .* \"GET /eh HTTP/1.1\""
+} -start
+
+haproxy h1 -conf {
+    global
+        tune.lua.bool-sample-conversion normal
+        lua-load ${testdir}/early_hint_deny.lua
+
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    frontend fe
+        bind "fd@${fe}"
+        log ${Slg1_addr}:${Slg1_port} local0
+        option httplog
+        http-request early-hint link "</style.css>; rel=preload" if { path /eh }
+        http-request lua.deny
+} -start
+
+client c1 -connect ${h1_fe_sock} {
+    txreq -url "/plain"
+    rxresp
+    expect resp.status == 403
+} -run
+
+client c2 -connect ${h1_fe_sock} {
+    txreq -url "/eh"
+    rxresp
+    expect resp.status == 103
+    expect resp.http.link == "</style.css>; rel=preload"
+    rxresp
+    expect resp.status == 403
+} -run
+
+syslog Slg1 -wait

diff --git a/src/cache.c b/src/cache.c
index 37fff6b0f6f4b09c163f5a6834ba1e0b367a0891..1235ffbd82dbdde3651cb23fa6d5be6da349837f 100644 (file)
--- a/src/cache.c
+++ b/src/cache.c
@@ -2560,7 +2560,7 @@ static int cache_emit_early_hints(struct stream *s, const char *hint_data,
 
   error:
        channel_htx_truncate(&s->res, htxbuf(&s->res.buf));
-       s->txn.http->status = 0;
+       s->txn.http->status = -1;
        return 0;
 }
 

diff --git a/src/http_act.c b/src/http_act.c
index 653c8fc95d45c38f5bc20ed0953c1ca1901e6cc7..640e139627d7e8798f78a568c6435f022e35f7a2 100644 (file)
--- a/src/http_act.c
+++ b/src/http_act.c
@@ -1410,7 +1410,7 @@ static enum act_return http_action_early_hint(struct act_rule *rule, struct prox
         * HTTP 103 response from the buffer */
        channel_htx_truncate(res, htx);
        ret = ACT_RET_ERR;
-       s->txn.http->status = 0;
+       s->txn.http->status = -1;
        goto leave;
 }
 

diff --git a/src/http_ana.c b/src/http_ana.c
index a23cb20f1aa367b405de57af61c7c49fc865480f..c3f5786f6458b4dcf3308c63fd87b867239a03d8 100644 (file)
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -4854,7 +4854,10 @@ int http_early_hint_end(struct stream *s)
                return 0;
        if (!http_forward_proxy_resp(s, 0))
                return 0;
-       s->txn.http->status = 0;
+       /* Restore the initial status of the transaction, so that rules
+        * evaluated after this one may set theirs, e.g. a deny.
+        */
+       s->txn.http->status = -1;
        return 1;
 }