On Fri 20 Mar 2009, Graham Leggett wrote:
> Torsten Foertsch wrote:
> > I need the include virtual directive to be able to issue POST
> > requests. It should pass the request body to the subrequest. So I
> > came up with the attached patch.
> >
> > It allows to write
> >
> > <!--#include method="post" virtual="..." --> or
> > <!--#include method="inherit" virtual="..." -->
> >
[...]
> Something like this has already been added to trunk, take a look at
> the KEEP_BODY and KEPT_BODY filters in modules/filters/mod_request.c.
I did and, frankly, it is not the solution I was looking for. One has to
define a max. body size to be kept. The body is kept in RAM which can
be a problem unless KeptBodySize is rather small. So I developed my
patch further.
It defers now the ap_discard_request_body call as much as possible. This
gives output filters the chance to read the req body. If the client is
expecting a "100 Continue" message it is sent just before the first
line of output.
Is there a chance for the patch to make it into 2.3++? If yes I'll merge
it with the KEPT_BODY stuff.
Currently my httpd passes the current test framework with a few more
patches that are not related to this one (see "2.2.11 & mod_include"
thread) with one exception. Since the request body is read when output
is potentially already on the wire a HTTP_REQUEST_ENTITY_TOO_LARGE
error cannot be sent to the client if it sends the request body in
chunked TE. The only sensible solution that I can think of would be to
always send a 413 response if TE is chunked and a LimitRequestBody is
active.
On Fri 20 Mar 2009, Nick Kew wrote:
> Erm ... that's ringing alarm bells. The client, not the
> server, determines HTTP methods. Or are you talking about
> proxied subrequests here?
I see it a bit different. Subrequests for included documents are made on
behalf of the HTML programmer who wrote the frame. He decides to pass
on the request body and he decides which method to use, IMHO. And yes,
the problem comes from subrequests that are proxied to another server.
Torsten
--
Need professional mod_perl support?
Just hire me: [email protected]
--- modules/filters/mod_include.c.orig 2008-03-17 15:32:47.000000000 +0100
+++ modules/filters/mod_include.c 2009-03-25 14:49:14.000000000 +0100
@@ -1656,6 +1656,7 @@
apr_bucket_brigade *bb)
{
request_rec *r = f->r;
+ enum {METHOD_GET, METHOD_POST, METHOD_INHERIT} method;
if (!ctx->argc) {
ap_log_rerror(APLOG_MARK,
@@ -1674,6 +1675,8 @@
return APR_SUCCESS;
}
+ method=METHOD_GET;
+
while (1) {
char *tag = NULL;
char *tag_val = NULL;
@@ -1686,6 +1689,29 @@
break;
}
+ if (tag[0] == 'm' && !strcmp(tag, "method")) {
+ if ((tag_val[0] == 'g' || tag_val[0] == 'G')
+ && !strcasecmp(tag_val, "get")) {
+ method=METHOD_GET;
+ }
+ else if ((tag_val[0] == 'p' || tag_val[0] == 'P')
+ && !strcasecmp(tag_val, "post")) {
+ method=METHOD_POST;
+ }
+ else if ((tag_val[0] == 'i' || tag_val[0] == 'I')
+ && !strcasecmp(tag_val, "inherit")) {
+ method=METHOD_INHERIT;
+ }
+ else {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "unknown value "
+ "\"%s\" to parameter \"method\" of tag "
+ "include in %s", tag_val, r->filename);
+ SSI_CREATE_ERROR_BUCKET(ctx, f, bb);
+ break;
+ }
+ continue;
+ }
+
if (strcmp(tag, "virtual") && strcmp(tag, "file")) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, "unknown parameter "
"\"%s\" to tag include in %s", tag, r->filename);
@@ -1712,7 +1738,15 @@
}
}
else {
- rr = ap_sub_req_lookup_uri(parsed_string, r, f->next);
+ if (method == METHOD_GET
+ || (method == METHOD_INHERIT && strcmp(r->method, "POST"))) {
+ rr = ap_sub_req_lookup_uri(parsed_string, r, f->next);
+ }
+ else { /* POST */
+ method = METHOD_POST;
+ apr_table_setn(r->notes, "subreq-pass-request-body", "1");
+ rr = ap_sub_req_method_uri("POST", parsed_string, r, f->next);
+ }
}
if (!error_fmt && rr->status != HTTP_OK) {
@@ -1734,10 +1768,22 @@
ap_set_module_config(rr->request_config, &include_module, r);
}
+ /* XXX: would be good to check for EOS on rr->input_filters
+ * if method==POST and issue a warning if so.
+ */
+
if (!error_fmt && ap_run_sub_req(rr)) {
error_fmt = "unable to include \"%s\" in parsed file %s";
}
+ /* method="POST" must be specified *before* *each*
+ * virtual="..."
+ */
+ if (method != METHOD_GET) {
+ method = METHOD_GET;
+ apr_table_unset(r->notes, "subreq-pass-request-body");
+ }
+
if (error_fmt) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, error_fmt, tag_val,
r->filename);
--- server/core.c.orig 2009-03-21 13:15:41.000000000 +0100
+++ server/core.c 2009-03-25 14:13:13.000000000 +0100
@@ -3617,17 +3617,11 @@
ap_allow_standard_methods(r, MERGE_ALLOW, M_GET, M_OPTIONS, M_POST, -1);
- /* If filters intend to consume the request body, they must
- * register an InputFilter to slurp the contents of the POST
- * data from the POST input stream. It no longer exists when
- * the output filters are invoked by the default handler.
- */
- if ((errstatus = ap_discard_request_body(r)) != OK) {
- return errstatus;
- }
-
if (r->method_number == M_GET || r->method_number == M_POST) {
if (r->finfo.filetype == 0) {
+ if ((errstatus = ap_discard_request_body(r)) != OK) {
+ return errstatus;
+ }
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"File does not exist: %s", r->filename);
return HTTP_NOT_FOUND;
@@ -3637,6 +3631,9 @@
* raw I/O on a dir.
*/
if (r->finfo.filetype == APR_DIR) {
+ if ((errstatus = ap_discard_request_body(r)) != OK) {
+ return errstatus;
+ }
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"Attempt to serve directory: %s", r->filename);
return HTTP_NOT_FOUND;
@@ -3645,6 +3642,9 @@
if ((r->used_path_info != AP_REQ_ACCEPT_PATH_INFO) &&
r->path_info && *r->path_info)
{
+ if ((errstatus = ap_discard_request_body(r)) != OK) {
+ return errstatus;
+ }
/* default to reject */
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"File does not exist: %s",
@@ -3665,6 +3665,9 @@
req_cfg = ap_get_module_config(r->request_config, &core_module);
if (!req_cfg->deliver_script) {
+ if ((errstatus = ap_discard_request_body(r)) != OK) {
+ return errstatus;
+ }
/* The flag hasn't been set for this request. Punt. */
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"This resource does not accept the %s method.",
@@ -3680,6 +3683,9 @@
? 0 : APR_SENDFILE_ENABLED)
#endif
, 0, r->pool)) != APR_SUCCESS) {
+ if ((errstatus = ap_discard_request_body(r)) != OK) {
+ return errstatus;
+ }
ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r,
"file permissions deny server access: %s", r->filename);
return HTTP_FORBIDDEN;
@@ -3738,6 +3744,11 @@
APR_BRIGADE_INSERT_TAIL(bb, e);
status = ap_pass_brigade(r->output_filters, bb);
+
+ if ((errstatus = ap_discard_request_body(r)) != OK) {
+ return errstatus;
+ }
+
if (status == APR_SUCCESS
|| r->status != HTTP_OK
|| c->aborted) {
@@ -3752,6 +3763,9 @@
}
}
else { /* unusual method (not GET or POST) */
+ if ((errstatus = ap_discard_request_body(r)) != OK) {
+ return errstatus;
+ }
if (r->method_number == M_INVALID) {
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"Invalid method in request %s", r->the_request);
--- modules/http/http_filters.c.orig 2008-12-02 23:28:21.000000000 +0100
+++ modules/http/http_filters.c 2009-03-31 15:54:34.000000000 +0200
@@ -321,8 +321,9 @@
/* Since we're about to read data, send 100-Continue if needed.
* Only valid on chunked and C-L bodies where the C-L is > 0. */
if ((ctx->state == BODY_CHUNK ||
- (ctx->state == BODY_LENGTH && ctx->remaining > 0)) &&
- f->r->expecting_100 && f->r->proto_num >= HTTP_VERSION(1,1) &&
+ (ctx->state == BODY_LENGTH && ctx->remaining > 0)) &&
+ f->r->expecting_100==1 &&
+ f->r->proto_num >= HTTP_VERSION(1,1) &&
!(f->r->eos_sent || f->r->bytes_sent)) {
if (!ap_is_HTTP_SUCCESS(f->r->status)) {
ctx->state = BODY_NONE;
@@ -330,6 +331,9 @@
} else {
char *tmp;
+ /* don't send it twice */
+ f->r->expecting_100++;
+
tmp = apr_pstrcat(f->r->pool, AP_SERVER_PROTOCOL, " ",
ap_get_status_line(100), CRLF CRLF, NULL);
apr_brigade_cleanup(bb);
@@ -529,6 +533,7 @@
if (ctx->state == BODY_LENGTH && ctx->remaining == 0) {
e = apr_bucket_eos_create(f->c->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(b, e);
+ ctx->eos_sent = 1;
}
/* We have a limit in effect. */
@@ -851,6 +856,78 @@
}
+static apr_status_t check_input_body_length(request_rec *r)
+{
+ const char *tenc;
+ const char *lenp;
+
+ /* avoid recursion */
+ if (r->status == HTTP_REQUEST_ENTITY_TOO_LARGE) {
+ return APR_SUCCESS;
+ }
+
+ tenc = apr_table_get(r->headers_in, "Transfer-Encoding");
+ lenp = apr_table_get(r->headers_in, "Content-Length");
+
+ if( lenp && !tenc ) {
+ apr_off_t len=0, limit;
+ char *endstr;
+
+ if (apr_strtoff(&len, lenp, &endstr, 10)
+ || endstr == lenp
+ || *endstr
+ || len < 0) {
+ /* invalid CL */
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+ "Invalid Content-Length");
+ return HTTP_REQUEST_ENTITY_TOO_LARGE;
+ }
+
+ limit = ap_get_limit_req_body(r);
+ if (limit && limit < len) {
+ /* body too large */
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+ "Requested content-length of %" APR_OFF_T_FMT
+ " is larger than the configured limit"
+ " of %" APR_OFF_T_FMT, len, limit);
+ return HTTP_REQUEST_ENTITY_TOO_LARGE;
+ }
+ }
+
+ return APR_SUCCESS;
+ }
+
+static void http_send_continue_100(request_rec *r, apr_bucket_brigade *bb)
+{
+ const char *tenc = apr_table_get(r->headers_in, "Transfer-Encoding");
+ const char *lenp = apr_table_get(r->headers_in, "Content-Length");
+
+ if (r->expecting_100==1 && /* expected reply is not sent yet */
+ r->proto_num >= HTTP_VERSION(1,1) &&
+ ((tenc && !strcasecmp(tenc, "chunked")) || lenp)) {
+
+ struct iovec vec[3];
+
+ vec[0].iov_base = (void *)(AP_SERVER_PROTOCOL " ");
+ vec[0].iov_len = sizeof(AP_SERVER_PROTOCOL " ")-1;
+ vec[1].iov_base = (void *)(ap_get_status_line(100));
+ vec[1].iov_len = strlen(vec[1].iov_base);
+ vec[2].iov_base = (void *)(CRLF CRLF);
+ vec[2].iov_len = sizeof(CRLF CRLF) - 1;
+#if APR_CHARSET_EBCDIC
+ {
+ char *tmp;
+ apr_size_t len;
+ tmp = apr_pstrcatv(r->pool, vec, 3, &len);
+ ap_xlate_proto_to_ascii(tmp, len);
+ apr_brigade_write(bb, NULL, NULL, tmp, len);
+ }
+#else
+ apr_brigade_writev(bb, NULL, NULL, vec, 3);
+#endif
+ }
+}
+
/* fill "bb" with a barebones/initial HTTP response header */
static void basic_http_header(request_rec *r, apr_bucket_brigade *bb,
const char *protocol)
@@ -865,6 +942,9 @@
return;
}
+ /* this is the last chance to signal the client to send the req body */
+ http_send_continue_100(r, bb);
+
/* Output the HTTP/1.x Status-Line and the Date and Server fields */
vec[0].iov_base = (void *)protocol;
@@ -1104,6 +1184,7 @@
header_filter_ctx *ctx = f->ctx;
const char *ctype;
ap_bucket_error *eb = NULL;
+ int status;
AP_DEBUG_ASSERT(!r->main);
@@ -1135,12 +1216,17 @@
}
}
if (eb) {
- int status;
-
status = eb->status;
apr_brigade_cleanup(b);
ap_die(status, r);
return AP_FILTER_ERROR;
+ }
+
+ /* check request body in case the http input filter has not been called */
+ if ((status = check_input_body_length(r)) != APR_SUCCESS) {
+ apr_brigade_cleanup(b);
+ ap_die(status, r);
+ return AP_FILTER_ERROR;
}
if (r->assbackwards) {
--- modules/proxy/mod_proxy_http.c.orig 2008-11-11 21:04:34.000000000 +0100
+++ modules/proxy/mod_proxy_http.c 2009-03-22 15:41:05.000000000 +0100
@@ -907,8 +907,11 @@
* input_brigade and jump past all of the request body logic...
* Reading anything with ap_get_brigade is likely to consume the
* main request's body or read beyond EOS - which would be unplesant.
+ *
+ * An exception: if the main req is marked with the
+ * "subreq-pass-request-body" note the subreq may consume the body.
*/
- if (r->main) {
+ if (r->main && !apr_table_get(r->main->notes, "subreq-pass-request-body")) {
/* XXX: Why DON'T sub-requests use keepalives? */
p_conn->close++;
if (old_cl_val) {
--- server/protocol.c.orig 2007-12-12 21:43:04.000000000 +0100
+++ server/protocol.c 2009-03-22 15:44:39.000000000 +0100
@@ -1085,9 +1085,12 @@
/* did the original request have a body? (e.g. POST w/SSI tags)
* if so, make sure the subrequest doesn't inherit body headers
+ *
+ * Exception: the req is marked with "subreq-pass-request-body"
*/
- if (apr_table_get(r->headers_in, "Content-Length")
- || apr_table_get(r->headers_in, "Transfer-Encoding")) {
+ if (!apr_table_get(r->notes, "subreq-pass-request-body")
+ && (apr_table_get(r->headers_in, "Content-Length")
+ || apr_table_get(r->headers_in, "Transfer-Encoding"))) {
clone_headers_no_body(rnew, r);
} else {
/* no body (common case). clone headers the cheap way */
