You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ben Laurie <be...@algroup.co.uk> on 2001/01/09 10:37:17 UTC
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
dougm@apache.org wrote:
> Index: mod_file_cache.c
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/cache/mod_file_cache.c,v
> retrieving revision 1.34
> retrieving revision 1.35
> diff -u -r1.34 -r1.35
> --- mod_file_cache.c 2000/12/13 13:22:51 1.34
> +++ mod_file_cache.c 2001/01/08 23:54:47 1.35
> @@ -422,6 +422,10 @@
> int errstatus;
> int rc = OK;
>
> + if (strcmp(r->handler, "*.*")) {
> + return DECLINED;
> + }
> +
As extensively discussed, this is _wrong_.
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html
"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Posted by Ben Laurie <be...@algroup.co.uk>.
Doug MacEachern wrote:
>
> On Tue, 9 Jan 2001, Ben Laurie wrote:
> > >
> > > + if (strcmp(r->handler, "*.*")) {
> > > + return DECLINED;
> > > + }
> > > +
> >
> > As extensively discussed, this is _wrong_.
>
> yep and if you had looked at the next commit in your mailbox before
> shouting, you'd have seen its changed to use ap_strcmp_match().
Did I shout? :-)
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html
"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Posted by Ben Laurie <be...@algroup.co.uk>.
Doug MacEachern wrote:
>
> On Tue, 9 Jan 2001, Ben Laurie wrote:
> > >
> > > + if (strcmp(r->handler, "*.*")) {
> > > + return DECLINED;
> > > + }
> > > +
> >
> > As extensively discussed, this is _wrong_.
>
> yep and if you had looked at the next commit in your mailbox before
> shouting, you'd have seen its changed to use ap_strcmp_match().
Did I shout? :-)
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html
"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Posted by Doug MacEachern <do...@covalent.net>.
On Tue, 9 Jan 2001, Ben Laurie wrote:
> >
> > + if (strcmp(r->handler, "*.*")) {
> > + return DECLINED;
> > + }
> > +
>
> As extensively discussed, this is _wrong_.
yep and if you had looked at the next commit in your mailbox before
shouting, you'd have seen its changed to use ap_strcmp_match().
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Posted by Ben Laurie <be...@algroup.co.uk>.
rbb@covalent.net wrote:
>
> > > + if (strcmp(r->handler, "*.*")) {
> > > + return DECLINED;
> > > + }
> > > +
> >
> > As extensively discussed, this is _wrong_.
>
> Jon Travis pointed out to me yesterday that this is wrong in more ways
> than one. The problem is that before the change, Apache would run all the
> handlers that registered specific handler types, like DIR_MAGIC_TYPE or
> server-parsed-html before it ran any handler that registered generic
> handler types, ones with *'s in them.
>
> This behavior is now gone, which is a bad thing IMHO. This means that a
> module that matches multiple handler types, only gets one chance to
> matche them all. I didn't think this was important yesterday, but I woke
> up this morning, and I believe it is. Somebody tell me I'm wrong
> please. I just want somebody else to say I am wrong. :-)
No, you are right ... but ... read the @tip:
* @tip non-wildcard handlers should HOOK_MIDDLE, wildcard HOOK_LAST
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html
"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Posted by Ben Laurie <be...@algroup.co.uk>.
rbb@covalent.net wrote:
>
> > > + if (strcmp(r->handler, "*.*")) {
> > > + return DECLINED;
> > > + }
> > > +
> >
> > As extensively discussed, this is _wrong_.
>
> Jon Travis pointed out to me yesterday that this is wrong in more ways
> than one. The problem is that before the change, Apache would run all the
> handlers that registered specific handler types, like DIR_MAGIC_TYPE or
> server-parsed-html before it ran any handler that registered generic
> handler types, ones with *'s in them.
>
> This behavior is now gone, which is a bad thing IMHO. This means that a
> module that matches multiple handler types, only gets one chance to
> matche them all. I didn't think this was important yesterday, but I woke
> up this morning, and I believe it is. Somebody tell me I'm wrong
> please. I just want somebody else to say I am wrong. :-)
No, you are right ... but ... read the @tip:
* @tip non-wildcard handlers should HOOK_MIDDLE, wildcard HOOK_LAST
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html
"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Posted by Ben Laurie <be...@algroup.co.uk>.
rbb@covalent.net wrote:
>
> > I was mulling the same issue over yesterday. It seems to me that we
> > _should_ be giving any specific match first shot. One way to do
> > this is to specify AP_HOOK_LAST for any handlers that have wildcards.
> > Question though, do we need to ensure foo/* get its shot before */* ?
>
> A) Specifying AP_HOOK_LAST won't work unless we separate the checks into
> two functions, so that we have something like:
>
> ap_hook_1(...)
> {
> if(!strcmp(SPECIFIC, r->handler)
> real_handler
> }
>
> ap_hook_2(...)
> {
> if (!strcmp_match(GENERAL, r->handler)
> real_handler
> }
>
> real_handler(...)
> {
> ...
> }
>
> ap_hook_handler(ap_hook_1, NULL, NULL, AP_HOOK_MIDDLE)
> ap_hook_handler(ap_hook_2, NULL, NULL, AP_HOOK_LAST)
Although this is nominally correct, I have found _no instances_ where it
actually happens. The only one I can find is the default handler, which
handles "default-handler" or "*/*" and that clearly can come last. Hmm.
Actually I suppose you might want to force the "real" default handler
before some other default handler, but that really is stretching a
point.
> While this will work, it is really bad, and I seriously dislike it, a lot.
Why is it bad? At least it is clear (as well as rare).
> b) We never used to distinguish between foo/* and */* when ordering our
> matches before, so we don't need to do it now.
Seems to me that was a bug, and a good argument for explicit ordering.
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html
"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff
RE: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Posted by Allan Edwards <ak...@meepzor.com>.
> A) Specifying AP_HOOK_LAST won't work unless we separate the checks into
> two functions, so that we have something like:
>
> ap_hook_1(...)
> {
> if(!strcmp(SPECIFIC, r->handler)
> real_handler
> }
>
> ap_hook_2(...)
> {
> if (!strcmp_match(GENERAL, r->handler)
> real_handler
> }
>
> real_handler(...)
> {
> ...
> }
>
> ap_hook_handler(ap_hook_1, NULL, NULL, AP_HOOK_MIDDLE)
> ap_hook_handler(ap_hook_2, NULL, NULL, AP_HOOK_LAST)
>
> While this will work, it is really bad, and I seriously dislike it, a lot.
Ah, I think I see what you are getting at now. The issue is not a
module that matches multiple handler types, but one that matches both
specific and wildcard values. Is there such a beast (maybe that was
your original question) -- other than default_handler which is
AP_HOOK_REALLY_LAST and therefore doesn't pose a problem?
Allan
RE: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Posted by rb...@covalent.net.
> I was mulling the same issue over yesterday. It seems to me that we
> _should_ be giving any specific match first shot. One way to do
> this is to specify AP_HOOK_LAST for any handlers that have wildcards.
> Question though, do we need to ensure foo/* get its shot before */* ?
A) Specifying AP_HOOK_LAST won't work unless we separate the checks into
two functions, so that we have something like:
ap_hook_1(...)
{
if(!strcmp(SPECIFIC, r->handler)
real_handler
}
ap_hook_2(...)
{
if (!strcmp_match(GENERAL, r->handler)
real_handler
}
real_handler(...)
{
...
}
ap_hook_handler(ap_hook_1, NULL, NULL, AP_HOOK_MIDDLE)
ap_hook_handler(ap_hook_2, NULL, NULL, AP_HOOK_LAST)
While this will work, it is really bad, and I seriously dislike it, a lot.
b) We never used to distinguish between foo/* and */* when ordering our
matches before, so we don't need to do it now.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
RE: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Posted by Allan Edwards <ak...@meepzor.com>.
> Jon Travis pointed out to me yesterday that this is wrong in more ways
> than one. The problem is that before the change, Apache would run all the
> handlers that registered specific handler types, like DIR_MAGIC_TYPE or
> server-parsed-html before it ran any handler that registered generic
> handler types, ones with *'s in them.
>
> This behavior is now gone, which is a bad thing IMHO. This means that a
> module that matches multiple handler types, only gets one chance to
> matche them all. I didn't think this was important yesterday, but I woke
> up this morning, and I believe it is. Somebody tell me I'm wrong
> please. I just want somebody else to say I am wrong. :-)
>
I was mulling the same issue over yesterday. It seems to me that we
_should_ be giving any specific match first shot. One way to do
this is to specify AP_HOOK_LAST for any handlers that have wildcards.
Question though, do we need to ensure foo/* get its shot before */* ?
Allan
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Posted by rb...@covalent.net.
> > + if (strcmp(r->handler, "*.*")) {
> > + return DECLINED;
> > + }
> > +
>
> As extensively discussed, this is _wrong_.
Jon Travis pointed out to me yesterday that this is wrong in more ways
than one. The problem is that before the change, Apache would run all the
handlers that registered specific handler types, like DIR_MAGIC_TYPE or
server-parsed-html before it ran any handler that registered generic
handler types, ones with *'s in them.
This behavior is now gone, which is a bad thing IMHO. This means that a
module that matches multiple handler types, only gets one chance to
matche them all. I didn't think this was important yesterday, but I woke
up this morning, and I believe it is. Somebody tell me I'm wrong
please. I just want somebody else to say I am wrong. :-)
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Posted by Doug MacEachern <do...@covalent.net>.
On Tue, 9 Jan 2001, Ben Laurie wrote:
> >
> > + if (strcmp(r->handler, "*.*")) {
> > + return DECLINED;
> > + }
> > +
>
> As extensively discussed, this is _wrong_.
yep and if you had looked at the next commit in your mailbox before
shouting, you'd have seen its changed to use ap_strcmp_match().
Re: cvs commit: httpd-2.0/server/mpm/winnt mpm_winnt.c
Posted by rb...@covalent.net.
> > + if (strcmp(r->handler, "*.*")) {
> > + return DECLINED;
> > + }
> > +
>
> As extensively discussed, this is _wrong_.
Jon Travis pointed out to me yesterday that this is wrong in more ways
than one. The problem is that before the change, Apache would run all the
handlers that registered specific handler types, like DIR_MAGIC_TYPE or
server-parsed-html before it ran any handler that registered generic
handler types, ones with *'s in them.
This behavior is now gone, which is a bad thing IMHO. This means that a
module that matches multiple handler types, only gets one chance to
matche them all. I didn't think this was important yesterday, but I woke
up this morning, and I believe it is. Somebody tell me I'm wrong
please. I just want somebody else to say I am wrong. :-)
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------