Rewrite .Xauthority parsing using BinarySource.

This rewrite replaces a particularly hairy macro-based system.

x11fwd.c

@@ -429,15 +429,28 @@ static const char *x11_verify(unsigned long peer_ip, int peer_port,
     return NULL;
 }
 
+ptrlen BinarySource_get_string_xauth(BinarySource *src)
+{
+    size_t len = get_uint16(src);
+    return get_data(src, len);
+}
+#define get_string_xauth(src) \
+    BinarySource_get_string_xauth(BinarySource_UPCAST(src))
+
 void x11_get_auth_from_authfile(struct X11Display *disp,
 				const char *authfilename)
 {
     FILE *authfp;
-    char *buf, *ptr, *str[4];
+    char *buf;
-    int len[4];
+    int size;
+    BinarySource src[1];
     int family, protocol;
+    ptrlen addr, protoname, data;
+    char *displaynum_string;
+    int displaynum;
     int ideal_match = FALSE;
     char *ourhostname;
+    const size_t MAX_RECORD_SIZE = 0x80, BUF_SIZE = 2 * MAX_RECORD_SIZE;
 
     /*
      * Normally we should look for precisely the details specified in
@@ -468,29 +481,41 @@ void x11_get_auth_from_authfile(struct X11Display *disp,
 
     ourhostname = get_hostname();
 
-    /* Records in .Xauthority contain four strings of up to 64K each */
+    /*
-    buf = snewn(65537 * 4, char);
+     * Allocate enough space to hold two maximally sized records, so
+     * that a full record can start anywhere in the first half. That
+     * way we avoid the accidentally-quadratic algorithm that would
+     * arise if we moved everything to the front of the buffer after
+     * consuming each record; instead, we only move everything to the
+     * front after our current position gets past the half-way mark.
+     * Before then, there's no need to move anyway; so this guarantees
+     * linear time, in that every byte written into this buffer moves
+     * at most once (because every move is from the second half of the
+     * buffer to the first half).
+     */
+    buf = snewn(BUF_SIZE, char);
+    size = fread(buf, 1, BUF_SIZE, authfp);
+    BinarySource_BARE_INIT(src, buf, size);
 
     while (!ideal_match) {
-	int c, i, j, match = FALSE;
+        int match = FALSE;
-	
+
-#define GET do { c = fgetc(authfp); if (c == EOF) goto done; c = (unsigned char)c; } while (0)
+        if (src->pos >= MAX_RECORD_SIZE) {
-	/* Expect a big-endian 2-byte number giving address family */
+            size -= src->pos;
-	GET; family = c;
+            memcpy(buf, buf + src->pos, size);
-	GET; family = (family << 8) | c;
+            size += fread(buf + size, 1, BUF_SIZE - size, authfp);
-	/* Then expect four strings, each composed of a big-endian 2-byte
+            BinarySource_BARE_INIT(src, buf, size);
-	 * length field followed by that many bytes of data */
+        }
-	ptr = buf;
+
-	for (i = 0; i < 4; i++) {
+        family = get_uint16(src);
-	    GET; len[i] = c;
+        addr = get_string_xauth(src);
-	    GET; len[i] = (len[i] << 8) | c;
+        displaynum_string = mkstr(get_string_xauth(src));
-	    str[i] = ptr;
+        displaynum = atoi(displaynum_string);
-	    for (j = 0; j < len[i]; j++) {
+        sfree(displaynum_string);
-		GET; *ptr++ = c;
+        protoname = get_string_xauth(src);
-	    }
+        data = get_string_xauth(src);
-	    *ptr++ = '\0';
+        if (get_err(src))
-	}
+            break;
-#undef GET
 
 	/*
 	 * Now we have a full X authority record in memory. See
@@ -504,7 +529,7 @@ void x11_get_auth_from_authfile(struct X11Display *disp,
 	 *    connect to the display. 0 means IPv4; 6 means IPv6;
 	 *    256 means Unix-domain sockets.
 	 * 
-	 *  - str[0] is the network address itself. For IPv4 and
+	 *  - 'addr' is the network address itself. For IPv4 and
 	 *    IPv6, this is a string of binary data of the
 	 *    appropriate length (respectively 4 and 16 bytes)
 	 *    representing the address in big-endian format, e.g.
@@ -515,24 +540,23 @@ void x11_get_auth_from_authfile(struct X11Display *disp,
 	 *    authority entries for Unix-domain displays on
 	 *    several machines without them clashing).
 	 * 
-	 *  - str[1] is the display number. I've no idea why
+	 *  - 'displaynum' is the display number. I've no idea why
 	 *    .Xauthority stores this as a string when it has a
 	 *    perfectly good integer format, but there we go.
 	 * 
-	 *  - str[2] is the authorisation method, encoded as its
+	 *  - 'protoname' is the authorisation protocol, encoded as
-	 *    canonical string name (i.e. "MIT-MAGIC-COOKIE-1",
+	 *    its canonical string name (i.e. "MIT-MAGIC-COOKIE-1",
-	 *    "XDM-AUTHORIZATION-1" or something we don't
+	 *    "XDM-AUTHORIZATION-1" or something we don't recognise).
-	 *    recognise).
 	 * 
-	 *  - str[3] is the actual authorisation data, stored in
+	 *  - 'data' is the actual authorisation data, stored in
 	 *    binary form.
 	 */
 
-	if (disp->displaynum < 0 || disp->displaynum != atoi(str[1]))
+	if (disp->displaynum < 0 || disp->displaynum != displaynum)
 	    continue;		       /* not the one */
 
 	for (protocol = 1; protocol < lenof(x11_authnames); protocol++)
-	    if (!strcmp(str[2], x11_authnames[protocol]))
+	    if (ptrlen_eq_string(protoname, x11_authnames[protocol]))
 		break;
 	if (protocol == lenof(x11_authnames))
 	    continue;  /* don't recognise this protocol, look for another */
@@ -543,7 +567,7 @@ void x11_get_auth_from_authfile(struct X11Display *disp,
 		sk_addrtype(disp->addr) == ADDRTYPE_IPV4) {
 		char buf[4];
 		sk_addrcopy(disp->addr, buf);
-		if (len[0] == 4 && !memcmp(str[0], buf, 4)) {
+		if (addr.len == 4 && !memcmp(addr.ptr, buf, 4)) {
 		    match = TRUE;
 		    /* If this is a "localhost" entry, note it down
 		     * but carry on looking for a Unix-domain entry. */
@@ -556,7 +580,7 @@ void x11_get_auth_from_authfile(struct X11Display *disp,
 		sk_addrtype(disp->addr) == ADDRTYPE_IPV6) {
 		char buf[16];
 		sk_addrcopy(disp->addr, buf);
-		if (len[0] == 16 && !memcmp(str[0], buf, 16)) {
+		if (addr.len == 16 && !memcmp(addr.ptr, buf, 16)) {
 		    match = TRUE;
 		    ideal_match = !localhost;
 		}
@@ -564,7 +588,7 @@ void x11_get_auth_from_authfile(struct X11Display *disp,
 	    break;
 	  case 256: /* Unix-domain / localhost */
 	    if ((disp->unixdomain || localhost)
-	       	&& ourhostname && !strcmp(ourhostname, str[0]))
+                && ourhostname && ptrlen_eq_string(addr, ourhostname))
 		/* A matching Unix-domain socket is always the best
 		 * match. */
 		match = ideal_match = TRUE;
@@ -575,15 +599,14 @@ void x11_get_auth_from_authfile(struct X11Display *disp,
 	    /* Current best guess -- may be overridden if !ideal_match */
 	    disp->localauthproto = protocol;
 	    sfree(disp->localauthdata); /* free previous guess, if any */
-	    disp->localauthdata = snewn(len[3], unsigned char);
+	    disp->localauthdata = snewn(data.len, unsigned char);
-	    memcpy(disp->localauthdata, str[3], len[3]);
+	    memcpy(disp->localauthdata, data.ptr, data.len);
-	    disp->localauthdatalen = len[3];
+	    disp->localauthdatalen = data.len;
 	}
     }
 
-    done:
     fclose(authfp);
-    smemclr(buf, 65537 * 4);
+    smemclr(buf, 2 * MAX_RECORD_SIZE);
     sfree(buf);
     sfree(ourhostname);
 }