You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by cm...@collab.net on 2001/11/16 07:00:17 UTC
Re: Gack! Weirdo DAV bug.
Bahahahaha! I found the problem, and it is the same problem that
caused Karl and I to hack mod_dav.c a while ago (see Issue #470, to
which I will be appending my find).
As it turns out, the REPORT request method is NOT one to which the
Apache developers have assigned a nifty #define in httpd.h. That
should be all fine and dandy, since the modules can see that the
request number is M_INVALID and then choose parse the request method
string ("REPORT" in this case) for themselves to determine if they
want to handle that request.
BUT!!
The presence of this set of directives throws a wrench into the works:
> <LimitExcept GET PROPFIND OPTIONS REPORT>
> require valid-user
> </LimitExcept>
The code that parses this from the conf file sees "REPORT" as a
request type, sez, "Hmm...I dunno what REPORT is, so just to be safe
I'll dynamically register that method" [see core.c:ap_limit_section()]:
else if (methnum == M_INVALID) {
/* method has not been registered yet, but resorce restriction
* is always checked before method handling, so register it.
*/
methnum = ap_method_register(cmd->pool, method);
This results in the REPORT requests being assigned a request number
that is NOT M_INVALID (in fact, in my specific instance it's M_INVALID
+ 1, which I suppose is the first of many slots for dynamically
registered request types).
So, we get down into mod_dav.c, in dav_handler(), and after a series
of checks to see if the incoming request has a known request number,
dav_handler() make a quick sanity check:
/*
* NOTE: When Apache moves creates defines for the add'l DAV methods,
* then it will no longer use M_INVALID. This code must be
* updated each time Apache adds method defines.
*/
if (r->method_number != M_INVALID) {
return DECLINED;
}
It's this sanity check which is failing. The REPORT request has a
valid request number, but obviously not one that mod_dav could use --
since the number is dynamically allotted, who's to say it will always
be M_INVALID + 1. I betcha I could modify the conf file in such a way
that some other request type got that slot and REPORT got a different
one.
I'm not sure what the right fix is, having no real knowledge of
httpd's internals. The options that come to mind are:
* Have the server NOT register the REPORT request type.
But this seems like a bad idea...I assume that code is there for a
reason, and the comment above it may be all the reason that's
needed.
* Remove the sanity check from mod_dav.c
We *know* this works (that's how the code is on svn.collab.net
today), but it means that every unregistered request type coming
into mod_dav will suffer about 20 strcmp's. That sucks. But, I
can't think of anything else offhand to solve it.
Note that when you remove those <LimitExcept> directives, all works
well because REPORT is never assigned a dynamic request method number,
which means mod_dav sees M_INVALID and goes about its strcmp'ing merry
way.
Buh.
I hate knowing the problem but not the solution. Oh well, it's better
than not even knowing the problem.
GregS, can you make a solid determination on the solution here,
implement that badboy, and get us OUT of this ooze!? Issue #470
awaits you...
> I have a Subversion repository at /usr/www/repositories/test on my
> box. Been using it forever. Until last night, the httpd.conf
> directives looked like this:
>
> <Location /repos/test>
> DAV svn
> SVNPath /usr/www/repositories/test
> </Location>
>
> Then, I needed to test something auth-related. So, I copied the
> Subversion users auth file from svn.collab.net to my local box, and
> copied the directives for using that file from svn.collab.net's
> httpd.conf. Now it looks like this:
>
> <Location /repos/test>
> DAV svn
> SVNPath /usr/www/repositories/test
> AuthType Basic
> AuthName "Subversion repository"
> AuthUserFile /usr/www/svn-user-file
> <LimitExcept GET PROPFIND OPTIONS REPORT>
> require valid-user
> </LimitExcept>
> </Location>
>
> Upon doing this, I was able to checkout and commit and such with no
> problem. My first commit even appropriate queried me for a password.
> However, when I tried to update from the top level of my working copy,
> I got this:
>
> apr_error: #20014, src_err 0 : <Error string not specified yet>
> The REPORT status was 500, but expected 200.
>
> Checking out Apache's error log, I saw:
>
> [Wed Nov 14 08:56:42 2001] [warn] [client 127.0.0.1] handler
> "dav-handler" not found for: /usr/www/docroot/repos
>
> If I reverted my httpd.conf to the non-auth setup, all was well. With
> auth, I got the error, every time.
>
> I looks as though somewhere, something is splitting the URL from
> http://localhost/repos/test to http://localhost/repos, which maps to
> /usr/www/docroot/repos, and of course is NOT a dav-handled thing.
>
> Thinking further, I wondered if this was happening on svn.collab.net
> as well, but that I never saw it because I have `trunk' checked out
> (trunk's parent dir is still inside the repos). So I checked out
> http://svn.collab.net/repos/svn, and tried to update. Same error.
> That sucks.
>
> Weird thing is that is only seems to happen when the auth stuff is
> present. Hopefully that will help me (or someone else) track this
> down.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Gack! Weirdo DAV bug.
Posted by Keith Wannamaker <Ke...@Wannamaker.org>.
It is used extensively in the webdav acl and
delta-v drafts to query indirect information
about a url.
Keith
| -----Original Message-----
| From: Roy T. Fielding [mailto:fielding@ebuilt.com]
| Sent: Friday, November 16, 2001 1:14 AM
| To: cmpilato@collab.net
| Cc: dev@subversion.tigris.org; dev@httpd.apache.org
| Subject: Re: Gack! Weirdo DAV bug.
|
|
| > As it turns out, the REPORT request method is NOT one to which the
| > Apache developers have assigned a nifty #define in httpd.h.
|
| What the heck is the REPORT request method?
|
| ....Roy
|
Re: Gack! Weirdo DAV bug.
Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
> As it turns out, the REPORT request method is NOT one to which the
> Apache developers have assigned a nifty #define in httpd.h.
What the heck is the REPORT request method?
....Roy
Re: Gack! Weirdo DAV bug.
Posted by Ryan Bloom <rb...@covalent.net>.
On Friday 16 November 2001 06:49 pm, Greg Stein wrote:
> The current logic is basically:
>
> if (method_number != M_GET)
> return do_get();
>
> if (method_number != M_PROPFIND)
> return do_propfind();
>
> if (method_number != M_INVALID)
> return DECLINED;
>
> if (strcmp(method_name, "REPORT") == 0)
> return do_report();
>
> if (strcmp(method_name, "MERGE") == 0)
> return do_merge();
>
>
> We use the M_INVALID to shortcut a bunch of strcmp() operations on the
> method name. Switching that logic over to use the APIs is not going to save
> us much (if anything!) over the strcmp(). For each method, we'd have to
> call into the API to say "is it <this> one?" or "is it <that> one?"
>
> I'm going to have to look into this whole method registration thing. I saw
> it go in, but never bothered looking too closely. It didn't seem like it
> was going to monkey with the method number stored in the request. Eek.
The point was to allow modules to be able to associate a number with a
method that the core server didn't know anything about. In order to do that,
we did need to change the number.
> Dunno that the method stuff must change. At a minimum, mod_dav can just
> pre-register everything and keep the numbers around for use in the above
> logic. But I still want to get a look at the stuff and see what's there.
> I've got some particular experience with extended methods :-)
That was the original idea. Modules would register the method at startup,
and compare against that number, which is what the API's do.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Gack! Weirdo DAV bug.
Posted by Ryan Bloom <rb...@covalent.net>.
On Friday 16 November 2001 06:49 pm, Greg Stein wrote:
> The current logic is basically:
>
> if (method_number != M_GET)
> return do_get();
>
> if (method_number != M_PROPFIND)
> return do_propfind();
>
> if (method_number != M_INVALID)
> return DECLINED;
>
> if (strcmp(method_name, "REPORT") == 0)
> return do_report();
>
> if (strcmp(method_name, "MERGE") == 0)
> return do_merge();
>
>
> We use the M_INVALID to shortcut a bunch of strcmp() operations on the
> method name. Switching that logic over to use the APIs is not going to save
> us much (if anything!) over the strcmp(). For each method, we'd have to
> call into the API to say "is it <this> one?" or "is it <that> one?"
>
> I'm going to have to look into this whole method registration thing. I saw
> it go in, but never bothered looking too closely. It didn't seem like it
> was going to monkey with the method number stored in the request. Eek.
The point was to allow modules to be able to associate a number with a
method that the core server didn't know anything about. In order to do that,
we did need to change the number.
> Dunno that the method stuff must change. At a minimum, mod_dav can just
> pre-register everything and keep the numbers around for use in the above
> logic. But I still want to get a look at the stuff and see what's there.
> I've got some particular experience with extended methods :-)
That was the original idea. Modules would register the method at startup,
and compare against that number, which is what the API's do.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: Gack! Weirdo DAV bug.
Posted by Greg Stein <gs...@lyra.org>.
On Fri, Nov 16, 2001 at 03:30:39PM -0800, Ryan Bloom wrote:
> On Friday 16 November 2001 11:18 am, cmpilato@collab.net wrote:
> > Ryan Bloom <rb...@covalent.net> writes:
>...
> > > Option 3. :-) We have APIs to allow you to check the dynamically
> > > registered methods.
> >
> > I'd already thought about this. But the methods hash is just
> > NAME->number mapping. What would this gain us? We already have both
> > the name and the number in the request structure. Maybe I'm missing
> > something (I hope).
>
> You misunderstood me. We already have the APIs to do this. The same way
> that you compare against M_INVALID, you could actually check against
> REPORT directly.
The current logic is basically:
if (method_number != M_GET)
return do_get();
if (method_number != M_PROPFIND)
return do_propfind();
if (method_number != M_INVALID)
return DECLINED;
if (strcmp(method_name, "REPORT") == 0)
return do_report();
if (strcmp(method_name, "MERGE") == 0)
return do_merge();
We use the M_INVALID to shortcut a bunch of strcmp() operations on the
method name. Switching that logic over to use the APIs is not going to save
us much (if anything!) over the strcmp(). For each method, we'd have to call
into the API to say "is it <this> one?" or "is it <that> one?"
I'm going to have to look into this whole method registration thing. I saw
it go in, but never bothered looking too closely. It didn't seem like it was
going to monkey with the method number stored in the request. Eek.
Dunno that the method stuff must change. At a minimum, mod_dav can just
pre-register everything and keep the numbers around for use in the above
logic. But I still want to get a look at the stuff and see what's there.
I've got some particular experience with extended methods :-)
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: Gack! Weirdo DAV bug.
Posted by Greg Stein <gs...@lyra.org>.
On Fri, Nov 16, 2001 at 03:30:39PM -0800, Ryan Bloom wrote:
> On Friday 16 November 2001 11:18 am, cmpilato@collab.net wrote:
> > Ryan Bloom <rb...@covalent.net> writes:
>...
> > > Option 3. :-) We have APIs to allow you to check the dynamically
> > > registered methods.
> >
> > I'd already thought about this. But the methods hash is just
> > NAME->number mapping. What would this gain us? We already have both
> > the name and the number in the request structure. Maybe I'm missing
> > something (I hope).
>
> You misunderstood me. We already have the APIs to do this. The same way
> that you compare against M_INVALID, you could actually check against
> REPORT directly.
The current logic is basically:
if (method_number != M_GET)
return do_get();
if (method_number != M_PROPFIND)
return do_propfind();
if (method_number != M_INVALID)
return DECLINED;
if (strcmp(method_name, "REPORT") == 0)
return do_report();
if (strcmp(method_name, "MERGE") == 0)
return do_merge();
We use the M_INVALID to shortcut a bunch of strcmp() operations on the
method name. Switching that logic over to use the APIs is not going to save
us much (if anything!) over the strcmp(). For each method, we'd have to call
into the API to say "is it <this> one?" or "is it <that> one?"
I'm going to have to look into this whole method registration thing. I saw
it go in, but never bothered looking too closely. It didn't seem like it was
going to monkey with the method number stored in the request. Eek.
Dunno that the method stuff must change. At a minimum, mod_dav can just
pre-register everything and keep the numbers around for use in the above
logic. But I still want to get a look at the stuff and see what's there.
I've got some particular experience with extended methods :-)
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Gack! Weirdo DAV bug.
Posted by Ryan Bloom <rb...@covalent.net>.
On Friday 16 November 2001 11:18 am, cmpilato@collab.net wrote:
> Ryan Bloom <rb...@covalent.net> writes:
> > > I'm not sure what the right fix is, having no real knowledge of
> > > httpd's internals. The options that come to mind are:
> > >
> > > * Have the server NOT register the REPORT request type.
> > >
> > > But this seems like a bad idea...I assume that code is there for a
> > > reason, and the comment above it may be all the reason that's
> > > needed.
> > >
> > > * Remove the sanity check from mod_dav.c
> > >
> > > We *know* this works (that's how the code is on svn.collab.net
> > > today), but it means that every unregistered request type coming
> > > into mod_dav will suffer about 20 strcmp's. That sucks. But, I
> > > can't think of anything else offhand to solve it.
> >
> > Option 3. :-) We have APIs to allow you to check the dynamically
> > registered methods.
>
> I'd already thought about this. But the methods hash is just
> NAME->number mapping. What would this gain us? We already have both
> the name and the number in the request structure. Maybe I'm missing
> something (I hope).
You misunderstood me. We already have the APIs to do this. The same way
that you compare against M_INVALID, you could actually check against
REPORT directly.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: Gack! Weirdo DAV bug.
Posted by Ryan Bloom <rb...@covalent.net>.
On Friday 16 November 2001 11:18 am, cmpilato@collab.net wrote:
> Ryan Bloom <rb...@covalent.net> writes:
> > > I'm not sure what the right fix is, having no real knowledge of
> > > httpd's internals. The options that come to mind are:
> > >
> > > * Have the server NOT register the REPORT request type.
> > >
> > > But this seems like a bad idea...I assume that code is there for a
> > > reason, and the comment above it may be all the reason that's
> > > needed.
> > >
> > > * Remove the sanity check from mod_dav.c
> > >
> > > We *know* this works (that's how the code is on svn.collab.net
> > > today), but it means that every unregistered request type coming
> > > into mod_dav will suffer about 20 strcmp's. That sucks. But, I
> > > can't think of anything else offhand to solve it.
> >
> > Option 3. :-) We have APIs to allow you to check the dynamically
> > registered methods.
>
> I'd already thought about this. But the methods hash is just
> NAME->number mapping. What would this gain us? We already have both
> the name and the number in the request structure. Maybe I'm missing
> something (I hope).
You misunderstood me. We already have the APIs to do this. The same way
that you compare against M_INVALID, you could actually check against
REPORT directly.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Gack! Weirdo DAV bug.
Posted by cm...@collab.net.
Ryan Bloom <rb...@covalent.net> writes:
> > I'm not sure what the right fix is, having no real knowledge of
> > httpd's internals. The options that come to mind are:
> >
> > * Have the server NOT register the REPORT request type.
> >
> > But this seems like a bad idea...I assume that code is there for a
> > reason, and the comment above it may be all the reason that's
> > needed.
> >
> > * Remove the sanity check from mod_dav.c
> >
> > We *know* this works (that's how the code is on svn.collab.net
> > today), but it means that every unregistered request type coming
> > into mod_dav will suffer about 20 strcmp's. That sucks. But, I
> > can't think of anything else offhand to solve it.
>
> Option 3. :-) We have APIs to allow you to check the dynamically registered
> methods.
I'd already thought about this. But the methods hash is just
NAME->number mapping. What would this gain us? We already have both
the name and the number in the request structure. Maybe I'm missing
something (I hope).
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Gack! Weirdo DAV bug.
Posted by cm...@collab.net.
Ryan Bloom <rb...@covalent.net> writes:
> > I'm not sure what the right fix is, having no real knowledge of
> > httpd's internals. The options that come to mind are:
> >
> > * Have the server NOT register the REPORT request type.
> >
> > But this seems like a bad idea...I assume that code is there for a
> > reason, and the comment above it may be all the reason that's
> > needed.
> >
> > * Remove the sanity check from mod_dav.c
> >
> > We *know* this works (that's how the code is on svn.collab.net
> > today), but it means that every unregistered request type coming
> > into mod_dav will suffer about 20 strcmp's. That sucks. But, I
> > can't think of anything else offhand to solve it.
>
> Option 3. :-) We have APIs to allow you to check the dynamically registered
> methods.
I'd already thought about this. But the methods hash is just
NAME->number mapping. What would this gain us? We already have both
the name and the number in the request structure. Maybe I'm missing
something (I hope).
Re: Gack! Weirdo DAV bug.
Posted by Ryan Bloom <rb...@covalent.net>.
On Thursday 15 November 2001 10:00 pm, cmpilato@collab.net wrote:
> The presence of this set of directives throws a wrench into the works:
> > <LimitExcept GET PROPFIND OPTIONS REPORT>
> > require valid-user
> > </LimitExcept>
>
> The code that parses this from the conf file sees "REPORT" as a
> request type, sez, "Hmm...I dunno what REPORT is, so just to be safe
> I'll dynamically register that method" [see core.c:ap_limit_section()]:
>
> else if (methnum == M_INVALID) {
> /* method has not been registered yet, but resorce restriction
> * is always checked before method handling, so register it.
> */
> methnum = ap_method_register(cmd->pool, method);
>
> This results in the REPORT requests being assigned a request number
> that is NOT M_INVALID (in fact, in my specific instance it's M_INVALID
> + 1, which I suppose is the first of many slots for dynamically
> registered request types).
>
> So, we get down into mod_dav.c, in dav_handler(), and after a series
> of checks to see if the incoming request has a known request number,
> dav_handler() make a quick sanity check:
>
> /*
> * NOTE: When Apache moves creates defines for the add'l DAV methods,
> * then it will no longer use M_INVALID. This code must be
> * updated each time Apache adds method defines.
> */
> if (r->method_number != M_INVALID) {
> return DECLINED;
> }
>
> It's this sanity check which is failing. The REPORT request has a
> valid request number, but obviously not one that mod_dav could use --
> since the number is dynamically allotted, who's to say it will always
> be M_INVALID + 1. I betcha I could modify the conf file in such a way
> that some other request type got that slot and REPORT got a different
> one.
>
> I'm not sure what the right fix is, having no real knowledge of
> httpd's internals. The options that come to mind are:
>
> * Have the server NOT register the REPORT request type.
>
> But this seems like a bad idea...I assume that code is there for a
> reason, and the comment above it may be all the reason that's
> needed.
>
> * Remove the sanity check from mod_dav.c
>
> We *know* this works (that's how the code is on svn.collab.net
> today), but it means that every unregistered request type coming
> into mod_dav will suffer about 20 strcmp's. That sucks. But, I
> can't think of anything else offhand to solve it.
Option 3. :-) We have APIs to allow you to check the dynamically registered
methods.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: Gack! Weirdo DAV bug.
Posted by Ryan Bloom <rb...@covalent.net>.
On Thursday 15 November 2001 10:00 pm, cmpilato@collab.net wrote:
> The presence of this set of directives throws a wrench into the works:
> > <LimitExcept GET PROPFIND OPTIONS REPORT>
> > require valid-user
> > </LimitExcept>
>
> The code that parses this from the conf file sees "REPORT" as a
> request type, sez, "Hmm...I dunno what REPORT is, so just to be safe
> I'll dynamically register that method" [see core.c:ap_limit_section()]:
>
> else if (methnum == M_INVALID) {
> /* method has not been registered yet, but resorce restriction
> * is always checked before method handling, so register it.
> */
> methnum = ap_method_register(cmd->pool, method);
>
> This results in the REPORT requests being assigned a request number
> that is NOT M_INVALID (in fact, in my specific instance it's M_INVALID
> + 1, which I suppose is the first of many slots for dynamically
> registered request types).
>
> So, we get down into mod_dav.c, in dav_handler(), and after a series
> of checks to see if the incoming request has a known request number,
> dav_handler() make a quick sanity check:
>
> /*
> * NOTE: When Apache moves creates defines for the add'l DAV methods,
> * then it will no longer use M_INVALID. This code must be
> * updated each time Apache adds method defines.
> */
> if (r->method_number != M_INVALID) {
> return DECLINED;
> }
>
> It's this sanity check which is failing. The REPORT request has a
> valid request number, but obviously not one that mod_dav could use --
> since the number is dynamically allotted, who's to say it will always
> be M_INVALID + 1. I betcha I could modify the conf file in such a way
> that some other request type got that slot and REPORT got a different
> one.
>
> I'm not sure what the right fix is, having no real knowledge of
> httpd's internals. The options that come to mind are:
>
> * Have the server NOT register the REPORT request type.
>
> But this seems like a bad idea...I assume that code is there for a
> reason, and the comment above it may be all the reason that's
> needed.
>
> * Remove the sanity check from mod_dav.c
>
> We *know* this works (that's how the code is on svn.collab.net
> today), but it means that every unregistered request type coming
> into mod_dav will suffer about 20 strcmp's. That sucks. But, I
> can't think of anything else offhand to solve it.
Option 3. :-) We have APIs to allow you to check the dynamically registered
methods.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org