RE: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

[Ryan Bloom]

> From: William A. Rowe, Jr. [mailto:[EMAIL PROTECTED]]
> Sent: Saturday, June 22, 2002 9:45 AM
> To: [EMAIL PROTECTED]
> Subject: Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c
mod_cgid.c
> 
> Looking for both faults (passing the next filter when we are doing
> lookups,
> and passing NULL when we are going to serve the content) ... it really
> seems that;
> 
> D:\clean\httpd-2.0\modules\mappers\mod_negotiation.c(2550):
>  sub_req = ap_sub_req_lookup_file(variant->file_name, r,
NULL);
> 
> SHOULD be setting the next filter.  Anyone want to dig on this?

I am missing something.  The third argument has nothing to do with
whether the request will be served.  It has to do with how the filter
stack is initialized in the sub-request.  If the argument is NULL, the
sub-requests filters are set to the protocol filters of the original
request.  If the argument isn't NULL, then the sub-request's filters are
initialized to the filter passed in.  In either case, the data can
either be sent or not.

Please revert the doc portions of your last change, they are incorrect.
The code is safe to leave, because the requests aren't actually served.
However, mod_negotiation is 100% correct, and should NOT be changed.

Ryan

> 
> Bill
> 
> >wrowe   2002/06/22 09:32:45
> >
> >   Modified:include  http_request.h
> >modules/generators mod_cgi.c mod_cgid.c
> >   Log:
> > Note the changed meaning of the NULL next_filter argument to the
> > ap_sub_req_lookup() family, and fix a few oddball cases (those
are,
> > PATH_TRANSLATED reference issues.)
> 

Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

[William A. Rowe, Jr.]

Looking for both faults (passing the next filter when we are doing lookups,
and passing NULL when we are going to serve the content) ... it really
seems that;

D:\clean\httpd-2.0\modules\mappers\mod_negotiation.c(2550):
 sub_req = ap_sub_req_lookup_file(variant->file_name, r, NULL);

SHOULD be setting the next filter.  Anyone want to dig on this?

Bill

>wrowe   2002/06/22 09:32:45
>
>   Modified:include  http_request.h
>modules/generators mod_cgi.c mod_cgid.c
>   Log:
> Note the changed meaning of the NULL next_filter argument to the
> ap_sub_req_lookup() family, and fix a few oddball cases (those are,
> PATH_TRANSLATED reference issues.)

Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

[Jeff Trawick]

"Ryan Bloom" <[EMAIL PROTECTED]> writes:

> It's good to feel loved and appreciated.  :-)

;)

-- 
Jeff Trawick | [EMAIL PROTECTED]
Born in Roswell... married an alien...

RE: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

[Ryan Bloom]

From: [EMAIL PROTECTED] [mailto:trawick@rdu88-250-
> Thom May <[EMAIL PROTECTED]> writes:
> 
> > OK, this is fixed in the current 2.0.34 tag - my apologies for the
wrong
> > diagnosis.
> 
> no apologies necessary...  frankly, I always blame Ryan for everything
> even if he is nowhere in sight...  if you want to blame me than that
> is only fair :)

It's good to feel loved and appreciated.  :-)

Ryan

Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

[Jeff Trawick]

Thom May <[EMAIL PROTECTED]> writes:

> OK, this is fixed in the current 2.0.34 tag - my apologies for the wrong
> diagnosis.

no apologies necessary...  frankly, I always blame Ryan for everything
even if he is nowhere in sight...  if you want to blame me than that
is only fair :)

-- 
Jeff Trawick | [EMAIL PROTECTED]
Born in Roswell... married an alien...

Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

[Thom May]

* Jeff Trawick ([EMAIL PROTECTED]) wrote :
> Thom May <[EMAIL PROTECTED]> writes:
> 
> > This change appears to have broken some cgі scripts - cricket
> >  worked fine in 2.0.33 but in 2.0.34 and
> > 35-dev:
> > 
> > [Sat Mar 30 15:20:03 2002] [error] [client 192.168.0.34] malformed header
> > from 
: /usr/lib/cgi-bin/cricket/grapher.cgi
> > 
> > Cheers,
> > -Thom
> 
> I don't really see how.  The code executed was completely unchanged
> except when nph is set.  nph should only be set for a nph script --
> one that starts with nph-.
> 
> That message "malformed header" makes me wonder if a recent change to
> avoid reading one byte at a time broke it.  Look at Brian Pane's (I
> think it was him) recent commit to change the way we read headers from
> the CGI script.  Maybe there is a connection between that change and
> the breakage of your cgi.
> 
> If you have a simple example I'm willing to debug.
> 
OK, this is fixed in the current 2.0.34 tag - my apologies for the wrong
diagnosis.
-Thom

Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

[Brian Havard]

On Sat, 30 Mar 2002 21:14:41 -0800, Brian Pane wrote:

>Brian Havard wrote:
>
>>On Sat, 30 Mar 2002 16:14:55 +, Thom May wrote:
>>
>>>This change appears to have broken some cgÑ  scripts - cricket
>>> worked fine in 2.0.33 but in 2.0.34 and
>>>35-dev:
>>>
>>>[Sat Mar 30 15:20:03 2002] [error] [client 192.168.0.34] malformed header
>>>
>>>from
>>: /usr/lib/cgi-bin/cricket/grapher.cgi
>>
>>I'm seeing something similar to this in recent code. This is from a virtual
>>include in an SSI:
>>
>>[Sun Mar 31 14:24:05 2002] [error] [client 202.12.87.82] malformed header
>>from script. Bad header=
>>: F:/Data/Web.Pages/Silk/cgi-bin/count, referer: http://kheldar/silk/
>>
>>Looking at the log file in a hex view the bad header appears to be a single
>>CR character, suggesting that CR/LF terminated lines are no longer accepted
>>from CGI scripts.
>>
>
>Yes, that's the problem.  I based getsfunc_BRIGADE() on the
>semantics of the apr_file_gets() call that it replaced--but
>the Unix version, which only checks for LF and not CR.  Your
>patch looks good.

Ok, I've just committed it.

--
 __
 |  Brian Havard |  "He is not the messiah!   |
 |  [EMAIL PROTECTED]  |  He's a very naughty boy!" - Life of Brian |
 --

Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

[Brian Pane]

Brian Havard wrote:

>On Sat, 30 Mar 2002 16:14:55 +, Thom May wrote:
>
>>This change appears to have broken some cgÑ  scripts - cricket
>> worked fine in 2.0.33 but in 2.0.34 and
>>35-dev:
>>
>>[Sat Mar 30 15:20:03 2002] [error] [client 192.168.0.34] malformed header
>>
>>from 
>: /usr/lib/cgi-bin/cricket/grapher.cgi
>
>I'm seeing something similar to this in recent code. This is from a virtual
>include in an SSI:
>
>[Sun Mar 31 14:24:05 2002] [error] [client 202.12.87.82] malformed header
>from script. Bad header=
>: F:/Data/Web.Pages/Silk/cgi-bin/count, referer: http://kheldar/silk/
>
>Looking at the log file in a hex view the bad header appears to be a single
>CR character, suggesting that CR/LF terminated lines are no longer accepted
>from CGI scripts.
>

Yes, that's the problem.  I based getsfunc_BRIGADE() on the
semantics of the apr_file_gets() call that it replaced--but
the Unix version, which only checks for LF and not CR.  Your
patch looks good.

Thanks,
--Brian

Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

[Brian Havard]

On Sat, 30 Mar 2002 16:14:55 +, Thom May wrote:

>This change appears to have broken some cgÑ  scripts - cricket
> worked fine in 2.0.33 but in 2.0.34 and
>35-dev:
>
>[Sat Mar 30 15:20:03 2002] [error] [client 192.168.0.34] malformed header
>from
: /usr/lib/cgi-bin/cricket/grapher.cgi

I'm seeing something similar to this in recent code. This is from a virtual
include in an SSI:

[Sun Mar 31 14:24:05 2002] [error] [client 202.12.87.82] malformed header
from script. Bad header=
: F:/Data/Web.Pages/Silk/cgi-bin/count, referer: http://kheldar/silk/

Looking at the log file in a hex view the bad header appears to be a single
CR character, suggesting that CR/LF terminated lines are no longer accepted
from CGI scripts.

This fixes it for me, though may not be ideal:

Index: server/util_script.c
===
RCS file: /home/cvs/httpd-2.0/server/util_script.c,v
retrieving revision 1.75
diff -u -r1.75 util_script.c
--- server/util_script.c2002/03/30 22:48:39 1.75
+++ server/util_script.c2002/03/31 04:34:42
@@ -645,7 +645,7 @@
 if (*src == '\n') {
 done = 1;
 }
-else {
+else if (*src != '\r') {
 *dst++ = *src;
 }
 src++;

--
 __
 |  Brian Havard |  "He is not the messiah!   |
 |  [EMAIL PROTECTED]  |  He's a very naughty boy!" - Life of Brian |
 --

Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

[Jeff Trawick]

Thom May <[EMAIL PROTECTED]> writes:

> This change appears to have broken some cgі scripts - cricket
>  worked fine in 2.0.33 but in 2.0.34 and
> 35-dev:
> 
> [Sat Mar 30 15:20:03 2002] [error] [client 192.168.0.34] malformed header
> from 
: /usr/lib/cgi-bin/cricket/grapher.cgi
> 
> Cheers,
> -Thom

I don't really see how.  The code executed was completely unchanged
except when nph is set.  nph should only be set for a nph script --
one that starts with nph-.

That message "malformed header" makes me wonder if a recent change to
avoid reading one byte at a time broke it.  Look at Brian Pane's (I
think it was him) recent commit to change the way we read headers from
the CGI script.  Maybe there is a connection between that change and
the breakage of your cgi.

If you have a simple example I'm willing to debug.

> > trawick 02/03/20 05:57:08
> > 
> >   Modified:.CHANGES
> >modules/generators mod_cgi.c mod_cgid.c
> >   Log:
> >   Get nph- CGI scripts working again by avoiding filters that can't
> >   possibly deal with a script that generates the protocol header.
> >   
> >   PR:   8902, 8907, 9983

-- 
Jeff Trawick | [EMAIL PROTECTED]
Born in Roswell... married an alien...

Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

[Thom May]

This change appears to have broken some cgі scripts - cricket
 worked fine in 2.0.33 but in 2.0.34 and
35-dev:

[Sat Mar 30 15:20:03 2002] [error] [client 192.168.0.34] malformed header
from 
: /usr/lib/cgi-bin/cricket/grapher.cgi

Cheers,
-Thom

* [EMAIL PROTECTED] ([EMAIL PROTECTED]) wrote :
> trawick 02/03/20 05:57:08
> 
>   Modified:.CHANGES
>modules/generators mod_cgi.c mod_cgid.c
>   Log:
>   Get nph- CGI scripts working again by avoiding filters that can't
>   possibly deal with a script that generates the protocol header.
>   
>   PR:   8902, 8907, 9983
>   
>   Revision  ChangesPath
>   1.648 +3 -0  httpd-2.0/CHANGES
>   
>   Index: CHANGES
>   ===
>   RCS file: /home/cvs/httpd-2.0/CHANGES,v
>   retrieving revision 1.647
>   retrieving revision 1.648
>   diff -u -r1.647 -r1.648
>   --- CHANGES 20 Mar 2002 07:10:21 -  1.647
>   +++ CHANGES 20 Mar 2002 13:57:07 -  1.648
>   @@ -1,5 +1,8 @@
>Changes with Apache 2.0.34-dev
>
>   +  *) Get nph- CGI scripts working again.  PRs 8902, 8907, 9983
>   + [Jeff Trawick]
>   +
>  *) Upgraded PCRE library to latest version 3.9 [Brian Pane]
>
>  *) Add accessor function to set r->content_type. From now on,
>   
>   
>   
>   1.123 +13 -0 httpd-2.0/modules/generators/mod_cgi.c
>   
>   Index: mod_cgi.c
>   ===
>   RCS file: /home/cvs/httpd-2.0/modules/generators/mod_cgi.c,v
>   retrieving revision 1.122
>   retrieving revision 1.123
>   diff -u -r1.122 -r1.123
>   --- mod_cgi.c   20 Mar 2002 01:58:46 -  1.122
>   +++ mod_cgi.c   20 Mar 2002 13:57:08 -  1.123
>   @@ -739,6 +739,19 @@
>}
>
>if (script_in && nph) {
>   +struct ap_filter_t *cur;
>   +
>   +/* get rid of all filters up through protocol...  since we
>   + * haven't parsed off the headers, there is no way they can
>   + * work
>   + */
>   +
>   +cur = r->proto_output_filters;
>   +while (cur && cur->frec->ftype < AP_FTYPE_CONNECTION) {
>   +cur = cur->next;
>   +}
>   +r->output_filters = r->proto_output_filters = cur;
>   +
>bb = apr_brigade_create(r->pool);
>   b = apr_bucket_pipe_create(script_in);
>   APR_BRIGADE_INSERT_TAIL(bb, b);
>   
>   
>   
>   1.120 +13 -0 httpd-2.0/modules/generators/mod_cgid.c
>   
>   Index: mod_cgid.c
>   ===
>   RCS file: /home/cvs/httpd-2.0/modules/generators/mod_cgid.c,v
>   retrieving revision 1.119
>   retrieving revision 1.120
>   diff -u -r1.119 -r1.120
>   --- mod_cgid.c  20 Mar 2002 01:58:46 -  1.119
>   +++ mod_cgid.c  20 Mar 2002 13:57:08 -  1.120
>   @@ -1159,11 +1159,24 @@
>} 
>
>if (nph) {
>   +struct ap_filter_t *cur;
>   +
>/* Passing our socket down the filter chain in a pipe bucket
> * gives up the responsibility of closing the socket, so
> * get rid of the cleanup.
> */
>apr_pool_cleanup_kill(r->pool, (void *)sd, close_unix_socket);
>   +
>   +/* get rid of all filters up through protocol...  since we
>   + * haven't parsed off the headers, there is no way they can
>   + * work
>   + */
>   +
>   +cur = r->proto_output_filters;
>   +while (cur && cur->frec->ftype < AP_FTYPE_CONNECTION) {
>   +cur = cur->next;
>   +}
>   +r->output_filters = r->proto_output_filters = cur;
>
>bb = apr_brigade_create(r->pool);
>b = apr_bucket_pipe_create(tempsock);
>   
>   
>   

Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

[Jeff Trawick]

[EMAIL PROTECTED] writes:

>   Index: mod_include.c
>   ===
>   RCS file: /home/cvs/httpd-2.0/modules/filters/mod_include.c,v
>   retrieving revision 1.128
>   retrieving revision 1.129
>   diff -u -r1.128 -r1.129
>   --- mod_include.c   2001/08/23 15:35:50 1.128
>   +++ mod_include.c   2001/08/23 15:45:49 1.129
>   @@ -98,6 +98,29 @@
>
>#define BYTE_COUNT_THRESHOLD AP_MIN_BYTES_TO_WRITE
>
>   +/* This function is used to split the brigade at the beginning of
>   + *   the tag and forward the pretag buckets before any substitution
>   + *   work is performed on the tag. This maintains proper ordering.
>   + */
>   +static int split_and_pass_pretag_buckets(apr_bucket_brigade **brgd, 

should be "apr_status_t" instead of "int", right?

>   + include_ctx_t *cntxt, 
>   + ap_filter_t *next)
>   +{
>   +apr_bucket_brigade *tag_plus;
>   +int rv;

apr_status_t rv;

>   +
>   +if ((APR_BRIGADE_EMPTY(cntxt->ssi_tag_brigade)) &&
>   +(cntxt->head_start_bucket != NULL)) {
>   +tag_plus = apr_brigade_split(*brgd, cntxt->head_start_bucket);
>   +rv = ap_pass_brigade(next, *brgd);
>   +cntxt->bytes_parsed = 0;
>   +*brgd = tag_plus;
>   +if (rv != APR_SUCCESS) {
>   +return rv;
>   +}
>   +}

besides not compiling cleanly this can't work dependably... you need
"return APR_SUCCESS;" here at the end of the function...

and then there is the issue Ryan raised...

-- 
Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site:
   http://www.geocities.com/SiliconValley/Park/9289/
 Born in Roswell... married an alien...

Re: cvs commit: httpd-2.0/modules/generators mod_cgi.c mod_cgid.c

[Ryan Bloom]

-1.

You just took a small macro, and turned it into a function, and copied that
function into three different places in the source tree.  That's just asking
for future bugs.


Ryan
__
Ryan Bloom  [EMAIL PROTECTED]
Covalent Technologies   [EMAIL PROTECTED]
--