From: robk-tahoe <robk-tahoe@allmydata.com>
Date: Wed, 24 Sep 2008 16:45:23 +0000 (-0700)
Subject: CLI: rework webopen, and moreover its tests w.r.t. path handling
X-Git-Url: https://git.rkrishnan.org/pf/content/%22news.html/-?a=commitdiff_plain;h=ed9873c2a9dc7315b661d830fdd03dd6dcef0930;p=tahoe-lafs%2Ftahoe-lafs.git

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 98e48995..ec74dd8a 100644
--- 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 4065660b..6470bb40 100644
--- 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 21e1ea7d..9333a985 100644
--- 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