From: Brian Warner Date: Tue, 25 Dec 2007 10:48:57 +0000 (-0700) Subject: refactor webish.py slightly, improve the test coverage a bit. One test is marked... X-Git-Tag: allmydata-tahoe-0.7.0~68 X-Git-Url: https://git.rkrishnan.org/simplejson/...?a=commitdiff_plain;h=6b81ebfd8e87bd8f7931597065f378b4baee5a49;p=tahoe-lafs%2Ftahoe-lafs.git refactor webish.py slightly, improve the test coverage a bit. One test is marked SKIP pending improvements in webish.py --- diff --git a/src/allmydata/test/test_web.py b/src/allmydata/test/test_web.py index edf7ad2b..d4ad068d 100644 --- a/src/allmydata/test/test_web.py +++ b/src/allmydata/test/test_web.py @@ -548,6 +548,14 @@ class Web(WebMixin, unittest.TestCase): self.NEWFILE_CONTENTS)) return d + def test_PUT_NEWFILEURL_localfile_missingarg(self): + url = self.public_url + "/foo/new.txt?t=upload" + d = self.shouldHTTPError2("test_PUT_NEWFILEURL_localfile_missing", + 400, "Bad Request", + "t=upload requires localfile= or localdir=", + self.PUT, url, "") + return d + def test_PUT_NEWFILEURL_localfile_disabled(self): localfile = os.path.abspath("web/PUT_NEWFILEURL_local file_disabled") url = (self.public_url + "/foo/new.txt?t=upload&localfile=%s" % @@ -596,6 +604,13 @@ class Web(WebMixin, unittest.TestCase): d.addCallback(_check2) return d + def test_GET_FILEURL_badtype(self): + d = self.shouldHTTPError2("GET t=bogus", 400, "Bad Request", + "bad t=bogus", + self.GET, + self.public_url + "/foo/bar.txt?t=bogus") + return d + def test_GET_FILEURL_uri_missing(self): d = self.GET(self.public_url + "/foo/missing?t=uri") d.addBoth(self.should404, "test_GET_FILEURL_uri_missing") @@ -634,6 +649,14 @@ class Web(WebMixin, unittest.TestCase): return d + def test_GET_DIRURL_badtype(self): + d = self.shouldHTTPError2("test_GET_DIRURL_badtype", + 400, "Bad Request", + "bad t=bogus", + self.GET, + self.public_url + "/foo?t=bogus") + return d + def test_GET_DIRURL_large(self): # Nevow has a problem showing more than about 192 children of a # directory: it uses defer.success() and d.addCallback in a way that @@ -817,6 +840,14 @@ class Web(WebMixin, unittest.TestCase): d.addCallback(_check) return d + def test_GET_DIRURL_localdir_nolocaldir(self): + d = self.shouldHTTPError2("GET_DIRURL_localdir_nolocaldir", + 400, "Bad Request", + "t=download requires localdir=", + self.GET, + self.public_url + "/foo?t=download") + return d + def touch(self, localdir, filename): path = os.path.join(localdir, filename) f = open(path, "wb") @@ -954,6 +985,18 @@ class Web(WebMixin, unittest.TestCase): "contents of three/bar.txt\n")) return d + def test_PUT_NEWDIRURL_localdir_missing(self): + raise unittest.SkipTest("fix PUTHandler._upload_localdir to return " + "an error instead of silently passing") + localdir = os.path.abspath("web/PUT_NEWDIRURL_localdir_missing") + # we do *not* create it, to trigger an error + url = (self.public_url + "/foo/subdir/newdir?t=upload&localdir=%s" + % urllib.quote(localdir)) + d = self.shouldHTTPError2("test_PUT_NEWDIRURL_localdir_missing", + 400, "Bad Request", "random", + self.PUT, url, "") + return d + def test_POST_upload(self): d = self.POST(self.public_url + "/foo", t="upload", file=("new.txt", self.NEWFILE_CONTENTS)) @@ -1066,6 +1109,14 @@ class Web(WebMixin, unittest.TestCase): self.NEWFILE_CONTENTS)) return d + def test_POST_upload_no_replace_ok(self): + d = self.POST(self.public_url + "/foo?replace=false", t="upload", + file=("new.txt", self.NEWFILE_CONTENTS)) + d.addCallback(lambda res: self.GET(self.public_url + "/foo/new.txt")) + d.addCallback(lambda res: self.failUnlessEqual(res, + self.NEWFILE_CONTENTS)) + return d + def test_POST_upload_no_replace_queryarg(self): d = self.POST(self.public_url + "/foo?replace=false", t="upload", file=("bar.txt", self.NEWFILE_CONTENTS)) @@ -1515,3 +1566,15 @@ class Web(WebMixin, unittest.TestCase): pass d.addCallback(_done) return d + + def test_bad_method(self): + url = self.webish_url + self.public_url + "/foo/bar.txt" + d = self.shouldHTTPError2("test_bad_method", 404, "Not Found", None, + client.getPage, url, method="BOGUS") + return d + + def test_short_url(self): + url = self.webish_url + "/uri" + d = self.shouldHTTPError2("test_short_url", 404, "Not Found", None, + client.getPage, url, method="DELETE") + return d diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index 06c3b8f7..84bd4aac 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -906,6 +906,11 @@ class PUTHandler(rend.Page): localfile = self._localfile localdir = self._localdir + if t == "upload" and not (localfile or localdir): + req.setResponseCode(http.BAD_REQUEST) + req.setHeader("content-type", "text/plain") + return "t=upload requires localfile= or localdir=" + # we must traverse the path, creating new directories as necessary d = self._get_or_create_directories(self._node, self._path[:-1]) name = self._path[-1] @@ -913,12 +918,11 @@ class PUTHandler(rend.Page): if t == "upload": if localfile: d.addCallback(self._upload_localfile, localfile, name) - elif localdir: + else: + # localdir # take the last step d.addCallback(self._get_or_create_directories, self._path[-1:]) d.addCallback(self._upload_localdir, localdir) - else: - raise RuntimeError("t=upload requires localfile= or localdir=") elif t == "uri": d.addCallback(self._attach_uri, req.content, name) elif t == "mkdir": @@ -1063,6 +1067,19 @@ class Manifest(rend.Page): ctx.fillSlots("refresh_capability", refresh_cap) return ctx.tag +class ChildError: + implements(inevow.IResource) + def renderHTTP(self, ctx): + req = inevow.IRequest(ctx) + req.setResponseCode(http.BAD_REQUEST) + req.setHeader("content-type", "text/plain") + return self.text + +def child_error(text): + ce = ChildError() + ce.text = text + return ce, () + class VDrive(rend.Page): def __init__(self, node, name): @@ -1079,13 +1096,6 @@ class VDrive(rend.Page): method = req.method path = segments - # when we're pointing at a directory (like /uri/$DIR_URI/my_pix), - # Directory.addSlash causes a redirect to /uri/$DIR_URI/my_pix/, - # which appears here as ['my_pix', '']. This is supposed to hit the - # same Directory as ['my_pix']. - if path and path[-1] == '': - path = path[:-1] - t = get_arg(req, "t", "") localfile = get_arg(req, "localfile", None) if localfile is not None: @@ -1131,13 +1141,13 @@ class VDrive(rend.Page): elif t == "readonly-uri": return FileReadOnlyURI(node), () else: - raise RuntimeError("bad t=%s" % t) + return child_error("bad t=%s" % t) elif IDirectoryNode.providedBy(node): if t == "download": if localdir: # recursive download to a local directory return LocalDirectoryDownloader(node, localdir), () - raise RuntimeError("t=download requires localdir=") + return child_error("t=download requires localdir=") elif t == "": # send an HTML representation of the directory return Directory(self.name, node, path), () @@ -1152,9 +1162,9 @@ class VDrive(rend.Page): elif t == 'rename-form': return RenameForm(self.name, node, path), () else: - raise RuntimeError("bad t=%s" % t) + return child_error("bad t=%s" % t) else: - raise RuntimeError("unknown node type") + return child_error("unknown node type") d.addCallback(file_or_dir) elif method == "POST": # the node must exist, and our operation will be performed on the @@ -1177,10 +1187,6 @@ class VDrive(rend.Page): return PUTHandler(self.node, path, t, localfile, localdir, replace), () else: return rend.NotFound - def _trap_KeyError(f): - f.trap(KeyError) - return rend.FourOhFour(), () - d.addErrback(_trap_KeyError) return d class URIPUTHandler(rend.Page):