You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by br...@hyperreal.org on 1998/05/20 06:22:11 UTC
cvs commit: apache-1.3/src/modules/standard mod_log_referer.c
brian 98/05/19 21:22:11
Modified: src CHANGES
src/modules/standard mod_log_referer.c
Log:
PR: 1947
Made RefererIgnore comparison case-insensitive
Revision Changes Path
1.854 +2 -0 apache-1.3/src/CHANGES
Index: CHANGES
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/CHANGES,v
retrieving revision 1.853
retrieving revision 1.854
diff -u -r1.853 -r1.854
--- CHANGES 1998/05/19 22:48:57 1.853
+++ CHANGES 1998/05/20 04:22:06 1.854
@@ -1,5 +1,7 @@
Changes with Apache 1.3b7
+ *) Made RefererIgnore case-insensitive.
+
*) Mod_log_agent, mod_log_referer now use ap_open_piped_log for piped logs.
[Brian Behlendorf]
1.28 +2 -1 apache-1.3/src/modules/standard/mod_log_referer.c
Index: mod_log_referer.c
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/modules/standard/mod_log_referer.c,v
retrieving revision 1.27
retrieving revision 1.28
diff -u -r1.27 -r1.28
--- mod_log_referer.c 1998/05/19 23:48:36 1.27
+++ mod_log_referer.c 1998/05/20 04:22:11 1.28
@@ -104,6 +104,7 @@
&referer_log_module);
addme = ap_push_array(cls->referer_ignore_list);
+ ap_str_tolower(arg);
*addme = arg;
return NULL;
}
@@ -175,7 +176,7 @@
referer = ap_table_get(orig->headers_in, "Referer");
if (referer != NULL) {
-
+ ap_str_tolower(referer);
/* The following is an upsetting mess of pointers, I'm sorry
Anyone with the motiviation and/or the time should feel free
to make this cleaner... */
Re: cvs commit: apache-1.3/src/modules/standard
mod_log_referer.c
Posted by Brian Behlendorf <br...@hyperreal.org>.
I just committed a far better patch. Sorry about that.
At 01:26 PM 5/20/98 -0700, you wrote:
>The comparison is a strstr though; so far as I know there isn't a
>strcasestr available. I suppose I can use regex... or just create a
>new buffer.
>
>Odd that the patch seemed to work fine, no compile errors and testing
>showed it did the job...
>
> Brian
>
>
>
>
--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
pure chewing satisfaction brian@apache.org
brian@hyperreal.org
Re: cvs commit: apache-1.3/src/modules/standard
mod_log_referer.c
Posted by Brian Behlendorf <br...@hyperreal.org>.
At 01:32 PM 5/21/98 +0100, Ben Laurie wrote:
>Ben Laurie wrote:
>> And fixing that produces this alarmingly large patch, that surprisingly
>> reveals no other bugs, but may prevent them in future.
>
>Tell a lie - there was a bug in mod_proxy, which modified something
>returned by table_get...
Hmm, yes, seems like the current code would convert the date as an object
passes through a proxy, probably not good. +1, feel free to commit.
>> ===================================================================
>> RCS file: /export/home/cvs/apache-1.3/src/modules/proxy/proxy_util.c,v
>> retrieving revision 1.60
>> diff -r1.60 proxy_util.c
>> 307,308c307,308
>> < char *
>> < ap_proxy_date_canon(pool *p, char *x)
>> ---
>> > const char *
>> > ap_proxy_date_canon(pool *p, const char *x)
>> 356,358c356,357
>> < if (strlen(x)+1 < 30)
>> < x = ap_palloc(p, 30);
>> < ap_snprintf(x, 30, "%s, %.2d %s %d %.2d:%.2d:%.2d GMT",
ap_day_snames[wk], mday,
>> ---
>> > q = ap_palloc(p, 30);
>> > ap_snprintf(q, 30, "%s, %.2d %s %d %.2d:%.2d:%.2d GMT",
ap_day_snames[wk], mday,
>> 360c359
>> < return x;
>> ---
>> > return q;
--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
pure chewing satisfaction brian@apache.org
brian@hyperreal.org
Re: cvs commit: apache-1.3/src/modules/standard mod_log_referer.c
Posted by Ben Laurie <be...@algroup.co.uk>.
Ben Laurie wrote:
> And fixing that produces this alarmingly large patch, that surprisingly
> reveals no other bugs, but may prevent them in future.
Tell a lie - there was a bug in mod_proxy, which modified something
returned by table_get...
> ===================================================================
> RCS file: /export/home/cvs/apache-1.3/src/modules/proxy/proxy_util.c,v
> retrieving revision 1.60
> diff -r1.60 proxy_util.c
> 307,308c307,308
> < char *
> < ap_proxy_date_canon(pool *p, char *x)
> ---
> > const char *
> > ap_proxy_date_canon(pool *p, const char *x)
> 356,358c356,357
> < if (strlen(x)+1 < 30)
> < x = ap_palloc(p, 30);
> < ap_snprintf(x, 30, "%s, %.2d %s %d %.2d:%.2d:%.2d GMT", ap_day_snames[wk], mday,
> ---
> > q = ap_palloc(p, 30);
> > ap_snprintf(q, 30, "%s, %.2d %s %d %.2d:%.2d:%.2d GMT", ap_day_snames[wk], mday,
> 360c359
> < return x;
> ---
> > return q;
Sorry 'bout the diff format, BTW - dunno what came over me!
Cheers,
Ben.
--
Ben Laurie |Phone: +44 (181) 735 0686| Apache Group member
Freelance Consultant |Fax: +44 (181) 735 0689|http://www.apache.org
and Technical Director|Email: ben@algroup.co.uk |
A.L. Digital Ltd, |Apache-SSL author http://www.apache-ssl.org/
London, England. |"Apache: TDG" http://www.ora.com/catalog/apache
Re: cvs commit: apache-1.3/src/modules/standard
mod_log_referer.c
Posted by Ben Laurie <be...@algroup.co.uk>.
Brian Behlendorf wrote:
>
> At 11:20 PM 5/20/98 +0100, ben wrote:
> >And fixing that produces this alarmingly large patch, that surprisingly
> >reveals no other bugs, but may prevent them in future.
>
> Let's not worry about it right now then. I'll just do a pstrdup and be
> done with it.
Like I said later, it actually did reveal a bug, I'd just forgotten
about it. I'd be happier if we committed it.
> >In fact, it doesn't even reveal this bug, coz I did it under Win32 which
> >doesn't include mod_log_referer - is that a bug?
>
> mod_log_referer is on the list of modules in STATUS called "modules that
> need to be made to work on win32". What doesn't work about it? Or does
> it? Maybe the other ones on that list could be looked at too. Hmm, I know
> we've had bug reports for mod_info on win32...
I suspect that all that doesn't work is that it hasn't had a makefile
made for it.
mod_info is slightly trickier, I seem to remember.
Cheers,
Ben.
--
Ben Laurie |Phone: +44 (181) 735 0686| Apache Group member
Freelance Consultant |Fax: +44 (181) 735 0689|http://www.apache.org
and Technical Director|Email: ben@algroup.co.uk |
A.L. Digital Ltd, |Apache-SSL author http://www.apache-ssl.org/
London, England. |"Apache: TDG" http://www.ora.com/catalog/apache
Re: cvs commit: apache-1.3/src/modules/standard
mod_log_referer.c
Posted by Brian Behlendorf <br...@hyperreal.org>.
At 11:20 PM 5/20/98 +0100, ben wrote:
>And fixing that produces this alarmingly large patch, that surprisingly
>reveals no other bugs, but may prevent them in future.
Let's not worry about it right now then. I'll just do a pstrdup and be
done with it.
>In fact, it doesn't even reveal this bug, coz I did it under Win32 which
>doesn't include mod_log_referer - is that a bug?
mod_log_referer is on the list of modules in STATUS called "modules that
need to be made to work on win32". What doesn't work about it? Or does
it? Maybe the other ones on that list could be looked at too. Hmm, I know
we've had bug reports for mod_info on win32...
Brian
--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
pure chewing satisfaction brian@apache.org
brian@hyperreal.org
Re: cvs commit: apache-1.3/src/modules/standard mod_log_referer.c
Posted by Ben Laurie <be...@algroup.co.uk>.
Ben Laurie wrote:
>
> Brian Behlendorf wrote:
> >
> > On Wed, 20 May 1998, Dean Gaudet wrote:
> > > On 20 May 1998 brian@hyperreal.org wrote:
> > >
> > > > @@ -175,7 +176,7 @@
> > > > referer = ap_table_get(orig->headers_in, "Referer");
> > > > if (referer != NULL) {
> > > >
> > > > -
> > > > + ap_str_tolower(referer);
> > > > /* The following is an upsetting mess of pointers, I'm sorry
> > > > Anyone with the motiviation and/or the time should feel free
> > > > to make this cleaner... */
> > >
> > > -1. This patch is broken. You cannot downcase the value you get from
> > > table_get()... the values in all tables are constants. You should replace
> > > the to str_tolower()s with a single strcasecmp() or whatever.
> >
> > The comparison is a strstr though; so far as I know there isn't a
> > strcasestr available. I suppose I can use regex... or just create a
> > new buffer.
> >
> > Odd that the patch seemed to work fine, no compile errors and testing
> > showed it did the job...
>
> That's coz ap_table_get() returns a char * instead of a const char *, I
> guess.
And fixing that produces this alarmingly large patch, that surprisingly
reveals no other bugs, but may prevent them in future.
In fact, it doesn't even reveal this bug, coz I did it under Win32 which
doesn't include mod_log_referer - is that a bug?
Cheers,
Ben.
--
Ben Laurie |Phone: +44 (181) 735 0686| Apache Group member
Freelance Consultant |Fax: +44 (181) 735 0689|http://www.apache.org
and Technical Director|Email: ben@algroup.co.uk |
A.L. Digital Ltd, |Apache-SSL author http://www.apache-ssl.org/
London, England. |"Apache: TDG" http://www.ora.com/catalog/apache
Re: cvs commit: apache-1.3/src/modules/standard mod_log_referer.c
Posted by Ben Laurie <be...@algroup.co.uk>.
Brian Behlendorf wrote:
>
> On Wed, 20 May 1998, Dean Gaudet wrote:
> > On 20 May 1998 brian@hyperreal.org wrote:
> >
> > > @@ -175,7 +176,7 @@
> > > referer = ap_table_get(orig->headers_in, "Referer");
> > > if (referer != NULL) {
> > >
> > > -
> > > + ap_str_tolower(referer);
> > > /* The following is an upsetting mess of pointers, I'm sorry
> > > Anyone with the motiviation and/or the time should feel free
> > > to make this cleaner... */
> >
> > -1. This patch is broken. You cannot downcase the value you get from
> > table_get()... the values in all tables are constants. You should replace
> > the to str_tolower()s with a single strcasecmp() or whatever.
>
> The comparison is a strstr though; so far as I know there isn't a
> strcasestr available. I suppose I can use regex... or just create a
> new buffer.
>
> Odd that the patch seemed to work fine, no compile errors and testing
> showed it did the job...
That's coz ap_table_get() returns a char * instead of a const char *, I
guess.
Cheers,
Ben.
--
Ben Laurie |Phone: +44 (181) 735 0686| Apache Group member
Freelance Consultant |Fax: +44 (181) 735 0689|http://www.apache.org
and Technical Director|Email: ben@algroup.co.uk |
A.L. Digital Ltd, |Apache-SSL author http://www.apache-ssl.org/
London, England. |"Apache: TDG" http://www.ora.com/catalog/apache
Re: cvs commit: apache-1.3/src/modules/standard mod_log_referer.c
Posted by Marc Slemko <ma...@znep.com>.
On Wed, 20 May 1998, Brian Behlendorf wrote:
>
>
> On Wed, 20 May 1998, Dean Gaudet wrote:
> > On 20 May 1998 brian@hyperreal.org wrote:
> >
> > > @@ -175,7 +176,7 @@
> > > referer = ap_table_get(orig->headers_in, "Referer");
> > > if (referer != NULL) {
> > >
> > > -
> > > + ap_str_tolower(referer);
> > > /* The following is an upsetting mess of pointers, I'm sorry
> > > Anyone with the motiviation and/or the time should feel free
> > > to make this cleaner... */
> >
> > -1. This patch is broken. You cannot downcase the value you get from
> > table_get()... the values in all tables are constants. You should replace
> > the to str_tolower()s with a single strcasecmp() or whatever.
>
> The comparison is a strstr though; so far as I know there isn't a
> strcasestr available. I suppose I can use regex... or just create a
> new buffer.
>
> Odd that the patch seemed to work fine, no compile errors and testing
> showed it did the job...
Except that if you check the referer after the fact in some other module,
etc. you will find that it has been modified. That is bad.
Re: const stuff (was Re: cvs commit: apache-1.3/src/modules/standard mod_log_referer.c)
Posted by Ben Laurie <be...@algroup.co.uk>.
Dean Gaudet wrote:
>
> Yeah I kind of figured this out later while I was thinking about it
> offline. Everyone else pointed out stuff that I would have -- Marc
> mentioned the main problem, that you're modifying the referer result for
> other callers.
>
> Ben L. provided a const patch... I actually did this earlier this year,
> but threw it away after I got about an hour into the work. I figured you
> all would complain to high-heaven if I made such a huge change.
I wouldn't have complained! In fact, I'd still like to put it into 1.3.
> My goal
> was to make apache compile with gcc -Wwrite-strings cleanly. Another
> reason for dropping it was the reason Ben H. cited -- that it's hard to
> nail it on all platforms.
Like I said before, I'd rather fix the platforms, one way or another,
than give up on correct constness.
> .... but I could give it a try with apache-nspr
> if folks are into this huge of a change.
Sure.
Cheers,
Ben.
--
Ben Laurie |Phone: +44 (181) 735 0686| Apache Group member
Freelance Consultant |Fax: +44 (181) 735 0689|http://www.apache.org
and Technical Director|Email: ben@algroup.co.uk |
A.L. Digital Ltd, |Apache-SSL author http://www.apache-ssl.org/
London, England. |"Apache: TDG" http://www.ora.com/catalog/apache
const stuff (was Re: cvs commit: apache-1.3/src/modules/standard mod_log_referer.c)
Posted by Dean Gaudet <dg...@arctic.org>.
Yeah I kind of figured this out later while I was thinking about it
offline. Everyone else pointed out stuff that I would have -- Marc
mentioned the main problem, that you're modifying the referer result for
other callers.
Ben L. provided a const patch... I actually did this earlier this year,
but threw it away after I got about an hour into the work. I figured you
all would complain to high-heaven if I made such a huge change. My goal
was to make apache compile with gcc -Wwrite-strings cleanly. Another
reason for dropping it was the reason Ben H. cited -- that it's hard to
nail it on all platforms. .... but I could give it a try with apache-nspr
if folks are into this huge of a change.
Personally I think that all code should pass:
-Wall -O -g note that -O adds to what gcc can detect, and on
x86 at least, gcc is so poor that you can easily
debug -O code
-fno-common
not all compilers do the bastardization that many
unix compilers do with "common"-style declarations
(think fortran). In particular windows compilers
don't do this.
-Wshadow
-Wmissing-prototypes
we pass all of the above on Linux libc5 and glibc2
right now, all modules
The rest we don't quite pass yet:
-Wpointer-arith
even better would be to just use -ansi and use
the __ form of things like __inline__, and
__attribute__
-Wcast-qual
-Wwrite-strings
we get nailed because of const issues like those
above
-Wstrict-prototypes
right now the only reason we can't use this is
because of the lame function pointer without
prototype in command_rec -- the correct fix
can be implemented during an API overhaul.
I run with this occasionally and just filter
the http_config.h warning out.
-Wnested-externs
Only .h files should declare externs -- everything
that is an exported interface should be declared
in a .h file.
Dean
On Wed, 20 May 1998, Brian Behlendorf wrote:
>
>
> On Wed, 20 May 1998, Dean Gaudet wrote:
> > On 20 May 1998 brian@hyperreal.org wrote:
> >
> > > @@ -175,7 +176,7 @@
> > > referer = ap_table_get(orig->headers_in, "Referer");
> > > if (referer != NULL) {
> > >
> > > -
> > > + ap_str_tolower(referer);
> > > /* The following is an upsetting mess of pointers, I'm sorry
> > > Anyone with the motiviation and/or the time should feel free
> > > to make this cleaner... */
> >
> > -1. This patch is broken. You cannot downcase the value you get from
> > table_get()... the values in all tables are constants. You should replace
> > the to str_tolower()s with a single strcasecmp() or whatever.
>
> The comparison is a strstr though; so far as I know there isn't a
> strcasestr available. I suppose I can use regex... or just create a
> new buffer.
>
> Odd that the patch seemed to work fine, no compile errors and testing
> showed it did the job...
>
> Brian
>
>
>
>
Re: cvs commit: apache-1.3/src/modules/standard mod_log_referer.c
Posted by Brian Behlendorf <br...@hyperreal.org>.
On Wed, 20 May 1998, Dean Gaudet wrote:
> On 20 May 1998 brian@hyperreal.org wrote:
>
> > @@ -175,7 +176,7 @@
> > referer = ap_table_get(orig->headers_in, "Referer");
> > if (referer != NULL) {
> >
> > -
> > + ap_str_tolower(referer);
> > /* The following is an upsetting mess of pointers, I'm sorry
> > Anyone with the motiviation and/or the time should feel free
> > to make this cleaner... */
>
> -1. This patch is broken. You cannot downcase the value you get from
> table_get()... the values in all tables are constants. You should replace
> the to str_tolower()s with a single strcasecmp() or whatever.
The comparison is a strstr though; so far as I know there isn't a
strcasestr available. I suppose I can use regex... or just create a
new buffer.
Odd that the patch seemed to work fine, no compile errors and testing
showed it did the job...
Brian
Re: cvs commit: apache-1.3/src/modules/standard mod_log_referer.c
Posted by Dean Gaudet <dg...@arctic.org>.
On 20 May 1998 brian@hyperreal.org wrote:
> @@ -175,7 +176,7 @@
> referer = ap_table_get(orig->headers_in, "Referer");
> if (referer != NULL) {
>
> -
> + ap_str_tolower(referer);
> /* The following is an upsetting mess of pointers, I'm sorry
> Anyone with the motiviation and/or the time should feel free
> to make this cleaner... */
-1. This patch is broken. You cannot downcase the value you get from
table_get()... the values in all tables are constants. You should replace
the to str_tolower()s with a single strcasecmp() or whatever.
Dean