httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Roy T. Fielding" <field...@liege.ICS.UCI.EDU>
Subject getline bug fixes, patch
Date Sat, 09 Nov 1996 08:35:23 GMT
I discovered a nasty little bug in the header parsing code when I
was testing IMS -- if the continuation line was a single space, the
entire header field above it was ignored and the message terminated.
This would normally only occur when cut-n-pasting from an xterm, but
it revealed a bunch of other problems with the way 1.2 was looking
for the request-line and header fields.  I fixed it by rewriting
the routines so that they would be more natural and not discard the
error conditions, and introduced a new routine to buff.* for looking
at the top of the input stream without changing it.

The good news is that we now handle input lines robustly and do not
puke when an extra CRLF is placed at the end of a POST request (the 
actual cause of a recent bug report).  The bad news is that I have
found several places where we should be responding with 400 or 414
(Request-URI Too Long) messages when our input buffer fills-up, but
we have no apparent mechanism for die'ing inside http_protocol.c.
For now I am just letting the input be truncated, which is good enough
to let people use these patches.

I have a feeling that this may also fix a number of other problems where
it seems like Apache and/or Netscape is hanging on a completed request,
but that is just a hunch.

......Roy

Index: buff.h
===================================================================
RCS file: /export/home/cvs/apache/src/buff.h,v
retrieving revision 1.7
diff -c -r1.7 buff.h
*** buff.h	1996/11/03 21:25:04	1.7
--- buff.h	1996/11/09 08:13:18
***************
*** 112,117 ****
--- 112,118 ----
  /* I/O */
  extern int bread(BUFF *fb, void *buf, int nbyte);
  extern int bgets(char *s, int n, BUFF *fb);
+ extern int blookc(char *buff, BUFF *fb);
  extern int bskiplf(BUFF *fb);
  extern int bwrite(BUFF *fb, const void *buf, int nbyte);
  extern int bflush(BUFF *fb);
Index: buff.c
===================================================================
RCS file: /export/home/cvs/apache/src/buff.c,v
retrieving revision 1.8
diff -c -r1.8 buff.c
*** buff.c	1996/11/03 21:25:04	1.8
--- buff.c	1996/11/09 08:13:18
***************
*** 356,361 ****
--- 356,405 ----
  }
  
  /*
+  * Looks at the stream fb and places the first character into buff
+  * without removing it from the stream buffer.
+  *
+  * Returns 1 on success, zero on end of transmission, or -1 on an error.
+  *
+  */
+ int blookc(char *buff, BUFF *fb)
+ {
+     int i;
+ 
+     *buff = '\0';
+     
+     if (!(fb->flags & B_RD)) {   /* Can't do blookc on an unbuffered stream */
+         errno = EINVAL;
+         return -1;
+     }
+     if (fb->flags & B_RDERR) return -1;
+ 
+     if (fb->incnt == 0) {        /* no characters left in stream buffer */
+         fb->inptr = fb->inbase;
+         if (fb->flags & B_EOF)
+             return 0;
+ 
+         do {
+             i = read(fb->fd_in, fb->inptr, fb->bufsiz);
+         } while (i == -1 && errno == EINTR);
+ 
+         if (i == -1) {
+             if (errno != EAGAIN)
+                 doerror(fb, B_RD);
+             return -1;
+         }
+         if (i == 0) {
+             fb->flags |= B_EOF;
+             return 0; /* EOF */
+         }
+         else fb->incnt = i;
+     }
+ 
+     *buff = fb->inptr[0];
+     return 1;
+ }
+ 
+ /*
   * Skip data until a linefeed character is read
   * Returns 1 on success, 0 if no LF found, or -1 on error
   */
Index: http_protocol.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_protocol.c,v
retrieving revision 1.73
diff -c -r1.73 http_protocol.c
*** http_protocol.c	1996/11/08 02:51:34	1.73
--- http_protocol.c	1996/11/09 08:13:18
***************
*** 348,362 ****
      return OK;
  }
  
! static char *getline(char *s, int n, BUFF *in)
  {
!     int retval = bgets (s, n, in);
  
!     if (retval == -1) return NULL;
! 
!     if (retval > 0 && s[retval-1] == '\n') s[retval-1] = '\0';
! 
!     return s;
  }
  
  void parse_uri (request_rec *r, const char *uri)
--- 348,401 ----
      return OK;
  }
  
! /* Get a line of protocol input, including any continuation lines
!  * caused by MIME folding (or broken clients) if fold != 0, and place it
!  * in the buffer s, of size n bytes, without the ending newline.
!  *
!  * Returns -1 on error, or the length of s.
!  *
!  * Note: Because bgets uses 1 char for newline and 1 char for NUL,
!  *       the most we can get is (n - 2) actual characters if it
!  *       was ended by a newline, or (n - 1) characters if the line
!  *       length exceeded (n - 1).  So, if the result == (n - 1),
!  *       then the actual input line exceeded the buffer length,
!  *       and it would be a good idea for the caller to puke 400 or 414.
!  */
! static int getline(char *s, int n, BUFF *in, int fold)
  {
!     char *pos, next;
!     int retval;
!     int total = 0;
! 
!     pos = s;
! 
!     do {
!         retval = bgets(pos, n, in);    /* retval == -1 if error, 0 if EOF */
! 
!         if (retval <= 0)
!             return ((retval < 0) && (total == 0)) ? -1 : total;
! 
!         /* retval is the number of characters read, not including NUL     */
! 
!         n     -= retval;      /* Keep track of how much of s is full      */
!         pos   += (retval-1);  /*               and where s ends           */
!         total += retval;      /*               and how long s has become  */
! 
!         if (*pos == '\n') {   /* Did we get a full line of input?         */
!             *pos = '\0';
!             --total; ++n;
!         }
!         else return total;    /* if not, input line exceeded buffer size  */
! 
!     /* Continue appending if line folding is desired and
!      * the last line was not empty and we have room in the buffer and
!      * the next line begins with a continuation character.
!      */
!     } while (fold && (retval != 1) && (n > 1) &&
!              (blookc(&next, in) == 1) &&
!              ((next == ' ') || (next == '\t')));
  
!     return total;
  }
  
  void parse_uri (request_rec *r, const char *uri)
***************
*** 459,470 ****
      const char *ll = l, *uri;
      conn_rec *conn = r->connection;
      int major = 1, minor = 0;	/* Assume HTTP/1.0 if non-"HTTP" protocol*/
      
!     l[0] = '\0';
!     if(!getline(l, HUGE_STRING_LEN, conn->client))
!         return 0;
!     if(!l[0]) 
!         return 0;
  
      r->the_request = pstrdup (r->pool, l);
      r->method = getword_white(r->pool, &ll);
--- 498,514 ----
      const char *ll = l, *uri;
      conn_rec *conn = r->connection;
      int major = 1, minor = 0;	/* Assume HTTP/1.0 if non-"HTTP" protocol*/
+     int len;
      
!     /* Read past empty lines until we get a real request line,
!      * a read error, the connection closes (EOF), or we timeout.
!      */
!     while ((len = getline(l, HUGE_STRING_LEN, conn->client, 0)) <= 0) {
!         if ((len < 0) || bgetflag(conn->client, B_EOF))
!             return 0;
!     }
!     if (len == (HUGE_STRING_LEN - 1))
!         return 0;               /* Should be a 414 error status instead */
  
      r->the_request = pstrdup (r->pool, l);
      r->method = getword_white(r->pool, &ll);
***************
*** 480,528 ****
      return 1;
  }
  
! void get_mime_headers(request_rec *r)
  {
-     char w[MAX_STRING_LEN];
-     char *t;
      conn_rec *c = r->connection;
!     int len = 0;
!     char lookahead[2];
  
!     if (getline(w, MAX_STRING_LEN-1, c->client)) {
!         do {
! 	    if(!w[len])
! 	        return;
! 	    /* w[] contains the _current_ line. Lets read the
! 	     * first char of the _next_ line into lookahead[] and see
! 	     * if it is a continuation line */
! 	    if (!getline(lookahead, 2, c->client) ||
! 		*lookahead == '\0' ||
! 		(*lookahead != ' ' && *lookahead != '\t')) {
!  	        /* Not a continuation line -- _next_ line is either
! 		 * a read error, empty, or doesn't start with SPACE or TAB
! 		 * -- so store the _current_ line now */
! 		if(!(t = strchr(w,':')))
! 		    continue;
! 		*t++ = '\0';
! 		while(isspace(*t)) ++t;
! 
! 		table_merge (r->headers_in, w, t);
! 
! 		if (!*lookahead) /* did we read an empty line? */
! 		    return;
! 
! 		/* Put what we read as the start of the new _current_ line */
! 		w[0] = '\0';
! 	    }
! 	    /* To get here, here have got a lookahead character in
! 	     * *lookahead, so append it onto the end of w[], then
! 	     * read the next line onto the end of that. Move
! 	     * len on to point to the first char read from the next
! 	     * line of input... we use this at the top of the loop
! 	     * to check whether we actually read anything. */
!  	} while (len = strlen(w),
! 		 w[len++] = *lookahead,
! 		 getline (w+len, MAX_STRING_LEN-1-len, c->client));
      }
  }
  
--- 524,550 ----
      return 1;
  }
  
! void get_mime_headers (request_rec *r)
  {
      conn_rec *c = r->connection;
!     int len;
!     char *value;
!     char field[MAX_STRING_LEN];
! 
!     /* Read header lines until we get the empty separator line,
!      * a read error, the connection closes (EOF), or we timeout.
!      * Should we also check for overflow (len == MAX_STRING_LEN-1)?
!      */
!     while ((len = getline(field, MAX_STRING_LEN, c->client, 1)) > 0) {
! 
!         if (!(value = strchr(field,':')))     /* Find the colon separator */
!             continue;                         /*  or should puke 400 here */
! 
!         *value = '\0';
!         ++value;
!         while (isspace(*value)) ++value;      /* Skip to start of value   */
  
!         table_merge(r->headers_in, field, value);
      }
  }
  

Mime
View raw message