[MERGE] ChrootTransportDecorator

[John Arbash Meinel]

Andrew Bennetts wrote:
> This bundle adds a ChrootTransportDecorator.  I use this in the latest revision
> of my WSGI branch, so that the smart server won't provide a way to access any
> file on your filesystem.
> 
> The bundle is relative to the MemoryTransport.abspath fix posted earlier.
> 
> -Andrew.
> 

Sorry about the delay. This patch does what it claims to do. Adding a
'chroot+' decorator on urls, and preventing you from accessing files
outside of that domain.

I feel like it is a bit of a band-aid, and not really the way things
should work. Because you suddenly have to override every command of
Transport, and really Transport was never intended to allow paths
outside of 'base', other than at 'clone()' time.

So what I would prefer from an internals POV, is to have Transport
classes audited or refactored, so that whatever function they use for
generating the 'real' path from a relative path does not support '..',
and then only 'clone()' functions are allowed to support '..'.

This would do a few things:

1) We would have to change tests that do transport.get('/foo') or
.has('/') . But those shouldn't be using absolute paths anyway, since
they aren't 'relpath', aka url fragments.

2) ChrootDecorator then only has to trap 'def clone()', rather than
trapping every function and adding all of the associated overhead.

3) We should add transport implementation tests that assert for all
functions that they don't allow an absolute path or a relative path with
'..' in it. There is no real need to support path/../foo, so I would
rather not try to support it.

4) If you want, we could actually deprecate clone('..'), in favor of
using Transport.parent_dir_transport(), and then
Transport.child_dir_transport(relpath).

I was never settled on clone(), it was just the closest I could get. The
only time we need clone('..') is when we are looking for a containing
object, and we should only be using it as clone('..'), never
clone('../path/../other/path/../')  or anything like that.


So if this is a strict requirement to getting the smart server fully
operational over http, we can merge it. I'd rather we clean up the
Transport implementations rather than wrapping another band-aid layer on
top of them, though.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20061107/ea59cb59/attachment.pgp