You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Marc Slemko <ma...@worldgate.com> on 1998/09/24 00:47:32 UTC

Re: cvs commit: apache-1.3/src/modules/standard mod_speling.c

I'm a bit confused; why should modules have to do anything special to keep
some obscene thing from taking over their output?  Just because something
logs something to the error log and uses one of Apache's output forms
doesn't mean what they log should be sent to the client!

I also didn't notice that this sort of thing is now sent to the client by
default.  Is there even a way to disable it?  That is a bad thing to
just start doing all the time from a security viewpoint.

On 23 Sep 1998 manoj@hyperreal.org wrote:

> manoj       98/09/23 15:42:06
> 
>   Modified:    src      CHANGES
>                src/modules/standard mod_speling.c
>   Log:
>   PR: 3052
>   Reviewed by:	Ken Coar <co...@apache.org>
>   
>   Fix mod_speling's handling of multiple matched URLs.
>   
>   Revision  Changes    Path
>   1.1073    +3 -0      apache-1.3/src/CHANGES
>   
>   Index: CHANGES
>   ===================================================================
>   RCS file: /export/home/cvs/apache-1.3/src/CHANGES,v
>   retrieving revision 1.1072
>   retrieving revision 1.1073
>   diff -u -r1.1072 -r1.1073
>   --- CHANGES	1998/09/22 09:19:51	1.1072
>   +++ CHANGES	1998/09/23 22:42:03	1.1073
>   @@ -1,5 +1,8 @@
>    Changes with Apache 1.3.3
>    
>   +  *) Fix mod_speling's handling of multiple matched URLs.
>   +     [Manoj Kasichainula]
>   +
>      *) Correct comment in mod_log_config.c about its internals.
>         [Elf Sternberg <el...@halcyon.com>]
>    
>   
>   
>   
>   1.27      +4 -0      apache-1.3/src/modules/standard/mod_speling.c
>   
>   Index: mod_speling.c
>   ===================================================================
>   RCS file: /export/home/cvs/apache-1.3/src/modules/standard/mod_speling.c,v
>   retrieving revision 1.26
>   retrieving revision 1.27
>   diff -u -r1.26 -r1.27
>   --- mod_speling.c	1998/09/19 12:16:38	1.26
>   +++ mod_speling.c	1998/09/23 22:42:05	1.27
>   @@ -506,6 +506,10 @@
>    			     : "Spelling fix: %s: %d candidates",
>    			 r->uri, candidates->nelts, ref);
>    
>   +            /* The log entry above will be sent to the client instead of the
>   +             * page we've constructed unless we take it out of error-notes */
>   +            ap_table_unset(r->notes, "error-notes");
>   +
>                return HTTP_MULTIPLE_CHOICES;
>            }
>        }
>   
>   
>   
> 


Re: cvs commit: apache-1.3/src/modules/standard mod_speling.c

Posted by Dean Gaudet <dg...@arctic.org>.

On Thu, 24 Sep 1998, Rodent of Unusual Size wrote:

> I'll go along with that, with the clarification that the
> brokennes
> is in ap_send_error_response()'s use of error-notes, not with the
> error-notes mechanism itself.

Right.

> So.. two possible solutions (among others) present themselves:
> modify the way ap_send_error_response() treats error-notes,
> and/or not set error-notes in ap_log_rerror() unless the
> level is higher than N (pick N).

I think both.  I don't think debugging messages, for example, should be
put into "error-notes". 

> For now I prefer the former, since doing the latter properly
> will involve recording the level of the last error stored in
> error-notes so that a subsequent call can override it if the
> error level is more severe.  (This addresses Doug's concerns.)

This is a problem no matter which of the two (three?) solutions are used.

Dean


Re: cvs commit: apache-1.3/src/modules/standard mod_speling.c

Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Dean Gaudet wrote:
> 
> 300s aren't "errors".

I used the term advisedly because the routine in question is
ap_send_error_response().

> This error-notes thing is broken imnsho.  All of a sudden all the useful,
> informative error messages are completely overridden by terse log
> messages... this is a decrease in useability.  If error-notes was sent *in
> addition* to the age old responses I'd be a lot less troubled.

I'll go along with that, with the clarification that the
brokennes
is in ap_send_error_response()'s use of error-notes, not with the
error-notes mechanism itself.

So.. two possible solutions (among others) present themselves:
modify the way ap_send_error_response() treats error-notes,
and/or not set error-notes in ap_log_rerror() unless the
level is higher than N (pick N).

For now I prefer the former, since doing the latter properly
will involve recording the level of the last error stored in
error-notes so that a subsequent call can override it if the
error level is more severe.  (This addresses Doug's concerns.)

#ken    P-)}

Ken Coar                    <http://Web.Golux.Com/coar/>
Apache Group member         <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>

Re: cvs commit: apache-1.3/src/modules/standard mod_speling.c

Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Dean Gaudet wrote:
> 
> This error-notes thing is broken imnsho.  All of a sudden all the useful,
> informative error messages are completely overridden by terse log
> messages... this is a decrease in useability.  If error-notes was sent *in
> addition* to the age old responses I'd be a lot less troubled.

I think the error-notes confusion is due to some unfortunate
overloading.  On the one hand, anything in that cell is made
available as an envariable for error processing.  On the other
hand, ap_send_error_response() has treated it as a way for
modules to override the default error message.

Both were evidently intended by the change when it was
implemented in May.  I suspect that the correct solution
is to simply remove its special meaning in
ap_send_error_response().
If someone wants to make use of the string in an error display,
let 'im use an ErrorDocument to customise the response.

The more I think about this the more I like that solution.
Currently, any module can usurp the text of the error
display page from the core.  I don't think that's appropriate.

#ken    P-)}

Ken Coar                    <http://Web.Golux.Com/coar/>
Apache Group member         <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>

Re: cvs commit: apache-1.3/src/modules/standard mod_speling.c

Posted by Marc Slemko <ma...@worldgate.com>.
On Thu, 24 Sep 1998, Dean Gaudet wrote:

> 300s aren't "errors". 

Exactly.  There are so many cases where you may want to log something and
send a "auto generated" response without having it filled with this.

Also, the major security issue of revealing all sorts of information that
formerly was private in the logs isn't dealt with.  That and not giving
anyone a way to disable it.

> 
> This error-notes thing is broken imnsho.  All of a sudden all the useful,
> informative error messages are completely overridden by terse log
> messages... this is a decrease in useability.  If error-notes was sent *in
> addition* to the age old responses I'd be a lot less troubled.
> 
> Dean
> 
> On Wed, 23 Sep 1998, Rodent of Unusual Size wrote:
> 
> > Marc Slemko wrote:
> > > 
> > > I'm a bit confused; why should modules have to do anything special to keep
> > > some obscene thing from taking over their output?  Just because something
> > > logs something to the error log and uses one of Apache's output forms
> > > doesn't mean what they log should be sent to the client!
> > > 
> > > I also didn't notice that this sort of thing is now sent to the client by
> > > default.  Is there even a way to disable it?  That is a bad thing to
> > > just start doing all the time from a security viewpoint.
> > 
> > mod_negotiation and mod_speling use a back door to let
> > ap_send_error_response() know about a variant list.
> > The back door involves putting constructed HTML into the
> > r->notes("variant-list") cell.  If ap_send_error_response()
> > determines that it's processing a 300 error, AND there's
> > a value in that cell, it will construct the error message's
> > content-body from the value.
> > 
> > On the other hand, when ap_send_error_response() starts
> > processing
> > an error, it checks for something in r->notes("error-notes").
> > If it finds something, it uses it in the construction of the
> > content-body of the error page.  This short-circuits the
> > special variant processing code.
> > 
> > So it's not the module's output, directly; it's a hint given
> > to the error handler for that specific error.  As it happens,
> > the general case was dominating the specific.  Manoj's patch
> > just restores things, although mod_negotiation may need a
> > similar patch if it calls ap_log_rerror().  (Haven't checked.)
> > The two simplest possibilities were this route, or to remove
> > the call to ap_log_rerror() in mod_speling altogether.  This
> > seemed the better.
> > 
> > #ken    P-)}
> > 
> > Ken Coar                    <http://Web.Golux.Com/coar/>
> > Apache Group member         <http://www.apache.org/>
> > "Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
> > 
> 


Re: cvs commit: apache-1.3/src/modules/standard mod_speling.c

Posted by Dean Gaudet <dg...@arctic.org>.
300s aren't "errors". 

This error-notes thing is broken imnsho.  All of a sudden all the useful,
informative error messages are completely overridden by terse log
messages... this is a decrease in useability.  If error-notes was sent *in
addition* to the age old responses I'd be a lot less troubled.

Dean

On Wed, 23 Sep 1998, Rodent of Unusual Size wrote:

> Marc Slemko wrote:
> > 
> > I'm a bit confused; why should modules have to do anything special to keep
> > some obscene thing from taking over their output?  Just because something
> > logs something to the error log and uses one of Apache's output forms
> > doesn't mean what they log should be sent to the client!
> > 
> > I also didn't notice that this sort of thing is now sent to the client by
> > default.  Is there even a way to disable it?  That is a bad thing to
> > just start doing all the time from a security viewpoint.
> 
> mod_negotiation and mod_speling use a back door to let
> ap_send_error_response() know about a variant list.
> The back door involves putting constructed HTML into the
> r->notes("variant-list") cell.  If ap_send_error_response()
> determines that it's processing a 300 error, AND there's
> a value in that cell, it will construct the error message's
> content-body from the value.
> 
> On the other hand, when ap_send_error_response() starts
> processing
> an error, it checks for something in r->notes("error-notes").
> If it finds something, it uses it in the construction of the
> content-body of the error page.  This short-circuits the
> special variant processing code.
> 
> So it's not the module's output, directly; it's a hint given
> to the error handler for that specific error.  As it happens,
> the general case was dominating the specific.  Manoj's patch
> just restores things, although mod_negotiation may need a
> similar patch if it calls ap_log_rerror().  (Haven't checked.)
> The two simplest possibilities were this route, or to remove
> the call to ap_log_rerror() in mod_speling altogether.  This
> seemed the better.
> 
> #ken    P-)}
> 
> Ken Coar                    <http://Web.Golux.Com/coar/>
> Apache Group member         <http://www.apache.org/>
> "Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
> 


Re: cvs commit: apache-1.3/src/modules/standard mod_speling.c

Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Marc Slemko wrote:
> 
> I'm a bit confused; why should modules have to do anything special to keep
> some obscene thing from taking over their output?  Just because something
> logs something to the error log and uses one of Apache's output forms
> doesn't mean what they log should be sent to the client!
> 
> I also didn't notice that this sort of thing is now sent to the client by
> default.  Is there even a way to disable it?  That is a bad thing to
> just start doing all the time from a security viewpoint.

mod_negotiation and mod_speling use a back door to let
ap_send_error_response() know about a variant list.
The back door involves putting constructed HTML into the
r->notes("variant-list") cell.  If ap_send_error_response()
determines that it's processing a 300 error, AND there's
a value in that cell, it will construct the error message's
content-body from the value.

On the other hand, when ap_send_error_response() starts
processing
an error, it checks for something in r->notes("error-notes").
If it finds something, it uses it in the construction of the
content-body of the error page.  This short-circuits the
special variant processing code.

So it's not the module's output, directly; it's a hint given
to the error handler for that specific error.  As it happens,
the general case was dominating the specific.  Manoj's patch
just restores things, although mod_negotiation may need a
similar patch if it calls ap_log_rerror().  (Haven't checked.)
The two simplest possibilities were this route, or to remove
the call to ap_log_rerror() in mod_speling altogether.  This
seemed the better.

#ken    P-)}

Ken Coar                    <http://Web.Golux.Com/coar/>
Apache Group member         <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>