home *** CD-ROM | disk | FTP | other *** search
/ CD Actual 25 / CDROM25.iso / Share / linux / apache / contrib / patches / 1.3 / http408.patch next >
Encoding:
Internet Message Format  |  1998-06-11  |  7.7 KB
  1. Date: Thu, 5 Feb 1998 12:01:21 -0800 (PST)
  2. From: Ed Korthof <ed@organic.com>
  3. To: new-httpd@apache.org
  4. Subject: [PATCH] http_protocol (was Re: case insensitive, multi-line headers)
  5.  
  6. Sorry 'bout missing the case-insensitivity & the way multi-line headers
  7. are handled. <rueful>
  8.  
  9. I'd like to fix the actual bugs Marc and Dean described, as I have some
  10. free time on my hands.
  11.  
  12. So far as fixing the 408 -> 414 problem, we could set the status to
  13. HTTP_OK if it's currently 408 (we'd just do this in the sections where
  14. we're reading headers and we're about to return a 414 error to the client
  15. anyway, so this wouldn't mess up other parts of the code & vice versa),
  16. and then die() with an appropriate error.  So far as I can see, it really
  17. is true that other than timeouts and the potential to return an error
  18. while reading the headers, there's no way we'll have to worry about
  19. r->status being set to HTTP_REQUEST_TIME_OUT. 
  20.  
  21. There are, of course, other ways to do the HTTP_REQUEST_TIME_OUT thing,
  22. and perhaps it'd be worthwhile to change it.  I dunno.  Each of the other
  23. solutions I heard when we last went over this had it's own set of
  24. problems...
  25.  
  26. Anyway, I put together a patch to fix some the problems Marc described
  27. (without changing how we handle HTTP_REQUEST_TIME_OUT) and the one Dean
  28. described.  So far as the latter problem is concerned, I wrote the patch
  29. to keep pulling data and discard it.  It'd certainly also be possible to
  30. have it return a 400 error or pull in all the data and then combine it; 
  31. I'd be happy to reimplement this either of those ways. 
  32.  
  33. In this patch, I have set it up to log failures in the access log as well
  34. as in the error log.  Also, if run_post_read_request() returned something,
  35. the request would be killed (via die()), but it wouldn't be logged.  I've
  36. changed that as well... it seems we should log such stuff.
  37. Clients who trigger 400, 408, and 414 errors generally don't see the error
  38. itself, though it will be logged.  That might be worth fixing...
  39.  
  40. Comments are welcome, as always.
  41.  
  42. This is only lightly tested, though it does appear to work fine.
  43.  
  44.      -- Ed Korthof        |  Web Server Engineer --
  45.      -- ed@organic.com    |  Organic Online, Inc --
  46.      -- (415) 278-5676    |  Fax: (415) 284-6891 --
  47.  
  48. On Wed, 4 Feb 1998, Marc Slemko wrote:
  49.  
  50. > Yes.  This is mixed in with a bunch of other brokenness in this area (eg.
  51. > logging 408 (timeout) for a 414 (URI too large) that I ranted on a bit
  52. > ago.  Unfortunately, I'm swamped right now.  Sigh.  Perhaps next year.
  53. >
  54. > On Wed, 4 Feb 1998, Dean Gaudet wrote:
  55. > > On Wed, 4 Feb 1998, Dean Gaudet wrote:
  56. > > > Look at getline() we handle multiline headers.
  57. > > 
  58. > > Although there's an obvious bug.  If there's a line that exceeds the
  59. > > buffer size then we bail without reading the rest of the line and any
  60. > > possible continuation lines.  So we'll treat the rest of the line as if
  61. > > it's a new line.  Bad. 
  62. > > 
  63. > > Dean
  64.  
  65.  
  66.  
  67. Index: http_protocol.c
  68. ===================================================================
  69. RCS file: /export/home/cvs/apache-1.3/src/main/http_protocol.c,v
  70. retrieving revision 1.212
  71. diff -C3 -r1.212 http_protocol.c
  72. *** http_protocol.c    1998/04/27 06:59:35    1.212
  73. --- http_protocol.c    1998/05/06 00:07:41
  74. ***************
  75. *** 662,668 ****
  76.           ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r->server,
  77.                       "request failed for %s, reason: URI too long",
  78.               ap_get_remote_host(r->connection, r->per_dir_config, REMOTE_NAME));
  79. !         r->status = HTTP_REQUEST_URI_TOO_LARGE;
  80.           return 0;
  81.       }
  82.   
  83. --- 662,672 ----
  84.           ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r->server,
  85.                       "request failed for %s, reason: URI too long",
  86.               ap_get_remote_host(r->connection, r->per_dir_config, REMOTE_NAME));
  87. !     /* hack to deal with the HTTP_REQUEST_TIME_OUT setting up above: */
  88. !     if (r->status == HTTP_REQUEST_TIME_OUT)
  89. !       r->status = HTTP_OK;
  90. !     r->request_time = time(NULL);
  91. !     ap_die (HTTP_REQUEST_URI_TOO_LARGE, r);
  92.           return 0;
  93.       }
  94.   
  95. ***************
  96. *** 721,736 ****
  97.       while ((len = getline(field, MAX_STRING_LEN, c->client, 1)) > 0) {
  98.           char *copy = ap_palloc(r->pool, len + 1);
  99.           memcpy(copy, field, len + 1);
  100.   
  101. -         if (!(value = strchr(copy, ':')))      /* Find the colon separator */
  102. -             continue;           /* or should puke 400 here */
  103. - 
  104.           *value = '\0';
  105.           ++value;
  106.           while (isspace(*value))
  107.               ++value;            /* Skip to start of value   */
  108.   
  109.           ap_table_mergen(r->headers_in, copy, value);
  110.       }
  111.   }
  112.   
  113. --- 725,757 ----
  114.       while ((len = getline(field, MAX_STRING_LEN, c->client, 1)) > 0) {
  115.           char *copy = ap_palloc(r->pool, len + 1);
  116.           memcpy(copy, field, len + 1);
  117. +     
  118. +     if (!(value = strchr(copy, ':'))) {     /* Find the colon separator */
  119. +       /* if there's none, this request is screwed up.
  120. +        * a hack to deal with how we set HTTP_REQUEST_TIME_OUT earlier.*/
  121. +       if (r->status == HTTP_REQUEST_TIME_OUT)
  122. +         r->status = HTTP_OK;
  123. +       
  124. +       ap_die (HTTP_BAD_REQUEST, r);
  125. +       return;
  126. +     }
  127.   
  128.           *value = '\0';
  129.           ++value;
  130.           while (isspace(*value))
  131.               ++value;            /* Skip to start of value   */
  132.   
  133.           ap_table_mergen(r->headers_in, copy, value);
  134. + 
  135. +     /* the header was too long; at the least we should skip extra data */
  136. +     if (len >= MAX_STRING_LEN - 1) { 
  137. +       char junk[MAX_STRING_LEN];     
  138. +       while ((len = getline(junk, MAX_STRING_LEN, c->client, 1))
  139. +          >= MAX_STRING_LEN - 1)   /* soak up the extra data */
  140. +         ;
  141. +       if (len == 0) /* time to exit the larger loop as well */
  142. +         break;
  143. +     }
  144.       }
  145.   }
  146.   
  147. ***************
  148. *** 768,773 ****
  149. --- 789,795 ----
  150.       r->read_body       = REQUEST_NO_BODY;
  151.   
  152.       r->status          = HTTP_REQUEST_TIME_OUT;  /* Until we get a request */
  153. +     r->the_request     = NULL;
  154.   
  155.       /* Get the request... */
  156.   
  157. ***************
  158. *** 777,787 ****
  159. --- 799,820 ----
  160.       ap_keepalive_timeout("read request line", r);
  161.       if (!read_request_line(r)) {
  162.           ap_kill_timeout(r);
  163. +     if (r->status != HTTP_REQUEST_TIME_OUT)  /* we must have had an error.*/
  164. +         ap_log_transaction(r);
  165.           return NULL;
  166.       }
  167.       if (!r->assbackwards) {
  168.           ap_hard_timeout("read request headers", r);
  169.           get_mime_headers(r);
  170. +         if (r->status != HTTP_REQUEST_TIME_OUT) {/* we must have had an error.*/
  171. +         ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, r->server,
  172. +                  "request failed for %s: error reading the headers",
  173. +                  ap_get_remote_host(r->connection, r->per_dir_config, 
  174. +                         REMOTE_NAME));
  175. +         ap_log_transaction(r);
  176. +         return NULL;
  177. +     }
  178. + 
  179.       }
  180.       ap_kill_timeout(r);
  181.   
  182. ***************
  183. *** 799,804 ****
  184. --- 832,839 ----
  185.   
  186.       if ((access_status = ap_run_post_read_request(r))) {
  187.           ap_die(access_status, r);
  188. +     ap_log_transaction(r);
  189. + 
  190.           return NULL;
  191.       }
  192.   
  193. ***************
  194. *** 1986,1992 ****
  195.            * redirect URL. We don't really want to output this URL
  196.            * as a text message, so first check the custom response
  197.            * string to ensure that it is a text-string (using the
  198. !          * same test used in die(), i.e. does it start with a ").
  199.            * If it doesn't, we've got a recursive error, so find
  200.            * the original error and output that as well.
  201.            */
  202. --- 2021,2027 ----
  203.            * redirect URL. We don't really want to output this URL
  204.            * as a text message, so first check the custom response
  205.            * string to ensure that it is a text-string (using the
  206. !          * same test used in ap_die(), i.e. does it start with a ").
  207.            * If it doesn't, we've got a recursive error, so find
  208.            * the original error and output that as well.
  209.            */
  210.