---------------------
PatchSet 11270 
Date: 2007/02/27 00:28:22
Author: hno
Branch: HEAD
Tag: (none) 
Log:
Bug #1906: Off-by-one error in httpMsgParseRequestLine

Clean up httpMsgParseRequestLine somewhat, getting rid of an off-by-one error
when unneededly trying to look for \r in \r\n. Also indicate parse errors
and deal with them in the (single) caller.

Members: 
	src/HttpMsg.c:1.13->1.14 
	src/client_side.c:1.706->1.707 

Index: squid/src/HttpMsg.c
===================================================================
RCS file: /cvsroot/squid/squid/src/HttpMsg.c,v
retrieving revision 1.13
retrieving revision 1.14
diff -u -r1.13 -r1.14
--- squid/src/HttpMsg.c	21 Jan 2007 12:53:56 -0000	1.13
+++ squid/src/HttpMsg.c	27 Feb 2007 00:28:22 -0000	1.14
@@ -1,6 +1,6 @@
 
 /*
- * $Id: HttpMsg.c,v 1.13 2007/01/21 12:53:56 adrian Exp $
+ * $Id: HttpMsg.c,v 1.14 2007/02/27 00:28:22 hno Exp $
  *
  * DEBUG: section 74    HTTP Message
  * AUTHOR: Alex Rousskov
@@ -164,26 +164,19 @@
 httpMsgParseRequestLine(HttpMsgBuf * hmsg)
 {
     int i = 0;
-    int retcode = 0;
+    int retcode;
     int maj = -1, min = -1;
     int last_whitespace = -1, line_end = -1;
+    const char *t;
 
     /* Find \r\n - end of URL+Version (and the request) */
-    for (i = 0; i < hmsg->size; i++) {
-	if (hmsg->buf[i] == '\n') {
-	    break;
-	}
-	if (i < hmsg->size - 1 && hmsg->buf[i - 1] == '\r' && hmsg->buf[i] == '\n') {
-	    i++;
-	    break;
-	}
-    }
-    if (i == hmsg->size) {
+    t = memchr(hmsg->buf, '\n', hmsg->size);
+    if (!t) {
 	retcode = 0;
 	goto finish;
     }
     /* XXX this should point to the -end- of the \r\n, \n, etc. */
-    hmsg->req_end = i;
+    hmsg->req_end = t - hmsg->buf;
     i = 0;
 
     /* Find first non-whitespace - beginning of method */
@@ -199,7 +192,7 @@
     /* Find first whitespace - end of method */
     for (; i < hmsg->req_end && (!isspace(hmsg->buf[i])); i++);
     if (i >= hmsg->req_end) {
-	retcode = 0;
+	retcode = -1;
 	goto finish;
     }
     hmsg->m_end = i - 1;
@@ -208,7 +201,7 @@
     /* Find first non-whitespace - beginning of URL+Version */
     for (; i < hmsg->req_end && (isspace(hmsg->buf[i])); i++);
     if (i >= hmsg->req_end) {
-	retcode = 0;
+	retcode = -1;
 	goto finish;
     }
     hmsg->u_start = i;
@@ -220,8 +213,8 @@
 	    line_end = i;
 	    break;
 	}
-	/* XXX could be off-by-one wrong! */
-	if (hmsg->buf[i] == '\r' && (i + 1) <= hmsg->req_end && hmsg->buf[i + 1] == '\n') {
+	/* we know for sure that there is at least a \n following.. */
+	if (hmsg->buf[i] == '\r' && hmsg->buf[i + 1] == '\n') {
 	    line_end = i;
 	    break;
 	}
@@ -231,7 +224,7 @@
 	}
     }
     if (i > hmsg->req_end) {
-	retcode = 0;
+	retcode = -1;
 	goto finish;
     }
     /* At this point we don't need the 'i' value; so we'll recycle it for version parsing */
@@ -251,7 +244,7 @@
 	/* XXX why <= vs < ? I do need to really re-audit all of this .. */
 	for (i = last_whitespace; i <= hmsg->req_end && isspace(hmsg->buf[i]); i++);
 	if (i > hmsg->req_end) {
-	    retcode = 0;
+	    retcode = -1;
 	    goto finish;
 	}
 	/* is it http/ ? if so, we try parsing. If not, the URL is the whole line; version is 0.9 */
@@ -272,7 +265,7 @@
 		maj = maj + (hmsg->buf[i]) - '0';
 	    }
 	    if (i >= hmsg->req_end) {
-		retcode = 0;
+		retcode = -1;
 		goto finish;
 	    }
 	    /* next should be .; we -have- to have this as we have a whole line.. */
@@ -281,7 +274,7 @@
 		goto finish;
 	    }
 	    if (i + 1 >= hmsg->req_end) {
-		retcode = 0;
+		retcode = -1;
 		goto finish;
 	    }
 	    /* next should be one or more digits */
Index: squid/src/client_side.c
===================================================================
RCS file: /cvsroot/squid/squid/src/client_side.c,v
retrieving revision 1.706
retrieving revision 1.707
diff -u -r1.706 -r1.707
--- squid/src/client_side.c	26 Feb 2007 09:11:10 -0000	1.706
+++ squid/src/client_side.c	27 Feb 2007 00:28:22 -0000	1.707
@@ -1,6 +1,6 @@
 
 /*
- * $Id: client_side.c,v 1.706 2007/02/26 09:11:10 hno Exp $
+ * $Id: client_side.c,v 1.707 2007/02/27 00:28:22 hno Exp $
  *
  * DEBUG: section 33    Client-side Routines
  * AUTHOR: Duane Wessels
@@ -3510,6 +3510,8 @@
 
     /* Parse the request line */
     ret = httpMsgParseRequestLine(hmsg);
+    if (ret == -1)
+	return parseHttpRequestAbort(conn, "error:invalid-request");
     if (ret == 0) {
 	debug(33, 5) ("Incomplete request, waiting for end of request line\n");
 	*status = 0;