[PATCH v2] os: Return BadLength instead of disconnecting BigReq clients (#4565)

[Aaron Plattner]

If a client sends a big request that's too big (i.e. bigger than
maxBigRequestSize << 2 bytes), the server just disconnects it.  This makes the
client receive SIGPIPE the next time it tries to send something.

The X Test Suite sends requests that are too big when the test specifies the
TOO_LONG test type.  When the client receives SIGPIPE, XTS marks it as
UNRESOLVED, which counts as a failure.

Instead, remember how long the request is supposed to be and then return that
size.  Dispatch() checks the length and sends BadLength to the client.  Then,
whenever oci->ignoreBytes is nonzero, ignore the data read instead of trying to
process it as a request.

Signed-off-by: Aaron Plattner <aplattner at nvidia.com>
---
Good point, fixed.

 os/io.c    |   27 ++++++++++++++++++++++++---
 os/osdep.h |    1 +
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/os/io.c b/os/io.c
index e2df2e3..52ef3bf 100644
--- a/os/io.c
+++ b/os/io.c
@@ -251,7 +251,14 @@ ReadRequestFromClient(ClientPtr client)
     need_header = FALSE;
     move_header = FALSE;
     gotnow = oci->bufcnt + oci->buffer - oci->bufptr;
-    if (gotnow < sizeof(xReq))
+
+    if (oci->ignoreBytes > 0) {
+	if (oci->ignoreBytes > oci->size)
+	    needed = oci->size;
+	else
+	    needed = oci->ignoreBytes;
+    }
+    else if (gotnow < sizeof(xReq))
     {
 	/* We don't have an entire xReq yet.  Can't tell how big
 	 * the request will be until we get the whole xReq.
@@ -294,8 +301,13 @@ ReadRequestFromClient(ClientPtr client)
 	if (needed > maxBigRequestSize << 2)
 	{
 	    /* request is too big for us to handle */
-	    YieldControlDeath();
-	    return -1;
+	    /*
+	     * Mark the rest of it as needing to be ignored, and then return
+	     * the full size.  Dispatch() will turn it into a BadLength error.
+	     */
+	    oci->ignoreBytes = needed - gotnow;
+	    oci->lenLastReq = gotnow;
+	    return needed;
 	}
 	if ((gotnow == 0) ||
 	    ((oci->bufptr - oci->buffer + needed) > oci->size))
@@ -400,6 +412,14 @@ ReadRequestFromClient(ClientPtr client)
     }
     oci->lenLastReq = needed;
 
+    /* If there are bytes to ignore, ignore them now. */
+
+    if (oci->ignoreBytes > 0) {
+	assert(needed == oci->ignoreBytes || needed == oci->size);
+	oci->ignoreBytes -= gotnow;
+	needed = gotnow = 0;
+    }
+
     /*
      *  Check to see if client has at least one whole request in the
      *  buffer beyond the request we're returning to the caller.
@@ -1030,6 +1050,7 @@ AllocateInputBuffer(void)
     oci->bufptr = oci->buffer;
     oci->bufcnt = 0;
     oci->lenLastReq = 0;
+    oci->ignoreBytes = 0;
     return oci;
 }
 
diff --git a/os/osdep.h b/os/osdep.h
index 1d87592..3c0e78f 100644
--- a/os/osdep.h
+++ b/os/osdep.h
@@ -125,6 +125,7 @@ typedef struct _connectionInput {
     int  bufcnt;                /* count of bytes in buffer */
     int lenLastReq;
     int size;
+    unsigned int ignoreBytes;   /* bytes to ignore before the next request */
 } ConnectionInput, *ConnectionInputPtr;
 
 typedef struct _connectionOutput {
-- 
1.7.0.4