summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authordormando <dormando@rydia.net>2023-02-16 22:49:52 -0800
committerdormando <dormando@rydia.net>2023-02-17 11:06:27 -0800
commitaf037c38cda6d9a07e82d0386202aa3c9d08faea (patch)
treee64bc3d5437d24002f11ad407923b3028a10e9e5
parent2623ed4af53e8e8190d14c6215d9b3c39144e58d (diff)
downloadmemcached-af037c38cda6d9a07e82d0386202aa3c9d08faea.tar.gz
proxy: fix "missingend" error on reading responses
If the backend handler reads an incomplete response from the network, it changes state to wait for more data. The want_read state was considering the data completed if "data read" was bigger than "value length", but it should have been "value + result line". This means if the response buffer landed in a bullseye where it has read more than the size of the value but less than the total size of the request (typically a span of 200 bytes or less), it would consider the request complete and look for the END\r\n marker. This change has been... here forever.
-rw-r--r--proxy_network.c2
-rw-r--r--t/proxyunits.t55
2 files changed, 56 insertions, 1 deletions
diff --git a/proxy_network.c b/proxy_network.c
index 71c1a0c..3983ec5 100644
--- a/proxy_network.c
+++ b/proxy_network.c
@@ -1080,7 +1080,7 @@ static int proxy_backend_drive_machine(mcp_backend_t *be) {
memcpy(r->buf+r->bread, be->rbuf, tocopy);
r->bread += tocopy;
- if (r->bread >= r->resp.vlen) {
+ if (r->bread >= r->blen) {
// all done copying data.
if (r->resp.type == MCMC_RESP_GET) {
be->state = mcp_backend_read_end;
diff --git a/t/proxyunits.t b/t/proxyunits.t
index 0a7476f..5d46d1c 100644
--- a/t/proxyunits.t
+++ b/t/proxyunits.t
@@ -108,6 +108,61 @@ for my $be (@mbe) {
$mbe[0] = $be;
}
+SKIP: {
+ skip "Remove this skip line to demonstrate pre-patch bug", 1;
+ # Test issue with finding response complete when read lands between value
+ # size and value + response line in size.
+ my $be = $mbe[0];
+ my $w = $p_srv->new_sock;
+ print $w "watch proxyevents\n";
+ is(<$w>, "OK\r\n", "watcher enabled");
+
+ print $ps "get /b/c\r\n";
+ is(scalar <$be>, "get /b/c\r\n", "get passthrough");
+
+ # Set off a "missingend" error.
+ # The server will wake up several times, thinking it has read the
+ # full size of response but it only read enough for the value portion.
+ print $be "VALUE /b/c 0 5\r\nhe";
+ sleep 0.1;
+ print $be "llo";
+ sleep 0.1;
+ print $be "\r\nEND\r\n";
+
+ is(scalar <$ps>, "SERVER_ERROR backend failure\r\n");
+
+ like(<$w>, qr/ts=(\S+) gid=\d+ type=proxy_backend error=missingend name=127.0.0.1 port=\d+ depth=1 rbuf=/, "got missingend error log line");
+
+ # 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;
+}
+
+{
+ # Test issue with finding response complete when read lands between value
+ # size and value + response line in size.
+ my $be = $mbe[0];
+
+ print $ps "get /b/c\r\n";
+ is(scalar <$be>, "get /b/c\r\n", "get passthrough");
+
+ # Set off a "missingend" error.
+ # The server will wake up several times, thinking it has read the
+ # full size of response but it only read enough for the value portion.
+ print $be "VALUE /b/c 0 5\r\nhe";
+ sleep 0.1;
+ print $be "llo";
+ sleep 0.1;
+ print $be "\r\nEND\r\n";
+
+ is(scalar <$ps>, "VALUE /b/c 0 5\r\n", "got value back");
+ is(scalar <$ps>, "hello\r\n", "got data back");
+ is(scalar <$ps>, "END\r\n", "got end string");
+}
+
#diag "ready for main tests";
# Target a single backend, validating basic syntax.
# Should test all command types.