On Fri, Nov 2, 2012 at 5:50 PM, Mark Phippard <markp...@gmail.com> wrote:
> On Fri, Nov 2, 2012 at 4:13 AM, Ivan Zhakov <i...@visualsvn.com> wrote:
>> On Tue, Oct 23, 2012 at 4:23 PM, C. Michael Pilato <cmpil...@collab.net>
>> wrote:
>>> On 10/23/2012 07:24 AM, Ivan Zhakov wrote:
>>>> I'm working on the patch to list only readable repositories. There is
>>>> already TODO comment in the code by cmpilato:
>>>> subversion\mod_dav_svn\repos.c:3461
>>>> [[[
>>>> /* ### TODO: We could test for readability of the root
>>>> directory of each repository and hide those that
>>>> the user can't see. */
>>>> ]]]
>>>
>>> I, too, started looking into this, Ivan, but I realized that I was probably
>>> about to run into a whole mess of code refactoring that I wasn't really up
>>> for dealing with at the time. (Trying to stay as 1.8-focused as I can.)
>>> I'm happy to review any work you do on this issue, though.
>>>
>> Hi Mike,
>>
>> Please find attached patch to hide unreadable repositories in
>> "Collection of Repositories":
>> [[[
>> mod_dav_svn: Hide repositories from list that are not accessible for user.
>>
>> * subversion/mod_dav_svn/authz.c
>> * subversion/mod_dav_svn/dav_svn.h
>> (dav_svn__allow_list_repos): New.
>>
>> * subversion/mod_dav_svn/repos.c
>> (deliver): Check for readability of the root directory of each
>> repository and hide those that the user can't see.
>> ]]]
>>
>> Code in deliver() method is not best now, but I was trying to minimize
>> changes in my patch. I'm going to refactor code later after committing
>> my patch.
>>
>> Looking forward for your review. Thanks!
>
> + /* Build a Public Resource uri representing repository root. */
> + uri = svn_urlpath__join(dav_svn__get_root_dir(r),
> + svn_path_uri_encode(repos_name, pool), pool);
> +
> + /* Check if GET would work against this uri. */
> + subreq = ap_sub_req_method_uri("GET", uri, r, r->output_filters);
>
> Just a drive-by, so if I am way-off just say so.
>
> I am assuming that since this is doing a GET, the server will have to
> fully process it as if it would for a web browser making the same
> request.
I didn't details, but I assume that actual sub request execution
happens on ap_run_sub_req() call.
> So on a repository like the ASF or Wordpress where there are
> a lot of top level folders then the server might have to do a fair
> amount of work to process the request and return. I assume we do not
> care about the content of the response, just the success or failure.
>
We already use sub requests for authorization checks in repository
folder listing and log.
> So I am just wondering if there is a lighter weight HTTP request we
> could do that would still trigger the authz check? Something like
> OPTIONS or PROPFIND. Whatever would make sense and be quick to
> process.
>
Another option is HEAD.
--
Ivan Zhakov
