← Back to team overview

dulwich-users team mailing list archive

Re: [PATCH v3 3/4] repo.Repo.get_named_file: normalize case

 

On Dec 26, 2010, at 12:45 PM, Jelmer Vernooij wrote:
> 
> On Mon, Dec 27, 2010 at 12:45:14AM +0800, Tay Ray Chuan wrote:
>> On Mon, Dec 27, 2010 at 12:36 AM, Jelmer Vernooij <jelmer@xxxxxxxxx> wrote:
>>> On Mon, Dec 27, 2010 at 12:15:45AM +0800, Tay Ray Chuan wrote:
>> [snip]
>>>> +def _norm_path(path):
>>>> + ? ?return os.path.normcase(os.path.realpath(path))
>>> Thanks for the patches.
>>> 
>>> I'm not sure this is a useful thing to factor out.
>> 
>> It makes things neater. In the next patch (#4), we go through the
>> whole gamut again for the parent directory.
>> 
>>> Also, why the os.path.realpath? We're just going to open these files, why do we care
>>> about their canonical location?
>> 
>> A malicious user could ask for an path like
>> 
>>  /../some/file
>> 
>> realpath "escapes" these for us.
> dulwich.rpeo is the wrong place for server-side permission checks IMHO. At the 
> very least we should raise an exception if the path is outside of the repository rather than returning None.
> 
> In genera though, I think we should put checks like this into the server rather than here.

+1 on both points.

> 
> Cheers,
> 
> Jelmer
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~dulwich-users
> Post to     : dulwich-users@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~dulwich-users
> More help   : https://help.launchpad.net/ListHelp





Follow ups

References