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 1997/07/19 18:13:18 UTC
[PATCH] PR#378 part 1: Fix handling of request body in core
This is a protocol bug fix. The patch is for 1.3 and I'll commit that,
but I'd also like to see it in any 1.2.2. Basically, we always need to
check for a request body, even for GET, OPTIONS, and TRACE. The latter
is required by HTTP/1.1 to not have a body, but we still must check for it.
I haven't fixed the problem with special-casing the "*" Request-URI
(we currently return 403), but this is all I have time for at the moment.
.....Roy
Index: http_core.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_core.c,v
retrieving revision 1.97
diff -c -r1.97 http_core.c
*** http_core.c 1997/07/17 22:27:29 1.97
--- http_core.c 1997/07/19 16:09:55
***************
*** 1368,1376 ****
(core_dir_config *)get_module_config(r->per_dir_config, &core_module);
int rangestatus, errstatus;
FILE *f;
!
r->allowed |= (1 << M_GET);
- r->allowed |= (1 << M_TRACE);
r->allowed |= (1 << M_OPTIONS);
if (r->method_number == M_INVALID) {
--- 1368,1381 ----
(core_dir_config *)get_module_config(r->per_dir_config, &core_module);
int rangestatus, errstatus;
FILE *f;
!
! /* This handler has no use for a request body (yet), but we still
! * need to read and discard it if the client sent one.
! */
! if ((errstatus = discard_request_body(r)) != OK)
! return errstatus;
!
r->allowed |= (1 << M_GET);
r->allowed |= (1 << M_OPTIONS);
if (r->method_number == M_INVALID) {
Index: http_protocol.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_protocol.c,v
retrieving revision 1.142
diff -c -r1.142 http_protocol.c
*** http_protocol.c 1997/07/19 10:20:50 1.142
--- http_protocol.c 1997/07/19 16:09:56
***************
*** 1094,1127 ****
bputs("\015\012", client); /* Send the terminating empty line */
}
static char *make_allow(request_rec *r)
{
! int allowed = r->allowed;
!
! if( allowed == 0 ) {
! /* RFC2068 #14.7, Allow must contain at least one method. So rather
! * than deal with the possibility of trying not to emit an Allow:
! * header, i.e. #10.4.6 says 405 Method Not Allowed MUST include
! * an Allow header, we'll just say TRACE is valid.
! */
! return( "TRACE" );
! }
!
! return 2 + pstrcat(r->pool, (allowed & (1 << M_GET)) ? ", GET, HEAD" : "",
! (allowed & (1 << M_POST)) ? ", POST" : "",
! (allowed & (1 << M_PUT)) ? ", PUT" : "",
! (allowed & (1 << M_DELETE)) ? ", DELETE" : "",
! (allowed & (1 << M_OPTIONS)) ? ", OPTIONS" : "",
! (allowed & (1 << M_TRACE)) ? ", TRACE" : "",
! NULL);
-
}
int send_http_trace (request_rec *r)
{
/* Get the original request */
while (r->prev) r = r->prev;
hard_timeout("send TRACE", r);
r->content_type = "message/http";
--- 1094,1124 ----
bputs("\015\012", client); /* Send the terminating empty line */
}
+ /* Build the Allow field-value from the request handler method mask.
+ * Note that we always allow TRACE, since it is handled below.
+ */
static char *make_allow(request_rec *r)
{
! return 2 + pstrcat(r->pool,
! (r->allowed & (1 << M_GET)) ? ", GET, HEAD" : "",
! (r->allowed & (1 << M_POST)) ? ", POST" : "",
! (r->allowed & (1 << M_PUT)) ? ", PUT" : "",
! (r->allowed & (1 << M_DELETE)) ? ", DELETE" : "",
! (r->allowed & (1 << M_OPTIONS)) ? ", OPTIONS" : "",
! ", TRACE",
! NULL);
}
int send_http_trace (request_rec *r)
{
+ int rv;
+
/* Get the original request */
while (r->prev) r = r->prev;
+ if ((rv = setup_client_block(r, REQUEST_NO_BODY)))
+ return rv;
+
hard_timeout("send TRACE", r);
r->content_type = "message/http";
***************
*** 1533,1538 ****
--- 1530,1572 ----
return (chunk_start + len_read);
}
+ /* In HTTP/1.1, any method can have a body. However, most GET handlers
+ * wouldn't know what to do with a request body if they received one.
+ * This helper routine tests for and reads any message body in the request,
+ * simply discarding whatever it receives. We need to do this because
+ * failing to read the request body would cause it to be interpreted
+ * as the next request on a persistent connection.
+ *
+ * Since we return an error status if the request is malformed, this
+ * routine should be called at the beginning of a no-body handler, e.g.,
+ *
+ * if ((retval = discard_request_body(r)) != OK)
+ * return retval;
+ */
+ API_EXPORT(int) discard_request_body(request_rec *r)
+ {
+ int rv;
+
+ if ((rv = setup_client_block(r, REQUEST_CHUNKED_PASS)))
+ return rv;
+
+ if (should_client_block(r)) {
+ char dumpbuf[HUGE_STRING_LEN];
+
+ hard_timeout("reading request body", r);
+ while ((rv = get_client_block(r, dumpbuf, HUGE_STRING_LEN)) > 0)
+ continue;
+ kill_timeout(r);
+
+ if (rv < 0)
+ return HTTP_BAD_REQUEST;
+ }
+ return OK;
+ }
+
+ /*
+ * Send the body of a response to the client.
+ */
API_EXPORT(long) send_fd(FILE *f, request_rec *r) {
return send_fd_length(f, r, -1);
}
Index: http_protocol.h
===================================================================
RCS file: /export/home/cvs/apache/src/http_protocol.h,v
retrieving revision 1.23
diff -c -r1.23 http_protocol.h
*** http_protocol.h 1997/07/15 21:39:56 1.23
--- http_protocol.h 1997/07/19 16:09:56
***************
*** 134,139 ****
--- 134,140 ----
API_EXPORT(int) setup_client_block (request_rec *r, int read_policy);
API_EXPORT(int) should_client_block (request_rec *r);
API_EXPORT(long) get_client_block (request_rec *r, char *buffer, int bufsiz);
+ API_EXPORT(int) discard_request_body (request_rec *r);
/* Sending a byterange */
Index: http_request.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_request.c,v
retrieving revision 1.64
diff -c -r1.64 http_request.c
*** http_request.c 1997/07/19 08:03:53 1.64
--- http_request.c 1997/07/19 16:09:56
***************
*** 930,937 ****
* we handle it specially.
*/
if (r->method_number == M_TRACE) {
! send_http_trace(r);
! finalize_request_protocol(r);
return;
}
--- 930,939 ----
* we handle it specially.
*/
if (r->method_number == M_TRACE) {
! if ((access_status = send_http_trace(r)))
! die(access_status, r);
! else
! finalize_request_protocol(r);
return;
}
Re: [PATCH] PR#378 part 1: Fix handling of request body in core
Posted by Dean Gaudet <dg...@arctic.org>.
This looks fine to me, +1. But what happens when things like mod_status
are sent a request body they don't handle? I thought the core took care
of that already. If not we've got a few little bugs to fix up.
On the topic of 1.2.2, I am totally surprised that the solaris HUP thing
has not proven to be as large a problem as it first seemed. We've had
only 2 or 3 PRs about it. It probably helps that known_bugs is so
explicit about the solution, but I expected far more reports of problems.
Maybe nobody HUPs their servers :)
Dean
On Sat, 19 Jul 1997, Roy T. Fielding wrote:
> This is a protocol bug fix. The patch is for 1.3 and I'll commit that,
> but I'd also like to see it in any 1.2.2. Basically, we always need to
> check for a request body, even for GET, OPTIONS, and TRACE. The latter
> is required by HTTP/1.1 to not have a body, but we still must check for it.
>
> I haven't fixed the problem with special-casing the "*" Request-URI
> (we currently return 403), but this is all I have time for at the moment.
>
> .....Roy