summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authordormando <dormando@rydia.net>2023-02-14 17:57:09 -0800
committerdormando <dormando@rydia.net>2023-02-14 17:57:09 -0800
commit58f81a51772163d641bc91b9af07756acad98bce (patch)
tree42a28c15b3223e4dca351186fd88eb252fa7c44a
parent3722d0d12be07ede1a8c3081c2e8e0a4c2b477b5 (diff)
downloadmemcached-58f81a51772163d641bc91b9af07756acad98bce.tar.gz
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.
-rw-r--r--proto_proxy.c6
-rw-r--r--t/proxyunits.lua1
-rw-r--r--t/proxyunits.t28
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
{