Re: [PATCH 5/5] git-daemon support for user-relative paths.

From: Andreas Ericsson <ae@op5.se>
Date: Nov 21, 2005, 11:10 UTC
Message-ID: <4381AB38.7090209@op5.se>
In-Reply-To: <7vfypquz88.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:

Show 14 quoted lines

> Andreas Ericsson <ae@op5.se> writes:
> 
> 
>>I'll run the clone/fetch/push test-suite again tomorrow, with this 
>>applied. It looks good though.
> 
> 
> Sorry, but there was a thinko in my butchered version of
> enter_repo().  While allowing only absolute path was good for
> the version with your daemon.c change, it was not with the
> current one that runs upload-pack with "." as repo.  In either
> case we _do_ chdir() to it after validating the path, so I am
> wondering if it is a good idea to keep sending "." as repo when
> executing upload-pack with this patch as well.

It might be, and it's good since it prevents the otherwise possible race that occurs when git-upload-pack chdir()'s again.

Show 5 quoted lines

>  This does not
> make any practical difference, but I think it makes the intent
> clearer -- "we are already there so do not try going anywhere
> else".
>

So enter_repo allows "." (exactly and without chdir()) and all paths starting with '/' if strict?

Show 9 quoted lines

> So I am thinking about applying something like this patch
> on top of the last part of your patch.
> 
>  - Do validation only on canonicalized paths;
>  - Run upload-pack with "." as repo, not full path;
>  - allow trailing slash under --strict-paths i.e. "git://host/my/repo.git/"
> 
> What do you think?
>

Apart from comments and indentation it's more or less exactly what I have in my revised git-daemon patch (although without what you mentioned in your own reply to this mail).

Do you want the revised one from me or will you apply the original with this on top?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231