[PATCH] xts5: Bail out with UNTESTED for BAD_LENGTH requests that would have length 0

[Peter Hutterer]

On Sun, Oct 17, 2010 at 03:46:31PM -0700, Aaron Plattner wrote:
> The pBell-2 test sets the test type to BAD_LENGTH, which attempts to
> create a request with a size that is too small by one long (4 bytes).
> However, it tries to do this with an xBellReq, which is exactly 4
> bytes in size.  This results in a request with a length field of 0,
> which triggers the big request handling in the X server.  This makes
> the server interpret the next request as the size part of the Bell
> request, and hilarity ensues.
> 
> Instead, detect this case and bail out with an UNTESTED result.
> 
> Signed-off-by: Aaron Plattner <aplattner at nvidia.com>

Acked-by: Peter Hutterer <peter.hutterer at who-t.net>

> ---
> pBell-2 and a lot of tests like it test two things: that you get BadLength
> when the length is too small by one, and that you get BadLength when it's
> too large by one.  The problem is that the -1 case triggers the big request
> processing.  This change masks the second half of the test.  Do you guys
> think it would be a good idea to split all of these tests into separate
> assertions, or should I leave them the way they are?

splitting it up would be the nice thing to do, though I realise this is
effort that may not pay off.

Cheers,
  Peter

> 
> It would be a little weird because pBell-2 and the new pBell-3 would have
> the same assertion text, but would have different methods:
> 
>   520|6 2 00030251 1 3|When a client sends an invalid Bell protocol request to the
>   520|6 2 00030251 1 4|X server, in which the length field of the request is not
>   520|6 2 00030251 1 5|the minimum length required to contain the request, then the
>   520|6 2 00030251 1 6|X server sends back a BadLength error to the client.
> 
>  xts5/src/libproto/MakeReq.c |   12 +++++++++++-
>  1 files changed, 11 insertions(+), 1 deletions(-)
> 
> 
> Here's the summary diff before and after this change:
> 
> >          CASES TESTS  PASS UNSUP UNTST NOTIU  WARN   FIP  FAIL UNRES  UNIN ABORT
> >
> > -Xproto    122   389   354     0     0     0     0     0    18    17     0     0
> > +Xproto    122   389   369     0    19     0     0     0     1     0     0     0
> 
> diff --git a/xts5/src/libproto/MakeReq.c b/xts5/src/libproto/MakeReq.c
> index df63ac6..90ed8c1 100644
> --- a/xts5/src/libproto/MakeReq.c
> +++ b/xts5/src/libproto/MakeReq.c
> @@ -1279,7 +1279,17 @@ int type;
>  	case BAD_LENGTH:
>  	case JUST_TOO_LONG:
>  	case TOO_LONG:
> -		rp->length = bad_len;
> +		if (bad_len == 0 && dpy->bigreq_size > 0) {
> +		    Log_Msg("This test sends an invalid request with size zero\n");
> +		    Log_Msg("  which would be interpreted as a Big Request by this\n");
> +		    Log_Msg("  server, so this test cannot be performed.\n");
> +		    Free_Req(rp);
> +		    rp = NULL;
> +		    Destroy_Client(client);
> +		    Untested();
> +		} else {
> +		    rp->length = bad_len;
> +		}
>  		break;
>  	default:
>  		Log_Msg ("INTERNAL ERROR: Make_Req - bad test type %d\n", Get_Test_Type(client));
> -- 
> 1.7.0.4