CLI: rework webopen, and moreover its tests w.r.t. path handling

in the recent reconciliation of webopen patches, I wound up adjusting
webopen to 'pass through' the state of the trailing slash on the given
argument to the resultant url passed to the browser.  this change
removes the requirement that arguments must be directories, and allows
webopen to be used with files.  it also broke the tests that assumed
that webopen would always normalise the url to have a trailing slash.

in fixing the tests, I realised that, IMHO, there's something deeply
awry with the way tahoe handles paths; specifically in the combination
of '/' being the name of the root path within an alias, but a leading
slash on paths, e.g. 'alias:/path', is catagorically incorrect. i.e.
 'tahoe:' == 'tahoe:/' == '/'
but 'tahoe:/foo' is an invalid path, and must be 'tahoe:foo'

I wound up making the internals of webopen simply spot a 'path' of
'/' and smash it to '', which 'fixes' webopen to match the behaviour
of tahoe's path handling elsewhere, but that special case sort of
points to the weirdness.

(fwiw, I personally found the fact that the leading / in a path was
disallowed to be weird - I'm just used to seeing paths qualified by
the leading / I guess - so in a debate about normalising path handling
I'd vote to include the /)

diff --git a/src/allmydata/scripts/cli.py b/src/allmydata/scripts/cli.py
index 98e489952c32adf6d50ba23fa61e6f2336a45eb1..ec74dd8af546aa1fb7906c080d2ce8e0cb8a94f0 100644 (file)
--- a/src/allmydata/scripts/cli.py
+++ b/src/allmydata/scripts/cli.py
@@ -189,7 +189,7 @@ class LnOptions(VDriveOptions):
         return "%s ln FROM TO" % (os.path.basename(sys.argv[0]),)
 
 class WebopenOptions(VDriveOptions):
-    def parseArgs(self, where=None):
+    def parseArgs(self, where=''):
         self.where = where
 
     def getSynopsis(self):

diff --git a/src/allmydata/scripts/tahoe_webopen.py b/src/allmydata/scripts/tahoe_webopen.py
index 4065660bbc4b47c78928b3d05bb10312bd64a2e0..6470bb4051daa633ccb236d4cb9b40f972777a97 100644 (file)
--- a/src/allmydata/scripts/tahoe_webopen.py
+++ b/src/allmydata/scripts/tahoe_webopen.py
@@ -7,9 +7,9 @@ def webopen(options, opener=None):
     if not nodeurl.endswith("/"):
         nodeurl += "/"
     where = options.where
-    if where is None:
-        where = 'tahoe:'
     rootcap, path = get_alias(options.aliases, where, DEFAULT_ALIAS)
+    if path == '/':
+        path = ''
     url = nodeurl + "uri/%s" % urllib.quote(rootcap)
     if path:
         url += "/" + escape_path(path)

diff --git a/src/allmydata/test/test_cli.py b/src/allmydata/test/test_cli.py
index 21e1ea7d5848dd9d74d68fb9730e7547879c2b71..9333a985141412bd06215f58fee9c251bc011524 100644 (file)
--- a/src/allmydata/test/test_cli.py
+++ b/src/allmydata/test/test_cli.py
@@ -263,20 +263,25 @@ class CreateAlias(SystemTestMixin, CLITestMixin, unittest.TestCase):
             node_url_file = os.path.join(self.getdir("client0"), "node.url")
             nodeurl = open(node_url_file, "r").read().strip()
             uribase = nodeurl + "uri/"
-            self.tahoe_url = uribase + urllib.quote(aliases["tahoe"]) + "/"
-            self.tahoe_subdir_url = self.tahoe_url + "subdir/"
-            self.two_url = uribase + urllib.quote(aliases["two"]) + "/"
+            self.tahoe_url = uribase + urllib.quote(aliases["tahoe"])
+            self.tahoe_subdir_url = self.tahoe_url + "/subdir"
+            self.two_url = uribase + urllib.quote(aliases["two"])
         d.addCallback(_stash_urls)
 
-        d.addCallback(lambda res: self._test_webopen([], self.tahoe_url))
-        d.addCallback(lambda res: self._test_webopen(["/"], self.tahoe_url))
-        d.addCallback(lambda res: self._test_webopen(["tahoe:"], self.tahoe_url))
-        d.addCallback(lambda res: self._test_webopen(["tahoe:/"], self.tahoe_url))
-        d.addCallback(lambda res: self._test_webopen(["tahoe:subdir"],
-                                                     self.tahoe_subdir_url))
-        d.addCallback(lambda res: self._test_webopen(["tahoe:subdir/"],
-                                                     self.tahoe_subdir_url))
-        d.addCallback(lambda res: self._test_webopen(["two:"], self.two_url))
+        def _test_urls(junk):
+            self._test_webopen([], self.tahoe_url)
+            self._test_webopen(["/"], self.tahoe_url)
+            self._test_webopen(["tahoe:"], self.tahoe_url)
+            self._test_webopen(["tahoe:/"], self.tahoe_url)
+            self._test_webopen(["tahoe:subdir"], self.tahoe_subdir_url)
+            self._test_webopen(["tahoe:subdir/"], self.tahoe_subdir_url + '/')
+            self._test_webopen(["tahoe:subdir/file"], self.tahoe_subdir_url + '/file')
+            # if "file" is indeed a file, then the url produced by webopen in this
+            # case is disallowed by the webui. but by design, webopen passes through
+            # the mistake from the user to the resultant webopened url
+            self._test_webopen(["tahoe:subdir/file/"], self.tahoe_subdir_url + '/file/')
+            self._test_webopen(["two:"], self.two_url)
+        d.addCallback(_test_urls)
 
         return d