You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Greg Stein <gs...@lyra.org> on 1999/02/23 13:25:47 UTC

ugly problem with sub-requests

Hi all,

In my mod_dav module, I'm using the sub-request mechanism to map the
destination of a move/copy onto a directory/file in the filesystem.

This is reasonably straight-forward. The sub-req mechanism doesn't take
an absolute URI, so I do some munging before calling it. Not a big deal,
although it would be nice to see the sub-req system take absolute URIs.

The *real* problem here is with ap_set_sub_req_protocol(). It hard-codes
the sub-request to use the "GET" method. For mod_dav, this means that
the target is authorized as a GET rather than a MOVE.

Needless to say, if the filesystem has the right permissions, it means
that somebody with access to the MOVE method can stomp files anywhere in
Apache's URL namespace. And there is no way to <limit> it.

I started to make a private copy of ap_sub_req_lookup_uri() and
ap_set_sub_req_protocol() so that I could change the method. Great,
until I realized that I also need a bazillion functions from
http_request.c such as file_walk(), directory_walk(), etc. It looks
almost like I would need to snarf in almost all of http_request.c!

It looks like maybe I could do a gross hack: call the sub-req mechanism,
tweak the method, and then call access/auth functions again.

I'm at a loss. Any help/ideas here would be appreciated.

thx!
-g

p.s. and yes, this is also a plea for changes in 1.3.5 and/or 2.0; I'll
help spec the requirements and/or provide code if needed... this is
quite important for providing DAV via Apache

--
Greg Stein, http://www.lyra.org/

Re: [PATCH] Re: ugly problem with sub-requests

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

On Thu, 25 Feb 1999, Dean Gaudet wrote:

> (I wonder how much noise -Wwrite-strings gives these days...) 

(lots, and it also depends on how nice your libc header files are...)

Dean


Re: [PATCH] Re: ugly problem with sub-requests

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

On Thu, 25 Feb 1999, Greg Stein wrote:

> > +     /* ### would be nice to pass "method" to ap_set_sub_rqe_protocol */
> > +     rnew->method = method;
> > +     rnew->method_number = ap_method_number_of(method);
> > +
> 
> Right here.  rnew->method is "char *", while the argument is "const char
> *". The compiler complains about dropping the "const".
> 
> I'm not sure whether to argue for a change to the request_rec structure,
> or to bite down and do an ap_pstrdup().

Change it to const char *method in the request_rec ... there's at least
one place where it's set to a ""-quoted string, so anyone mucking with it
is potentially doing harm.

(I wonder how much noise -Wwrite-strings gives these days...) 

Dean



Re: [PATCH] Re: ugly problem with sub-requests

Posted by Greg Stein <gs...@lyra.org>.
Greg Stein wrote:
>...
> Done :-)  ... attached.
> 
> It gets rid of my icky hack quite nicely!

Oop. One tweakiness is in here:

> ...
> diff -c -r ./src/main/http_request.c ../../apache_1.3.4/src/main/http_request.c
> ...
> ! API_EXPORT(request_rec *) ap_sub_req_method_uri(const char *method,
> !                                               const char *new_file,
> !                                               const request_rec *r)
>   {
>       request_rec *rnew;
>       int res;
> ***************
> *** 736,741 ****
> --- 737,746 ----
> 
>       ap_set_sub_req_protocol(rnew, r);
> 
> +     /* ### would be nice to pass "method" to ap_set_sub_rqe_protocol */
> +     rnew->method = method;
> +     rnew->method_number = ap_method_number_of(method);
> +

Right here.  rnew->method is "char *", while the argument is "const char
*". The compiler complains about dropping the "const".

I'm not sure whether to argue for a change to the request_rec structure,
or to bite down and do an ap_pstrdup().

??

thx
-g

--
Greg Stein, http://www.lyra.org/

Re: [PATCH] Re: ugly problem with sub-requests

Posted by Doug MacEachern <do...@pobox.com>.
>Done :-)  ... attached.
>
>It gets rid of my icky hack quite nicely!

very nice, +1

-Doug

[PATCH] Re: ugly problem with sub-requests

Posted by Greg Stein <gs...@lyra.org>.
Ben Laurie wrote:
> Greg Stein wrote:
> > I'm not sure that I'd recommend simply changing
> > ap_set_sub_req_protocol() to simply copying over the method. The
> > semantics of the sub-request might actually be "do a GET request". I
> > would probably request a function just like ap_sub_req_lookup_uri(), but
> > where I can pass a method. It would set the method and method_number
> > from the argument, but otherwise behave the same.
> 
> I'm probably stating the bleedin' obvious here, but its never stopped me
> before... do this, and rewrite ap_sub_req_lookup_uri in terms of it,
> passing "GET" as the method.

Done :-)  ... attached.

It gets rid of my icky hack quite nicely!

Cheers,
-g

--
Greg Stein, http://www.lyra.org/

Re: ugly problem with sub-requests

Posted by Ben Laurie <be...@algroup.co.uk>.
Greg Stein wrote:
> I'm not sure that I'd recommend simply changing
> ap_set_sub_req_protocol() to simply copying over the method. The
> semantics of the sub-request might actually be "do a GET request". I
> would probably request a function just like ap_sub_req_lookup_uri(), but
> where I can pass a method. It would set the method and method_number
> from the argument, but otherwise behave the same.

I'm probably stating the bleedin' obvious here, but its never stopped me
before... do this, and rewrite ap_sub_req_lookup_uri in terms of it,
passing "GET" as the method.

Cheers,

Ben.

--
http://www.apache-ssl.org/ben.html

"My grandfather once told me that there are two kinds of people: those
who work and those who take the credit. He told me to try to be in the
first group; there was less competition there."
     - Indira Gandhi

Re: ugly problem with sub-requests

Posted by Greg Stein <gs...@lyra.org>.
Dean Gaudet wrote:
> On Tue, 23 Feb 1999, Greg Stein wrote:
> ...
> > This is reasonably straight-forward. The sub-req mechanism doesn't take
> > an absolute URI, so I do some munging before calling it. Not a big deal,
> > although it would be nice to see the sub-req system take absolute URIs.
> 
> Huh?  ap_sub_req_lookup_uri() :
> ...
> That looks like it supports absolute uris to me... or are you talking
> about http://foo/blah ?

Absolute URIs are defined in the RFCs to include the hostname.
Otherwise, the URI is relative to the host. :-)

> Supporting hostnames there is a total pain in the ass -- because vhosts
> just aren't as clean as everyone pretends they are.

No doubt. I use ap_matches_request_vhost(). I don't remember the
specifics, but when I implemented that, I recall that the vhost
situation was quite shaky. For example, "localhost" or "kurgan" would
not match "kurgan.lyra.org". So my destination URIs always had to be
fully-qualified (sigh).

> > The *real* problem here is with ap_set_sub_req_protocol(). It hard-codes
> > the sub-request to use the "GET" method. For mod_dav, this means that
> > the target is authorized as a GET rather than a MOVE.
> 
> I wonder what breaks if you just copy the protocol.  Protocols expecting
> request bodies can't be copied... but perhaps they shouldn't be able to
> succeed in performing a subrequest -- or rather, later on when a module
> attempts to access the body it should get an error.

I tried the hack that I mentioned in my email: reset the method and
rerun the auth/auth stages. It seems to work as I'd like. However, I
just *know* there will be a module out there that is going to puke
because it was run twice for the same request.

Here are the standard modules that use sub-requests and what HTTP
methods they might be using when the sub_req is issued:

  mod_autoindex     GET only
  mod_cern_meta     any
  mod_dir           any
  mod_include       GET only
  mod_mime_magic    any
  mod_negotiation   any?
  mod_rewrite       any
  mod_isapi         any

However, the question is really what happens to somebody that processes
alternative methods when they are part of a sub-request? I can say that
mod_dav wouldn't be bothered: the bulk of it is a handler. There is a
type_checker, which sets the handler based on the method; with or
without the proper method, mod_dav is uninvolved in a sub_req (unless it
is run, of course).

If necessary, a module can always tell it is a subrequest and avoid
processing of a body. However, if it gets there, then it probably is an
error.

I'm not sure that I'd recommend simply changing
ap_set_sub_req_protocol() to simply copying over the method. The
semantics of the sub-request might actually be "do a GET request". I
would probably request a function just like ap_sub_req_lookup_uri(), but
where I can pass a method. It would set the method and method_number
from the argument, but otherwise behave the same.

Cheers,
-g

--
Greg Stein, http://www.lyra.org/

Re: ugly problem with sub-requests

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

On Tue, 23 Feb 1999, Greg Stein wrote:

> Hi all,
> 
> In my mod_dav module, I'm using the sub-request mechanism to map the
> destination of a move/copy onto a directory/file in the filesystem.
> 
> This is reasonably straight-forward. The sub-req mechanism doesn't take
> an absolute URI, so I do some munging before calling it. Not a big deal,
> although it would be nice to see the sub-req system take absolute URIs.

Huh?  ap_sub_req_lookup_uri() : 

    if (new_file[0] == '/')
        ap_parse_uri(rnew, new_file);
    else {
        udir = ap_make_dirstr_parent(rnew->pool, r->uri);
        udir = ap_escape_uri(rnew->pool, udir);    /* re-escape it */
        ap_parse_uri(rnew, ap_make_full_path(rnew->pool, udir, new_file));
    }

That looks like it supports absolute uris to me... or are you talking
about http://foo/blah ?

Supporting hostnames there is a total pain in the ass -- because vhosts
just aren't as clean as everyone pretends they are.

> The *real* problem here is with ap_set_sub_req_protocol(). It hard-codes
> the sub-request to use the "GET" method. For mod_dav, this means that
> the target is authorized as a GET rather than a MOVE.

I wonder what breaks if you just copy the protocol.  Protocols expecting
request bodies can't be copied... but perhaps they shouldn't be able to
succeed in performing a subrequest -- or rather, later on when a module
attempts to access the body it should get an error.

Dean