launchpad-dev team mailing list archive

[Michael Hudson <michael.hudson@xxxxxxxxxxxxx>]

Stuart Bishop wrote:
> 
> 
> On Thu, Nov 26, 2009 at 1:30 PM, Michael Hudson
> <michael.hudson@xxxxxxxxxxxxx> wrote:
>> Stuart Bishop wrote:
>>> On Thu, Nov 26, 2009 at 11:08 AM, Michael Hudson
>>> <michael.hudson@xxxxxxxxxxxxx> wrote:
>>>> Something that turns out to a bit annoying when you try to test bzr-svn
>>>> with Launchpad is that bzr doesn't allow a netloc part in file:// urls
>>>> and the launchpad "valid_absolute_url" insists on a netloc in all URLs
>>>> (of course it's essentially always 'localhost' in file:// URLs).  This
>>>> is a bit stupid for bzr and I'll fix it to accept file://localhost/
>>>> URLs, but would it be possible to change this for Launchpad too?
>>>
>>> I would think file: URLs are one of the things that valid_absolute_url
>>> is supposed to catch, as on the production system it would certainly
>>> indicate a mistake or an attack (the database constraint is our second
>>> layer of defense after the form validation).
>>
>> Well, we more-or-less need it for testing bzr-svn (and other) imports.
>> I don't really want to spin up an apache with the mod_dav_svn installed
>> for the test...  We've also used file://localhost/ urls to do imports
>> from disk in the past, although I think there are probably better ways
>> of doing this.
>>
>>> (I won't fix valid_absolute_url just now in case someone can point out
>>> sane use cases for allowing file: URLs to be accepted).
>>
>> It will break some code import tests, I'm fairly sure.
> 
> We should have a customized validator that allows file: then for those
> cases we know it is safe and can't be used for, say, accessing arbitrary
> files on the server. I can't think of any places where this is actually
> a problem, but I might be wrong and can't speak for the future either.

For Bazaar branches in particular, banning file:/// URLs in the database
strictly isn't enough, because an HTTP URL can point to a branch
reference that points at a file:/// URL.  So we already have a bunch of
application level code for dealing with this.

So yes, maybe for code import/Bazaar related fields we should have a
different validator in the db...

> (Just confirmed I can't mirror a bzr branch using a file:// URL, at
> least using the web UI to set it up).

And even if you got one set up, it still wouldn't be mirrored.

Cheers,
mwh