[MERGE][1.0][Bug 124089] Add root_client_path parameter to SmartWSGIApp, and use it to translate paths received from clients to local paths.
John Arbash Meinel
john at arbash-meinel.com
Fri Dec 7 03:42:02 GMT 2007
John Arbash Meinel has voted tweak.
Status is now: Conditionally approved
Comment:
+
+ def translate_client_path(self, client_path):
+ """Translate a path received from a network client into a local
+ relpath.
+
+ All paths received from the client *must* be translated.
+
+ :returns: a relpath that may be used with
self._backing_transport
+ """
+ if not client_path.startswith('/'):
+ client_path = '/' + client_path
+ if client_path.startswith(self._root_client_path):
+ path = client_path[len(self._root_client_path):]
+ relpath = urlutils.joinpath('/', path)
+ assert relpath.startswith('/')
+ return '.' + relpath
+ else:
+ raise errors.PathNotChild(client_path,
self._root_client_path)
+
^- Does this work for Windows paths? The:
If not path.startswith('/'):
path = '/' + path
seems like it would break for "C:/..."
Same thing for all of the places where you are doing:
- def __init__(self, sock, backing_transport):
+ def __init__(self, sock, backing_transport, root_client_path='/'):
Actually, now that I think about it, I'm a little concerned about using
a default path of '/'. Since if someone forgets to pass it from a
function to another one, then we end up exposing everything from '/'
when it was meant to be restricted elsewhere.
So what about defaulting to None, and using a DeprecationWarning if it
is not supplied.
This seems a bit weird:
- answer = 'yes'
+ default_format = BzrDirFormat.get_default_format()
+ real_bzrdir = default_format.open(t, _found=True)
+ try:
+ real_bzrdir._format.probe_transport(t)
+ except (errors.NotBranchError, errors.UnknownFormatError):
+ answer = 'no'
+ else:
+ answer = 'yes'
You get the default format, to open it on the target location, to probe
to see if it is really there?
What if the target is not a default format? Is that okay because only
the default format is supported by remote?
...
=== modified file 'bzrlib/smart/request.py'
--- bzrlib/smart/request.py 2007-10-19 05:38:36 +0000
+++ bzrlib/smart/request.py 2007-12-05 23:30:57 +0000
@@ -24,20 +24,40 @@
errors,
registry,
revision,
+ urlutils,
)
from bzrlib.bundle.serializer import write_bundle
+from bzrlib.trace import mutter
+from bzrlib.transport import get_transport
+from bzrlib.transport.chroot import ChrootServer
class SmartServerRequest(object):
- """Base class for request handlers."""
+ """Base class for request handlers.
+
+ To define a new request, subclass this class and override the `do`
method
+ (and if appropriate, `do_body` as well). Request implementors
should take
+ care to call `translate_client_path` and
`transport_from_client_path` as
+ appropriate when dealing with paths received from the client.
+ """
- def __init__(self, backing_transport):
+ def __init__(self, backing_transport, root_client_path='/'):
"""Constructor.
:param backing_transport: the base transport to be used when
performing
this request.
+ :param root_client_path: the client path that maps to the root
of
+ backing_transport. This is used to interpret relpaths
received
+ from the client. Clients will not be able to refer to
paths above
+ this root.
"""
+ rcp = root_client_path
self._backing_transport = backing_transport
+ if not root_client_path.startswith('/'):
+ root_client_path = '/' + root_client_path
+ if not root_client_path.endswith('/'):
+ root_client_path += '/'
+ self._root_client_path = root_client_path
^- You don't use "rcp" again
Also, the same issue about forcing paths to start with '/' causing
problems on Windows. I do understand forcing them to end with a '/'.
...
=== modified file 'bzrlib/smart/server.py'
--- bzrlib/smart/server.py 2007-07-20 03:20:20 +0000
+++ bzrlib/smart/server.py 2007-12-05 23:30:57 +0000
@@ -63,6 +63,7 @@
self.backing_transport = backing_transport
self._started = threading.Event()
self._stopped = threading.Event()
+ self.root_client_path = '/'
^- This doesn't quite seem right either.
...
=== modified file 'bzrlib/transport/http/wsgi.py'
--- bzrlib/transport/http/wsgi.py 2007-08-15 19:16:00 +0000
+++ bzrlib/transport/http/wsgi.py 2007-12-05 01:58:00 +0000
@@ -85,11 +85,14 @@
class SmartWSGIApp(object):
"""A WSGI application for the bzr smart server."""
- def __init__(self, backing_transport):
+ def __init__(self, backing_transport, root_client_path='/'):
"""Constructor.
:param backing_transport: a transport. Requests will be
processed
relative to this transport.
+ :param root_client_path: the client path that maps to the root
of
+ backing_transport. This is used to interpret relpaths
received from
+ the client.
"""
^- It would be really nice to have an example here.
Also, as you mentioned, the documentation http_smart_server.txt needs to
be updated. And it would seem that we need at least a little bit of
testing that all supported function properly "translate_client_path()".
It is kind of a shame that each implementation needs to repeat that
code.
For details, see:
http://bundlebuggy.aaronbentley.com/request/%3C20071206003415.GA23196%40steerpike.home.puzzling.org%3E
More information about the bazaar
mailing list