diff --git a/src/allmydata/download.py b/src/allmydata/download.py index 29ceaa79f..8b963fbb6 100644 --- a/src/allmydata/download.py +++ b/src/allmydata/download.py @@ -35,14 +35,12 @@ class Output: self._segment_number = 0 self._plaintext_hash_tree = None self._crypttext_hash_tree = None + self._opened = False def setup_hashtrees(self, plaintext_hashtree, crypttext_hashtree): self._plaintext_hash_tree = plaintext_hashtree self._crypttext_hash_tree = crypttext_hashtree - def open(self): - self.downloadable.open() - def write_segment(self, crypttext): self.length += len(crypttext) @@ -71,9 +69,13 @@ class Output: self._segment_number += 1 # We're still at 1*segment_size. The Downloadable is responsible for # any memory usage beyond this. + if not self._opened: + self._opened = True + self.downloadable.open() self.downloadable.write(plaintext) def fail(self, why): + log.msg("UNUSUAL: download failed: %s" % why) self.downloadable.fail(why) def close(self): @@ -269,7 +271,6 @@ class FileDownloader: self._num_needed_shares = d['needed_shares'] self._output = Output(downloadable, d['key']) - self._output.open() self.active_buckets = {} # k: shnum, v: bucket self._share_buckets = [] # list of (sharenum, bucket) tuples diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py index f2cb478d7..6582f9946 100644 --- a/src/allmydata/interfaces.py +++ b/src/allmydata/interfaces.py @@ -605,7 +605,9 @@ class IDecoder(Interface): class IDownloadTarget(Interface): def open(): - """Called before any calls to write(), close(), or fail().""" + """Called before any calls to write() or close(). If an error + occurs before any data is available, fail() may be called without + a previous call to open().""" def write(data): """Output some data to the target.""" def close(): diff --git a/src/allmydata/test/test_system.py b/src/allmydata/test/test_system.py index 0961a6129..59b69c728 100644 --- a/src/allmydata/test/test_system.py +++ b/src/allmydata/test/test_system.py @@ -14,7 +14,7 @@ from foolscap.eventual import flushEventualQueue from twisted.python import log from twisted.python.failure import Failure from twisted.web.client import getPage -from twisted.web.error import PageRedirect +from twisted.web.error import PageRedirect, Error def flush_but_dont_ignore(res): d = flushEventualQueue() @@ -527,9 +527,12 @@ class SystemTest(testutil.SignalMixin, unittest.TestCase): return getPage(base + "download_uri/%s?filename=%s" % (self.mangle_uri(self.uri), "mydata567")) d.addCallback(_get_from_bogus_uri) - def _got_from_bogus_uri(page): - self.failUnlessEqual(page, "problem during download\n") - d.addCallback(_got_from_bogus_uri) + d.addBoth(self.shouldFail, Error, "downloading bogus URI", + "NotEnoughPeersError") + + # TODO: mangle the second segment of a file, to test errors that + # occur after we've already sent some good data, which uses a + # different error path. # download from a URI pasted into a form. Use POST, build a # multipart/form-data, submit it. This actualy redirects us to a diff --git a/src/allmydata/webish.py b/src/allmydata/webish.py index e35c3ac14..34d1942e4 100644 --- a/src/allmydata/webish.py +++ b/src/allmydata/webish.py @@ -1,6 +1,6 @@ from twisted.application import service, strports -from twisted.web import static, resource, server, html +from twisted.web import static, resource, server, html, http from twisted.python import util, log from nevow import inevow, rend, loaders, appserver, url, tags as T from nevow.static import File as nevow_File # TODO: merge with static.File? @@ -322,27 +322,47 @@ class Directory(rend.Page): class WebDownloadTarget: implements(IDownloadTarget) - def __init__(self, req): + def __init__(self, req, content_type, content_encoding): self._req = req + self._content_type = content_type + self._content_encoding = content_encoding + self._opened = False + def open(self): - pass + self._opened = True + self._req.setHeader("content-type", self._content_type) + if self._content_encoding: + self._req.setHeader('content-encoding', self._content_encoding) + # TODO: it would be nice to set the size header here + def write(self, data): self._req.write(data) def close(self): self._req.finish() - def fail(self, why): - # I think the content-type is already set, and the response code has - # already been sent, so we can't provide a clean error indication. We - # can emit text (which a browser might interpret as something else), - # and if we sent a Size header, they might notice that we've - # truncated the data. Keep the error message small to improve the - # chances of having our error response be shorter than the intended - # results. - # - # We don't have a lot of options, unfortunately. - self._req.write("problem during download\n") + def fail(self, why): + if self._opened: + # The content-type is already set, and the response code + # has already been sent, so we can't provide a clean error + # indication. We can emit text (which a browser might interpret + # as something else), and if we sent a Size header, they might + # notice that we've truncated the data. Keep the error message + # small to improve the chances of having our error response be + # shorter than the intended results. + # + # We don't have a lot of options, unfortunately. + self._req.write("problem during download\n") + else: + # We haven't written anything yet, so we can provide a sensible + # error message. + msg = str(why.type) + msg.replace("\n", "|") + self._req.setResponseCode(http.INTERNAL_SERVER_ERROR, msg) + self._req.setHeader("content-type", "text/plain") + # TODO: HTML-formatted exception? + self._req.write(str(why)) self._req.finish() + def register_canceller(self, cb): pass def finish(self): @@ -374,12 +394,8 @@ class Downloader(resource.Resource): static.File.contentTypes, static.File.contentEncodings, defaultType="text/plain") - req.setHeader("content-type", type) - if encoding: - req.setHeader('content-encoding', encoding) - # TODO: it would be nice to set the size header here - d = self._filenode.download(WebDownloadTarget(req)) + d = self._filenode.download(WebDownloadTarget(req, type, encoding)) # exceptions during download are handled by the WebDownloadTarget d.addErrback(lambda why: None) return server.NOT_DONE_YET