You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "Roy T. Fielding" <fi...@kiwi.ics.uci.edu> on 1998/10/06 04:24:05 UTC

[PATCH] Fix 100 Continue and implement Expect

I've tested this one pretty well but, since I've been awake for 21 hours
straight, I think I'll sleep on it before committing.  This patch does
the following things:

  1) Adds a complete implementation of the Expect header field as
     specified in rev-05 of HTTP/1.1.  Will require an MMN bump.

  2) Uses that implementation as a means of disabling the 100 Continue
     response when we already know the final status, which is mighty
     useful for PUT responses that result in 302 or 401.

  3) Moves two ugly protocol condition checks that were in http_request.c
     over to where they belong in http_protocol.c.  They were put there
     originally because ap_read_request formerly could not return an
     error response, but I added that capability in 1.3.2.

  4) Removes extra trailing whitespace from the getline results as part
     of the getline processing, which is extra nice because it works
     between continuation lines, is almost no cost in the normal case
     of no extra whitespace, and saves memory.
 
....Roy


Index: include/httpd.h
===================================================================
RCS file: /home/cvs/apache-1.3/src/include/httpd.h,v
retrieving revision 1.245
diff -u -r1.245 httpd.h
--- httpd.h	1998/10/05 22:11:16	1.245
+++ httpd.h	1998/10/06 02:06:56
@@ -693,6 +693,8 @@
     int read_body;		/* how the request body should be read */
     int read_chunked;		/* reading chunked transfer-coding */
 
+    unsigned expecting_100;     /* is client waiting for a 100 response? */
+
     /* MIME header environments, in and out.  Also, an array containing
      * environment variables to be passed to subprocesses, so people can
      * write modules to add to that environment.
Index: main/http_protocol.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/main/http_protocol.c,v
retrieving revision 1.242
diff -u -r1.242 http_protocol.c
--- http_protocol.c	1998/10/05 22:11:16	1.242
+++ http_protocol.c	1998/10/06 02:06:56
@@ -551,6 +551,17 @@
         total += retval;        /* and how long s has become               */
 
         if (*pos == '\n') {     /* Did we get a full line of input?        */
+            /*
+             * Trim any extra trailing spaces or tabs except for the first
+             * space or tab at the beginning of a blank string.  This makes
+             * it much easier to check field values for exact matches, and
+             * saves memory as well.  Terminate string at end of line.
+             */
+            while (pos > (s + 1) && (*(pos - 1) == ' ' || *(pos - 1) == '\t')) {
+                --pos;          /* trim extra trailing spaces or tabs      */
+                --total;        /* but not one at the beginning of line    */
+                ++n;
+            }
             *pos = '\0';
             --total;
             ++n;
@@ -767,8 +778,6 @@
         while (*value == ' ' || *value == '\t')
             ++value;            /* Skip to start of value   */
 
-        /* XXX: should strip trailing whitespace as well */
-
 	ap_table_addn(tmp_headers, copy, value);
     }
 
@@ -778,8 +787,9 @@
 request_rec *ap_read_request(conn_rec *conn)
 {
     request_rec *r;
-    int access_status;
     pool *p;
+    const char *expect;
+    int access_status;
 
     p = ap_make_sub_pool(conn->pool);
     r = ap_pcalloc(p, sizeof(request_rec));
@@ -846,6 +856,23 @@
     }
     else {
         ap_kill_timeout(r);
+
+        if (r->header_only) {
+            /*
+             * Client asked for headers only with HTTP/0.9, which doesn't send
+             * headers! Have to dink things just to make sure the error message
+             * comes through...
+             */
+            ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
+                          "client sent invalid HTTP/0.9 request: HEAD %s",
+                          r->uri);
+            r->header_only = 0;
+            r->status = HTTP_BAD_REQUEST;
+            ap_send_error_response(r, 0);
+            ap_bflush(r->connection->client);
+            ap_log_transaction(r);
+            return r;
+        }
     }
 
     r->status = HTTP_OK;                         /* Until further notice. */
@@ -860,6 +887,49 @@
 
     conn->keptalive = 0;        /* We now have a request to play with */
 
+    if ((!r->hostname && (r->proto_num >= HTTP_VERSION(1,1))) ||
+        ((r->proto_num == HTTP_VERSION(1,1)) &&
+         !ap_table_get(r->headers_in, "Host"))) {
+        /*
+         * Client sent us an HTTP/1.1 or later request without telling us the
+         * hostname, either with a full URL or a Host: header. We therefore
+         * need to (as per the 1.1 spec) send an error.  As a special case,
+         * HTTP/1.1 mentions twice (S9, S14.23) that a request MUST contain
+         * a Host: header, and the server MUST respond with 400 if it doesn't.
+         */
+        ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
+                      "client sent HTTP/1.1 request without hostname "
+                      "(see RFC2068 section 9, and 14.23): %s", r->uri);
+        r->status = HTTP_BAD_REQUEST;
+        ap_send_error_response(r, 0);
+        ap_bflush(r->connection->client);
+        ap_log_transaction(r);
+        return r;
+    }
+    if (((expect = ap_table_get(r->headers_in, "Expect")) != NULL) &&
+        (expect[0] != '\0')) {
+        /*
+         * The Expect header field was added to HTTP/1.1 after RFC 2068
+         * as a means to signal when a 100 response is desired and,
+         * unfortunately, to signal a poor man's mandatory extension that
+         * the server must understand or return 417 Expectation Failed.
+         */
+        if (strcasecmp(expect, "100-continue") == 0) {
+            r->expecting_100 = 1;
+        }
+        else {
+            ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_INFO, r,
+                          "client sent an unrecognized expectation value: "
+                          "Expect: %s", expect);
+            r->status = HTTP_EXPECTATION_FAILED;
+            ap_send_error_response(r, 0);
+            ap_bflush(r->connection->client);
+            (void) ap_discard_request_body(r);
+            ap_log_transaction(r);
+            return r;
+        }
+    }
+
     if ((access_status = ap_run_post_read_request(r))) {
         ap_die(access_status, r);
         ap_log_transaction(r);
@@ -895,6 +965,7 @@
     rnew->err_headers_out = ap_make_table(rnew->pool, 5);
     rnew->notes           = ap_make_table(rnew->pool, 5);
 
+    rnew->expecting_100   = r->expecting_100;
     rnew->read_length     = r->read_length;
     rnew->read_body       = REQUEST_NO_BODY;
 
@@ -1457,7 +1528,7 @@
     if (r->read_length || (!r->read_chunked && (r->remaining <= 0)))
         return 0;
 
-    if (r->proto_num >= HTTP_VERSION(1,1)) {
+    if (r->expecting_100 && r->proto_num >= HTTP_VERSION(1,1)) {
         /* sending 100 Continue interim response */
         ap_bvputs(r->connection->client,
                SERVER_PROTOCOL, " ", status_lines[0], "\015\012\015\012",
@@ -1668,6 +1739,11 @@
 
     if ((rv = ap_setup_client_block(r, REQUEST_CHUNKED_PASS)))
         return rv;
+
+    /* If we are discarding the request body, then we must already know
+     * the final status code, therefore disable the sending of 100 continue.
+     */
+    r->expecting_100 = 0;
 
     if (ap_should_client_block(r)) {
         char dumpbuf[HUGE_STRING_LEN];
Index: main/http_request.c
===================================================================
RCS file: /home/cvs/apache-1.3/src/main/http_request.c,v
retrieving revision 1.132
diff -u -r1.132 http_request.c
--- http_request.c	1998/10/03 14:42:28	1.132
+++ http_request.c	1998/10/06 02:06:56
@@ -1024,39 +1024,6 @@
 {
     int access_status;
 
-    /*
-     * Kluge to be reading the assbackwards field outside of protocol.c, but
-     * we've got to check for this sort of nonsense somewhere...
-     */
-
-    if (r->assbackwards && r->header_only) {
-        /*
-         * Client asked for headers only with HTTP/0.9, which doesn't send
-         * headers!  Have to dink things even to make sure the error message
-         * comes through...
-         */
-        ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
-                    "client sent illegal HTTP/0.9 request: %s", r->uri);
-        r->header_only = 0;
-        ap_die(BAD_REQUEST, r);
-        return;
-    }
-
-    if ((!r->hostname && (r->proto_num >= HTTP_VERSION(1,1))) ||
-        ((r->proto_num == HTTP_VERSION(1,1)) && !ap_table_get(r->headers_in, "Host"))) {
-        /*
-         * Client sent us a HTTP/1.1 or later request without telling us the
-         * hostname, either with a full URL or a Host: header. We therefore
-         * need to (as per the 1.1 spec) send an error.  As a special case,
-	 * HTTP/1.1 mentions twice (S9, S14.23) that a request MUST contain
-	 * a Host: header, and the server MUST respond with 400 if it doesn't.
-         */
-        ap_log_rerror(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r,
-               "client sent HTTP/1.1 request without hostname (see RFC2068 section 9, and 14.23): %s", r->uri);
-        ap_die(BAD_REQUEST, r);
-        return;
-    }
-
     /* Ignore embedded %2F's in path for proxy requests */
     if (!r->proxyreq && r->parsed_uri.path) {
 	access_status = ap_unescape_url(r->parsed_uri.path);

Re: [PATCH] Fix 100 Continue and implement Expect

Posted by Martin Kraemer <ma...@mch.sni.de>.
On Mon, Oct 05, 1998 at 07:24:05PM -0700, Roy T. Fielding wrote:
> I've tested this one pretty well but, since I've been awake for 21 hours
> straight, I think I'll sleep on it before committing.

Though I would like to see the addition of this patch RSN,
I don't know the implications of this patch, and I wouldn't want to risk
the stability of the 1.3.3-to-be. Can this wait until 1.3.4-dev?

    Martin

Re: [PATCH] Fix 100 Continue and implement Expect

Posted by Alexei Kosut <ak...@leland.Stanford.EDU>.
On Mon, 5 Oct 1998, Roy T. Fielding wrote:

> Index: include/httpd.h
> ===================================================================
> RCS file: /home/cvs/apache-1.3/src/include/httpd.h,v
> retrieving revision 1.245
> diff -u -r1.245 httpd.h
> --- httpd.h	1998/10/05 22:11:16	1.245
> +++ httpd.h	1998/10/06 02:06:56
> @@ -693,6 +693,8 @@
>      int read_body;		/* how the request body should be read */
>      int read_chunked;		/* reading chunked transfer-coding */
>  
> +    unsigned expecting_100;     /* is client waiting for a 100 response? */
> +
>      /* MIME header environments, in and out.  Also, an array containing
>       * environment variables to be passed to subprocesses, so people can
>       * write modules to add to that environment.

Note that this breaks binary compatibility; if you place the new member at
the end of the struct, it would still be binary-compatible.

Something to consider.

-- Alexei Kosut <ak...@stanford.edu> <http://www.stanford.edu/~akosut/>
   Stanford University, Class of 2001 * Apache <http://www.apache.org> *