From 58f81a51772163d641bc91b9af07756acad98bce Mon Sep 17 00:00:00 2001
From: dormando <dormando@rydia.net>
Date: Tue, 14 Feb 2023 17:57:09 -0800
Subject: proxy: fix partial responses on backend timeouts

Response object error conditions were not being checked before looking
at the response buffer. If a response was partially filled then the
backend timed out, a partial response could be sent intead of the
proper backend error.
---
 proto_proxy.c    |  6 +++---
 t/proxyunits.lua |  1 +
 t/proxyunits.t   | 28 ++++++++++++++++++++++------
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/proto_proxy.c b/proto_proxy.c
index feeddc9..535800d 100644
--- a/proto_proxy.c
+++ b/proto_proxy.c
@@ -548,7 +548,9 @@ int proxy_run_coroutine(lua_State *Lc, mc_resp *resp, io_pending_proxy_t *p, con
         if (type == LUA_TUSERDATA) {
             mcp_resp_t *r = luaL_checkudata(Lc, 1, "mcp.response");
             _set_noreply_mode(resp, r);
-            if (r->buf) {
+            if (r->status != MCMC_OK) {
+                proxy_out_errstring(resp, "backend failure");
+            } else if (r->buf) {
                 // response set from C.
                 // FIXME (v2): write_and_free() ? it's a bit wrong for here.
                 resp->write_and_free = r->buf;
@@ -562,8 +564,6 @@ int proxy_run_coroutine(lua_State *Lc, mc_resp *resp, io_pending_proxy_t *p, con
                 memcpy(resp->wbuf, s, l);
                 resp_add_iov(resp, resp->wbuf, l);
                 lua_pop(Lc, 1);
-            } else if (r->status != MCMC_OK) {
-                proxy_out_errstring(resp, "backend failure");
             } else {
                 // Empty response: used for ascii multiget emulation.
             }
diff --git a/t/proxyunits.lua b/t/proxyunits.lua
index 411950d..69878f4 100644
--- a/t/proxyunits.lua
+++ b/t/proxyunits.lua
@@ -1,4 +1,5 @@
 mcp.backend_read_timeout(0.5)
+mcp.backend_connect_timeout(5)
 
 function mcp_config_pools(oldss)
     local srv = mcp.backend
diff --git a/t/proxyunits.t b/t/proxyunits.t
index da3447f..4c487d1 100644
--- a/t/proxyunits.t
+++ b/t/proxyunits.t
@@ -37,7 +37,7 @@ sub check_version {
 }
 
 my @mocksrvs = ();
-diag "making mock servers";
+#diag "making mock servers";
 for my $port (11411, 11412, 11413) {
     my $srv = mock_server($port);
     ok(defined $srv, "mock server created");
@@ -50,7 +50,7 @@ $ps->autoflush(1);
 
 # set up server backend sockets.
 my @mbe = ();
-diag "accepting mock backends";
+#diag "accepting mock backends";
 for my $msrv (@mocksrvs) {
     my $be = $msrv->accept();
     $be->autoflush(1);
@@ -58,13 +58,31 @@ for my $msrv (@mocksrvs) {
     push(@mbe, $be);
 }
 
-diag "validating backends";
+#diag "validating backends";
 for my $be (@mbe) {
     like(<$be>, qr/version/, "received version command");
     print $be "VERSION 1.0.0-mock\r\n";
 }
 
-diag "ready for main tests";
+{
+    # Test a fix for passing through partial read data if END ends up missing.
+    print $ps "get /b/a\r\n";
+    my $be = $mbe[0];
+
+    is(scalar <$be>, "get /b/a\r\n", "get passthrough");
+    print $be "VALUE /b/a 0 2\r\nhi\r\nEN";
+
+    is(scalar <$ps>, "SERVER_ERROR backend failure\r\n", "backend failure error");
+
+    # re-accept the backend.
+    $be = $mocksrvs[0]->accept();
+    $be->autoflush(1);
+    like(<$be>, qr/version/, "received version command");
+    print $be "VERSION 1.0.0-mock\r\n";
+    $mbe[0] = $be;
+}
+
+#diag "ready for main tests";
 # Target a single backend, validating basic syntax.
 # Should test all command types.
 # uses /b/ path for "basic"
@@ -295,8 +313,6 @@ check_version($ps);
 }
 
 check_version($ps);
-# TODO: Test specifically responding to a get but missing the END\r\n. it
-# should time out and not leak to the client.
 
 # Test Lua request API
 {
-- 
cgit v1.2.1