You are viewing a plain text version of this content. The canonical link for it is here.
Posted to apreq-dev@httpd.apache.org by Markus Wichitill <ma...@gmx.de> on 2004/08/01 17:55:35 UTC

Re: Problem use Apache::Upload

Joe Schaefer wrote:
> In your case (a content-handler using Apache::Request) 
> you should always call discard_request_body (not just for GET).
> The way to think of the discard_request_body call is
> 
>   "my content handler does not need the raw POST data,
>    so tell apache to pull it through the input 
>    filters via ap_get_brigade(), and then delete 
>    all the resulting buckets."

That sounds different from the RequestIO docs. So does the following make 
sense then?

[...]
if (MP) {
   $apr = Apache::Request->new($ap,
     POST_MAX => $cfg->{maxAttachLen},
     TEMP_DIR => $cfg->{attachFsPath},
   );

   $ap->discard_request_body() == 0
     or error("discard_request_body() failed.")
     if MP2;

   $apr->parse() == 0
     or error("Input exceeds maximum allowed size or is corrupted.")
     if $ap->method eq 'POST';
}
elsif (CGIAPR) {
   my $pool = APR::Pool->new();
   $apr = Apache::Request->new($pool,
     POST_MAX => $cfg->{maxAttachLen},
     TEMP_DIR => $cfg->{attachFsPath},
   );

   $apr->parse() == 0
     or error("Input exceeds maximum allowed size or is corrupted.")
     if $method eq 'POST';
}
elsif (CGI) {
[...]

I just want to make sure, because I'm getting upload errors with files of a 
specific size that I'm not getting without discard_request_body(). If the 
code should make sense, I'll post details about that.

Re: Problem use Apache::Upload

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Markus Wichitill <ma...@gmx.de> writes:

> Joe Schaefer wrote:
> > That looks like memory corruption, not a parser failure.  I see
> > your problem now- see if this patch fixes it:
> 
> Yes, it seems to be fixed, thanks.
> 
> On Windows I couldn't reproduce the problem anymore today, even
> without the patch, but I guess that's the nature of memory
> corruption issues. 

Right- it's a very hard bug to tickle out.  What was 
happening is this: when the param's headers fall on a 
separate brigade from the param's value, the transient 
bucket that was holding the param name gets setaside, 
which morphs it into a pool bucket (and, undesirably, 
copies the bucket's data).  Once that bucket is 
destroyed by the parser, the copied bucket data 
(which now represents the param's name) is reclaimed.

The correct fix is to use immortal buckets in place of 
the transient buckets in the mfd parser.  I've committed
the change now.

-- 
Joe Schaefer


Re: Problem use Apache::Upload

Posted by Markus Wichitill <ma...@gmx.de>.
Joe Schaefer wrote:
> That looks like memory corruption, not a parser failure.  I see
> your problem now- see if this patch fixes it:

Yes, it seems to be fixed, thanks.

On Windows I couldn't reproduce the problem anymore today, even without the 
patch, but I guess that's the nature of memory corruption issues.

Re: Problem use Apache::Upload

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Markus Wichitill <ma...@gmx.de> writes:

> [Mon Aug 02 13:20:29 2004] [error] [forum] [client 192.168.0.1] key =
> <autht-Disposition: form-data;

[...]

> --\r\n>, val => <1283460750>
> 

That looks like memory corruption, not a parser failure.  I see
your problem now- see if this patch fixes it:

Index: src/apreq_parsers.c
===================================================================
RCS file: /home/cvs/httpd-apreq-2/src/apreq_parsers.c,v
retrieving revision 1.59
diff -u -r1.59 apreq_parsers.c
--- src/apreq_parsers.c 31 Jul 2004 23:56:40 -0000      1.59
+++ src/apreq_parsers.c 2 Aug 2004 15:19:25 -0000
@@ -948,7 +948,7 @@

             case APR_SUCCESS:
                 /* part has no body- return CRLF to front */
-                e = apr_bucket_transient_create(CRLF, 2,
+                e = apr_bucket_immortal_create(CRLF, 2,
                                                 ctx->bb->bucket_alloc);
                 APR_BRIGADE_INSERT_HEAD(ctx->in,e);
                 break;
@@ -975,7 +975,7 @@

             if (s != APR_SUCCESS) {
                 name = apr_pstrmemdup(pool, name, nlen);
-                e = apr_bucket_transient_create(name, nlen,
+                e = apr_bucket_immortal_create(name, nlen,
                                                 ctx->bb->bucket_alloc);
                 APR_BRIGADE_INSERT_HEAD(ctx->bb,e);
                 ctx->status = MFD_PARAM;


-- 
Joe Schaefer


Re: Problem use Apache::Upload

Posted by Markus Wichitill <ma...@gmx.de>.
Joe Schaefer wrote:
> Ok- looks like the mfd parser is running to a succesful completion,
> so *it* thinks it parsed the full mfd data.  I can't reproduce
> the bug here, so we need to figure out where the last param 
> wound up.  Try dumping the body table with something like
> 
>   my $t = $req->body;
>   print "key = <$a>, val => <$b>" while ($a, $b) = each %$t;

[Mon Aug 02 13:20:29 2004] [error] [forum] [client 192.168.0.1] key = 
<file>, val => <test.dat>
[Mon Aug 02 13:20:29 2004] [error] [forum] [client 192.168.0.1] key = 
<attach>, val => <Upload>
[Mon Aug 02 13:20:29 2004] [error] [forum] [client 192.168.0.1] key = <pid>, 
val => <11381>
[Mon Aug 02 13:20:29 2004] [error] [forum] [client 192.168.0.1] key = <ori>, 
val => <>
[Mon Aug 02 13:20:29 2004] [error] [forum] [client 192.168.0.1] key = 
<autht-Disposition: form-data; 
name="pid"\r\n\r\n11381\r\n-----------------------------2921238217421\r\nContent-Disposition: 
form-data; 
name="ori"\r\n\r\n\r\n-----------------------------2921238217421\r\nContent-Disposition: 
form-data; 
name="auth"\r\n\r\n1283460750\r\n-----------------------------2921238217421
--\r\n>, val => <1283460750>

Re: Problem use Apache::Upload

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Markus Wichitill <ma...@gmx.de> writes:

[...]

> parse() is always 0, otherwise the init code I posted would stop the
> request early. body_status() see below.

Ok- looks like the mfd parser is running to a succesful completion,
so *it* thinks it parsed the full mfd data.  I can't reproduce
the bug here, so we need to figure out where the last param 
wound up.  Try dumping the body table with something like

  my $t = $req->body;
  print "key = <$a>, val => <$b>" while ($a, $b) = each %$t;

Double-check that the 'ori' parameter value is really empty.

-- 
Joe Schaefer


Re: Problem use Apache::Upload

Posted by Markus Wichitill <ma...@gmx.de>.
Joe Schaefer wrote:
> OK, could you show us some status codes for the failed request?
> Foe example, what do these report?
> 
>   $req->parse
>   $req->body_status

parse() is always 0, otherwise the init code I posted would stop the request 
early. body_status() see below.

> and set your LogLevel to Debug, so we can see what mod_apreq
> and the mfd parser are doing.

First with discard_request_body() enabled for all mp2 requests:

[Sun Aug 01 23:11:10 2004] [debug] worker.c(1631): AcceptMutex: sysvsem 
(default: sysvsem)
*** Successful upload on first request after restart:
[Sun Aug 01 23:11:13 2004] [error] [forum] [client 192.168.0.1] body_status 
after new(): 70022
[Sun Aug 01 23:11:13 2004] [error] [forum] [client 192.168.0.1] body_status 
after discard(): 0
[Sun Aug 01 23:11:13 2004] [error] [forum] [client 192.168.0.1] body_status 
after parse(): 0
[Sun Aug 01 23:11:13 2004] [debug] mod_apreq.c(591): [client 192.168.0.1] 
removing filter (1)
*** Next upload, missing auth param:
[Sun Aug 01 23:12:27 2004] [error] [forum] [client 192.168.0.1] body_status 
after new(): 70022
[Sun Aug 01 23:12:28 2004] [error] [forum] [client 192.168.0.1] body_status 
after discard(): 0
[Sun Aug 01 23:12:28 2004] [error] [forum] [client 192.168.0.1] body_status 
after parse(): 0
[Sun Aug 01 23:12:28 2004] [error] [forum] [client 192.168.0.1] Request 
authentication failed.

Now discard_request_body() only for GET requests:

[Sun Aug 01 23:17:22 2004] [debug] worker.c(1631): AcceptMutex: sysvsem 
(default: sysvsem)
*** First upload after restart ok
[Sun Aug 01 23:17:35 2004] [error] [forum] [client 192.168.0.1] body_status 
after new(): 70022
[Sun Aug 01 23:17:35 2004] [debug] mod_apreq.c(349): [client 192.168.0.1] 
prefetching 262144 bytes
[Sun Aug 01 23:17:35 2004] [debug] mod_apreq.c(349): [client 192.168.0.1] 
prefetching 262144 bytes
[Sun Aug 01 23:17:35 2004] [error] [forum] [client 192.168.0.1] body_status 
after parse(): 0
[Sun Aug 01 23:17:35 2004] [debug] mod_apreq.c(591): [client 192.168.0.1] 
removing filter (1)
*** Next upload ok, too
[Sun Aug 01 23:18:47 2004] [error] [forum] [client 192.168.0.1] body_status 
after new(): 70022
[Sun Aug 01 23:18:47 2004] [debug] mod_apreq.c(349): [client 192.168.0.1] 
prefetching 262144 bytes
[Sun Aug 01 23:18:48 2004] [debug] mod_apreq.c(349): [client 192.168.0.1] 
prefetching 262144 bytes
[Sun Aug 01 23:18:48 2004] [error] [forum] [client 192.168.0.1] body_status 
after parse(): 0
[Sun Aug 01 23:18:48 2004] [debug] mod_apreq.c(591): [client 192.168.0.1] 
removing filter (1)

Re: Problem use Apache::Upload

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Markus Wichitill <ma...@gmx.de> writes:

> Joe Schaefer wrote:
> > It's basically a lower-level description of what the RequestIO
> > docs mean by
> >   "This helper routine tests for and reads any message body in the
> > request, simply discarding whatever it receives."
> 
> Well, to me that sounds more like the mp1 behaviour, where it really
> completely discards the request body so that apreq can't access the
> data anymore. 

In mp1, Apache::Request takes the data away from the content-handler.
In mp2, it leaves the data alone.  That's why you can/should discard 
it in your mp2 content-handler, but not in your mp1 handler.

> 
> >>I just want to make sure, because I'm getting upload errors with
> >> files of a specific size that I'm not getting without
> >> discard_request_body(). If the code should make sense, I'll post
> >> details about that.
> > Please do.
> 
> This won't be easy to reproduce.

[...]

OK, could you show us some status codes for the failed request?
Foe example, what do these report?

  $req->parse
  $req->body_status

and set your LogLevel to Debug, so we can see what mod_apreq
and the mfd parser are doing.
-- 
Joe Schaefer


Re: Problem use Apache::Upload

Posted by Markus Wichitill <ma...@gmx.de>.
Joe Schaefer wrote:
> It's basically a lower-level description of what the RequestIO
> docs mean by
> 
>   "This helper routine tests for and reads any message body in 
>   the request, simply discarding whatever it receives."

Well, to me that sounds more like the mp1 behaviour, where it really 
completely discards the request body so that apreq can't access the data 
anymore.

>>I just want to make sure, because I'm getting upload errors with
>>files of a specific size that I'm not getting without 
>>discard_request_body(). If the code should make sense, I'll post
>>details about that. 
> 
> Please do.

This won't be easy to reproduce.

What I do is I send the POST request shown below to my forum application. 
It's basically one file upload of 302680 bytes (size matters, contents 
don't) followed by a few parameters. POST_MAX is 1MB, well above that.

The problem is that the last param ("auth") is undefined if the script is 
run under mod_perl (mp2 and apreq2 current CVS, Apache 2.0.50, Linux/WinXP, 
Worker/NT MPM).

This doesn't happen:
- if I don't call discard_request_body() for POST requests.
- on the first request of a freshly started server, only subsequent ones.
- under CGI.
- if I remove the obsolete and always empty "ori" parameter.
- if the file is a few hundred bytes smaller.


POST /forum/post_attach.pl HTTP/1.1
Host: red.mshome.net
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) 
Gecko/20040626 Firefox/0.9.1
Accept: application/xhtml+xml,application/xml,*/*
Accept-Encoding: gzip,deflate
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive: 300
Connection: keep-alive
Cookie: mwf_login=1-secret
Content-Type: multipart/form-data; 
boundary=---------------------------19568103730103
Content-Length: 303279
-----------------------------19568103730103
Content-Disposition: form-data; name="file"; filename="test.dat"
Content-Type: application/octet-stream

xxxxxxxxxxxxxxxxxx [302680 bytes]
-----------------------------19568103730103
Content-Disposition: form-data; name="attach"

Upload
-----------------------------19568103730103
Content-Disposition: form-data; name="pid"

11380
-----------------------------19568103730103
Content-Disposition: form-data; name="ori"


-----------------------------19568103730103
Content-Disposition: form-data; name="auth"

1234567890
-----------------------------19568103730103--


HTTP/1.x 200 OK
Date: Sun, 01 Aug 2004 17:37:14 GMT
Server: Apache/2.0.50 (Unix) mod_perl/1.99_15-dev Perl/v5.8.5 DAV/2 SVN/1.0.5
Expires: Thu, 01 Jan 1970 00:00:00 GMT
Cache-Control: private
Connection: close
Transfer-Encoding: chunked
Content-Type: application/xhtml+xml; charset=iso-8859-1


Re: Problem use Apache::Upload

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Markus Wichitill <ma...@gmx.de> writes:

> Joe Schaefer wrote:
>
> > The way to think of the discard_request_body call is
> >   "my content handler does not need the raw POST data,
> >    so tell apache to pull it through the input    filters via
> > ap_get_brigade(), and then delete    all the resulting buckets."
> 
> That sounds different from the RequestIO docs.

It's basically a lower-level description of what the RequestIO
docs mean by

  "This helper routine tests for and reads any message body in 
  the request, simply discarding whatever it receives."

> So does the following make sense then?

[...]

Looks good to me.

> I just want to make sure, because I'm getting upload errors with
> files of a specific size that I'm not getting without 
> discard_request_body(). If the code should make sense, I'll post
> details about that. 

Please do.

-- 
Joe Schaefer