From: Brian Warner Date: Wed, 18 Jun 2008 02:49:40 +0000 (-0700) Subject: web: stop using absolute links (or url.here) in forms and pages, since they break... X-Git-Tag: allmydata-tahoe-1.2.0~79 X-Git-Url: https://git.rkrishnan.org/components/com_hotproperty/flags/reliability?a=commitdiff_plain;h=5bdff74e5b1bbd6b1f42fd54fd6abfba4da11a9d;p=tahoe-lafs%2Ftahoe-lafs.git web: stop using absolute links (or url.here) in forms and pages, since they break behind proxies. Partially addresses #461 --- diff --git a/src/allmydata/test/test_web.py b/src/allmydata/test/test_web.py index a8d1b3e7..326689a3 100644 --- a/src/allmydata/test/test_web.py +++ b/src/allmydata/test/test_web.py @@ -129,6 +129,7 @@ class WebMixin(object): foo.set_uri(u"empty", res[3][1].get_uri()) sub_uri = res[4][1].get_uri() + self._sub_uri = sub_uri foo.set_uri(u"sub", sub_uri) sub = self.s.create_node_from_uri(sub_uri) @@ -726,18 +727,43 @@ class Web(WebMixin, unittest.TestCase): def test_GET_DIRURL(self): # the addSlash means we get a redirect here + # from /uri/$URI/foo/ , we need ../../../ to get back to the root + ROOT = "../../.." d = self.GET(self.public_url + "/foo", followRedirect=True) def _check(res): + self.failUnless(('Return to Welcome page' % ROOT) + in res, res) # the FILE reference points to a URI, but it should end in bar.txt - self.failUnless(re.search(r'' - 'bar.txt' - '' - '\s+FILE' - '\s+%d' % len(self.BAR_CONTENTS) - , res)) + bar_url = ("%s/file/%s/@@named=/bar.txt" % + (ROOT, urllib.quote(self._bar_txt_uri))) + get_bar = "".join([r'', + r'bar.txt' % bar_url, + r'', + r'\s+FILE', + r'\s+%d' % len(self.BAR_CONTENTS), + ]) + self.failUnless(re.search(get_bar, res), res) + for line in res.split("\n"): + # find the line that contains the delete button for bar.txt + if ("form action" in line and + 'value="delete"' in line and + 'value="bar.txt"' in line): + # the form target should use a relative URL + foo_url = urllib.quote("%s/uri/%s/" % (ROOT, self._foo_uri)) + self.failUnless(('action="%s"' % foo_url) in line, line) + # and the when_done= should too + #done_url = urllib.quote(???) + #self.failUnless(('name="when_done" value="%s"' % done_url) + # in line, line) + break + else: + self.fail("unable to find delete-bar.txt line", res) + # the DIR reference just points to a URI - self.failUnless(re.search(r'sub' - '\s+DIR', res)) + sub_url = ("%s/uri/%s/" % (ROOT, urllib.quote(self._sub_uri))) + get_sub = ((r'sub' % sub_url) + + r'\s+DIR') + self.failUnless(re.search(get_sub, res), res) d.addCallback(_check) # look at a directory which is readonly @@ -752,7 +778,7 @@ class Web(WebMixin, unittest.TestCase): d.addCallback(lambda res: self.GET(self.public_url, followRedirect=True)) def _check3(res): - self.failUnless(re.search(r'reedownlee' + self.failUnless(re.search(r'reedownlee' '\s+DIR-RO', res)) d.addCallback(_check3) @@ -1166,15 +1192,25 @@ class Web(WebMixin, unittest.TestCase): formaction=mo.group(1) formwhendone=mo.group(2) - fileurl = "/uri/" + urllib.quote(self._mutable_uri) - self.failUnless(formaction.startswith(fileurl), formaction) - return self.POST(formaction, + fileurl = "../../../uri/" + urllib.quote(self._mutable_uri) + self.failUnlessEqual(formaction, fileurl) + # to POST, we need to absoluteify the URL + new_formaction = "/uri/%s" % urllib.quote(self._mutable_uri) + self.failUnlessEqual(formwhendone, + "../uri/%s/" % urllib.quote(self._foo_uri)) + return self.POST(new_formaction, t="upload", file=("new.txt", EVEN_NEWER_CONTENTS), when_done=formwhendone, followRedirect=False) d.addCallback(_parse_overwrite_form_and_submit) - d.addBoth(self.shouldRedirect, urllib.quote(self.public_url + "/foo/")) + # This will redirect us to ../uri/$FOOURI, rather than + # ../uri/$PARENT/foo, but apparently twisted.web.client absolutifies + # the redirect for us, and remember that shouldRedirect prepends + # self.webish_url for us. + d.addBoth(self.shouldRedirect, + "/uri/%s/" % urllib.quote(self._foo_uri), + which="test_POST_upload_mutable.overwrite") d.addCallback(lambda res: self.failUnlessMutableChildContentsAre(fn, u"new.txt", EVEN_NEWER_CONTENTS)) @@ -1679,21 +1715,24 @@ class Web(WebMixin, unittest.TestCase): d.addCallback(self.failUnlessIsFooJSON) return d - def shouldRedirect(self, res, target=None, statuscode=None): + def shouldRedirect(self, res, target=None, statuscode=None, which=""): """ If target is not None then the redirection has to go to target. If statuscode is not None then the redirection has to be accomplished with that HTTP status code.""" if not isinstance(res, failure.Failure): - self.fail("we were expecting to get redirected %s, not get an" - " actual page: %s" % ((target is None) and "somewhere" or ("to " + target), res)) + to_where = (target is None) and "somewhere" or ("to " + target) + self.fail("%s: we were expecting to get redirected %s, not get an" + " actual page: %s" % (which, to_where, res)) res.trap(error.PageRedirect) if statuscode is not None: - self.failUnlessEqual(res.value.status, statuscode) + self.failUnlessEqual(res.value.status, statuscode, + "%s: not a redirect" % which) if target is not None: # the PageRedirect does not seem to capture the uri= query arg # properly, so we can't check for it. realtarget = self.webish_url + target - self.failUnlessEqual(res.value.location, realtarget) + self.failUnlessEqual(res.value.location, realtarget, + "%s: wrong target" % which) return res.value.location def test_GET_URI_form(self): @@ -1725,9 +1764,9 @@ class Web(WebMixin, unittest.TestCase): def test_GET_rename_form(self): d = self.GET(self.public_url + "/foo?t=rename-form&name=bar.txt", - followRedirect=True) # XXX [ ] todo: figure out why '.../foo' doesn't work + followRedirect=True) def _check(res): - self.failUnless(re.search(r'name="when_done" value=".*%s/foo/' % (urllib.quote(self.public_url),), res), (r'name="when_done" value=".*%s/foo/' % (urllib.quote(self.public_url),), res,)) + self.failUnless('name="when_done" value="."' in res, res) self.failUnless(re.search(r'name="from_name" value="bar\.txt"', res)) d.addCallback(_check) return d diff --git a/src/allmydata/web/directory.py b/src/allmydata/web/directory.py index 5fe2db63..8a55df1c 100644 --- a/src/allmydata/web/directory.py +++ b/src/allmydata/web/directory.py @@ -385,9 +385,15 @@ class DirectoryAsHTML(rend.Page): header.append(" (readonly)") return ctx.tag[header] - def render_welcome(self, ctx, data): - depth = len(IRequest(ctx).path) + 2 + def get_root(self, ctx): + req = IRequest(ctx) + # the addSlash=True gives us one extra (empty) segment + depth = len(req.prepath) + len(req.postpath) - 1 link = "/".join([".."] * depth) + return link + + def render_welcome(self, ctx, data): + link = self.get_root(ctx) return T.div[T.a(href=link)["Return to Welcome page"]] def data_children(self, ctx, data): @@ -419,6 +425,8 @@ class DirectoryAsHTML(rend.Page): name = name.encode("utf-8") assert not isinstance(name, unicode) + root = self.get_root(ctx) + here = "%s/uri/%s/" % (root, urllib.quote(self.node.get_uri())) if self.node.is_readonly(): delete = "-" rename = "-" @@ -426,25 +434,31 @@ class DirectoryAsHTML(rend.Page): # this creates a button which will cause our child__delete method # to be invoked, which deletes the file and then redirects the # browser back to this directory - delete = T.form(action=url.here, method="post")[ + delete = T.form(action=here, method="post")[ T.input(type='hidden', name='t', value='delete'), T.input(type='hidden', name='name', value=name), - T.input(type='hidden', name='when_done', value=url.here), + T.input(type='hidden', name='when_done', value="."), T.input(type='submit', value='del', name="del"), ] - rename = T.form(action=url.here, method="get")[ + rename = T.form(action=here, method="get")[ T.input(type='hidden', name='t', value='rename-form'), T.input(type='hidden', name='name', value=name), - T.input(type='hidden', name='when_done', value=url.here), + T.input(type='hidden', name='when_done', value="."), T.input(type='submit', value='rename', name="rename"), ] ctx.fillSlots("delete", delete) ctx.fillSlots("rename", rename) - check = T.form(action=url.here.child(name), method="post")[ + if IDirectoryNode.providedBy(target): + check_url = "%s/uri/%s/" % (root, urllib.quote(target.get_uri())) + check_done_url = "../../uri/%s/" % urllib.quote(self.node.get_uri()) + else: + check_url = "%s/uri/%s" % (root, urllib.quote(target.get_uri())) + check_done_url = "../uri/%s/" % urllib.quote(self.node.get_uri()) + check = T.form(action=check_url, method="post")[ T.input(type='hidden', name='t', value='check'), - T.input(type='hidden', name='when_done', value=url.here), + T.input(type='hidden', name='when_done', value=check_done_url), T.input(type='submit', value='check', name="check"), ] ctx.fillSlots("overwrite", @@ -475,7 +489,7 @@ class DirectoryAsHTML(rend.Page): # to prevent javascript in displayed .html files from stealing a # secret directory URI from the URL, send the browser to a URI-based # page that doesn't know about the directory at all - dlurl = "/file/%s/@@named=/%s" % (quoted_uri, urllib.quote(name)) + dlurl = "%s/file/%s/@@named=/%s" % (root, quoted_uri, urllib.quote(name)) ctx.fillSlots("filename", T.a(href=dlurl)[html.escape(name)]) @@ -483,11 +497,11 @@ class DirectoryAsHTML(rend.Page): ctx.fillSlots("size", "?") - text_plain_url = "/file/%s/@@named=/foo.txt" % quoted_uri + text_plain_url = "%s/file/%s/@@named=/foo.txt" % (root, quoted_uri) text_plain_tag = T.a(href=text_plain_url)["text/plain"] elif IFileNode.providedBy(target): - dlurl = "/file/%s/@@named=/%s" % (quoted_uri, urllib.quote(name)) + dlurl = "%s/file/%s/@@named=/%s" % (root, quoted_uri, urllib.quote(name)) ctx.fillSlots("filename", T.a(href=dlurl)[html.escape(name)]) @@ -495,13 +509,13 @@ class DirectoryAsHTML(rend.Page): ctx.fillSlots("size", target.get_size()) - text_plain_url = "/file/%s/@@named=/foo.txt" % quoted_uri + text_plain_url = "%s/file/%s/@@named=/foo.txt" % (root, quoted_uri) text_plain_tag = T.a(href=text_plain_url)["text/plain"] elif IDirectoryNode.providedBy(target): # directory - uri_link = "/uri/" + urllib.quote(target.get_uri()) + "/" + uri_link = "%s/uri/%s/" % (root, urllib.quote(target.get_uri())) ctx.fillSlots("filename", T.a(href=uri_link)[html.escape(name)]) if target.is_readonly(): @@ -562,7 +576,7 @@ class DirectoryAsHTML(rend.Page): enctype="multipart/form-data")[ T.fieldset[ T.input(type="hidden", name="t", value="mkdir"), - T.input(type="hidden", name="when_done", value=url.here), + T.input(type="hidden", name="when_done", value="."), T.legend(class_="freeform-form-label")["Create a new directory"], "New directory name: ", T.input(type="text", name="name"), " ", @@ -573,7 +587,7 @@ class DirectoryAsHTML(rend.Page): enctype="multipart/form-data")[ T.fieldset[ T.input(type="hidden", name="t", value="upload"), - T.input(type="hidden", name="when_done", value=url.here), + T.input(type="hidden", name="when_done", value="."), T.legend(class_="freeform-form-label")["Upload a file to this directory"], "Choose a file to upload: ", T.input(type="file", name="file", class_="freeform-input-file"), @@ -587,7 +601,7 @@ class DirectoryAsHTML(rend.Page): enctype="multipart/form-data")[ T.fieldset[ T.input(type="hidden", name="t", value="uri"), - T.input(type="hidden", name="when_done", value=url.here), + T.input(type="hidden", name="when_done", value="."), T.legend(class_="freeform-form-label")["Attach a file or directory" " (by URI) to this" " directory"], @@ -604,12 +618,14 @@ class DirectoryAsHTML(rend.Page): def build_overwrite_form(self, ctx, name, target): if IMutableFileNode.providedBy(target) and not target.is_readonly(): - action = "/uri/" + urllib.quote(target.get_uri()) + root = self.get_root(ctx) + action = "%s/uri/%s" % (root, urllib.quote(target.get_uri())) + done_url = "../uri/%s/" % urllib.quote(self.node.get_uri()) overwrite = T.form(action=action, method="post", enctype="multipart/form-data")[ T.fieldset[ T.input(type="hidden", name="t", value="upload"), - T.input(type='hidden', name='when_done', value=url.here), + T.input(type='hidden', name='when_done', value=done_url), T.legend(class_="freeform-form-label")["Overwrite"], "Choose new file: ", T.input(type="file", name="file", class_="freeform-input-file"), @@ -692,7 +708,7 @@ class RenameForm(rend.Page): return ctx.tag[header] def render_when_done(self, ctx, data): - return T.input(type="hidden", name="when_done", value=url.here) + return T.input(type="hidden", name="when_done", value=".") def render_get_name(self, ctx, data): req = IRequest(ctx)