From: Brian Warner Date: Wed, 4 Aug 2010 18:45:49 +0000 (-0700) Subject: One fix for bug #1154: webapi GETs with a 'Range' header broke new-downloader. X-Git-Tag: allmydata-tahoe-1.8.0b2~11 X-Git-Url: https://git.rkrishnan.org/simplejson/components/com_hotproperty/configuration.rst?a=commitdiff_plain;h=8844655705e4fb7633f2fda3e77c0d7bd7b9cacd;p=tahoe-lafs%2Ftahoe-lafs.git One fix for bug #1154: webapi GETs with a 'Range' header broke new-downloader. The Range header causes n.read() to be called with an offset= of type 'long', which eventually got used in a Spans/DataSpans object's __len__ method. Apparently python doesn't permit __len__() to return longs, only ints. Rewrote Spans/DataSpans to use s.len() instead of len(s) aka s.__len__() . Added a test in test_download. Note that test_web didn't catch this because it uses mock FileNodes for speed: it's probably time to rewrite that. There is still an unresolved error-recovery problem in #1154, so I'm not closing the ticket quite yet. --- diff --git a/src/allmydata/immutable/downloader/share.py b/src/allmydata/immutable/downloader/share.py index e3c90176..2cb54dea 100644 --- a/src/allmydata/immutable/downloader/share.py +++ b/src/allmydata/immutable/downloader/share.py @@ -259,7 +259,7 @@ class Share: # and sometimes you can't even get what you need disappointment = needed & self._unavailable - if len(disappointment): + if disappointment.len(): self.had_corruption = True raise DataUnavailable("need %s but will never get it" % disappointment.dump()) diff --git a/src/allmydata/test/test_download.py b/src/allmydata/test/test_download.py index 700594a2..fa42b247 100644 --- a/src/allmydata/test/test_download.py +++ b/src/allmydata/test/test_download.py @@ -321,6 +321,23 @@ class DownloadTest(_Base, unittest.TestCase): d.addCallback(_check_failover) return d + def test_long_offset(self): + # bug #1154: mplayer doing a seek-to-end results in an offset of type + # 'long', rather than 'int', and apparently __len__ is required to + # return an int. Rewrote Spans/DataSpans to provide s.len() instead + # of len(s) . + self.basedir = self.mktemp() + self.set_up_grid() + self.c0 = self.g.clients[0] + self.load_shares() + n = self.c0.create_node_from_uri(immutable_uri) + + c = MemoryConsumer() + d = n.read(c, 0L, 10L) + d.addCallback(lambda c: len("".join(c.chunks))) + d.addCallback(lambda size: self.failUnlessEqual(size, 10)) + return d + def test_badguess(self): self.basedir = self.mktemp() self.set_up_grid() diff --git a/src/allmydata/test/test_util.py b/src/allmydata/test/test_util.py index 0f973363..bac353ff 100644 --- a/src/allmydata/test/test_util.py +++ b/src/allmydata/test/test_util.py @@ -1614,8 +1614,10 @@ class SimpleSpans: if prevstart is not None: yield (prevstart, prevend-prevstart+1) - def __len__(self): - # this also gets us bool(s) + def __nonzero__(self): # this gets us bool() + return self.len() + + def len(self): return len(self._have) def __add__(self, other): @@ -1659,7 +1661,7 @@ class ByteSpans(unittest.TestCase): self.failUnlessEqual(list(s), []) self.failIf(s) self.failIf((0,1) in s) - self.failUnlessEqual(len(s), 0) + self.failUnlessEqual(s.len(), 0) s1 = Spans(3, 4) # 3,4,5,6 self._check1(s1) @@ -1672,15 +1674,15 @@ class ByteSpans(unittest.TestCase): self.failUnless((10,1) in s2) self.failIf((10,1) in s1) self.failUnlessEqual(list(s2.each()), [3,4,5,6,10,11]) - self.failUnlessEqual(len(s2), 6) + self.failUnlessEqual(s2.len(), 6) s2.add(15,2).add(20,2) self.failUnlessEqual(list(s2.each()), [3,4,5,6,10,11,15,16,20,21]) - self.failUnlessEqual(len(s2), 10) + self.failUnlessEqual(s2.len(), 10) s2.remove(4,3).remove(15,1) self.failUnlessEqual(list(s2.each()), [3,10,11,16,20,21]) - self.failUnlessEqual(len(s2), 6) + self.failUnlessEqual(s2.len(), 6) s1 = SimpleSpans(3, 4) # 3 4 5 6 s2 = SimpleSpans(5, 4) # 5 6 7 8 @@ -1690,7 +1692,7 @@ class ByteSpans(unittest.TestCase): def _check1(self, s): self.failUnlessEqual(list(s), [(3,4)]) self.failUnless(s) - self.failUnlessEqual(len(s), 4) + self.failUnlessEqual(s.len(), 4) self.failIf((0,1) in s) self.failUnless((3,4) in s) self.failUnless((3,1) in s) @@ -1817,7 +1819,7 @@ class ByteSpans(unittest.TestCase): s1 = s1 & ns1; s2 = s2 & ns2 #print "s2 now %s" % s2.dump() self.failUnlessEqual(list(s1.each()), list(s2.each())) - self.failUnlessEqual(len(s1), len(s2)) + self.failUnlessEqual(s1.len(), s2.len()) self.failUnlessEqual(bool(s1), bool(s2)) self.failUnlessEqual(list(s1), list(s2)) for j in range(10): @@ -1837,8 +1839,8 @@ class ByteSpans(unittest.TestCase): # list(s) -> list of (start,length) tuples, one per span # (start,length) in s -> True if (start..start+length-1) are all members # NOT equivalent to x in list(s) - # len(s) -> number of bytes, for testing, bool(), and accounting/limiting - # bool(s) (__len__) + # s.len() -> number of bytes, for testing, bool(), and accounting/limiting + # bool(s) (__nonzeron__) # s = s1+s2, s1-s2, +=s1, -=s1 def test_overlap(self): @@ -1893,7 +1895,9 @@ class SimpleDataSpans: for (start, data) in other.get_chunks(): self.add(start, data) - def __len__(self): + def __nonzero__(self): # this gets us bool() + return self.len() + def len(self): return len(self.missing.replace("1", "")) def _dump(self): return [i for (i,c) in enumerate(self.missing) if c == "0"] @@ -1930,7 +1934,7 @@ class SimpleDataSpans: class StringSpans(unittest.TestCase): def do_basic(self, klass): ds = klass() - self.failUnlessEqual(len(ds), 0) + self.failUnlessEqual(ds.len(), 0) self.failUnlessEqual(list(ds._dump()), []) self.failUnlessEqual(sum([len(d) for (s,d) in ds.get_chunks()]), 0) s = ds.get_spans() @@ -1939,7 +1943,7 @@ class StringSpans(unittest.TestCase): ds.remove(0, 4) ds.add(2, "four") - self.failUnlessEqual(len(ds), 4) + self.failUnlessEqual(ds.len(), 4) self.failUnlessEqual(list(ds._dump()), [2,3,4,5]) self.failUnlessEqual(sum([len(d) for (s,d) in ds.get_chunks()]), 4) s = ds.get_spans() @@ -1949,7 +1953,7 @@ class StringSpans(unittest.TestCase): self.failUnlessEqual(ds.get(4, 4), None) ds2 = klass(ds) - self.failUnlessEqual(len(ds2), 4) + self.failUnlessEqual(ds2.len(), 4) self.failUnlessEqual(list(ds2._dump()), [2,3,4,5]) self.failUnlessEqual(sum([len(d) for (s,d) in ds2.get_chunks()]), 4) self.failUnlessEqual(ds2.get(0, 4), None) @@ -1962,7 +1966,7 @@ class StringSpans(unittest.TestCase): self.failUnlessEqual(sum([len(d) for (s,d) in ds.get_chunks()]), 4) ds.add(0, "23") - self.failUnlessEqual(len(ds), 6) + self.failUnlessEqual(ds.len(), 6) self.failUnlessEqual(list(ds._dump()), [0,1,2,3,4,5]) self.failUnlessEqual(sum([len(d) for (s,d) in ds.get_chunks()]), 6) self.failUnlessEqual(ds.get(0, 4), "23fo") @@ -2125,7 +2129,7 @@ class StringSpans(unittest.TestCase): self.failUnlessEqual(d1, d2) #print "s1 now %s" % list(s1._dump()) #print "s2 now %s" % list(s2._dump()) - self.failUnlessEqual(len(s1), len(s2)) + self.failUnlessEqual(s1.len(), s2.len()) self.failUnlessEqual(list(s1._dump()), list(s2._dump())) for j in range(100): what = md5(what[12:14]+str(j)).hexdigest() diff --git a/src/allmydata/util/spans.py b/src/allmydata/util/spans.py index b09244c8..fcdd577e 100755 --- a/src/allmydata/util/spans.py +++ b/src/allmydata/util/spans.py @@ -140,7 +140,7 @@ class Spans: return self def dump(self): - return "len=%d: %s" % (len(self), + return "len=%d: %s" % (self.len(), ",".join(["[%d-%d]" % (start,start+l-1) for (start,l) in self._spans]) ) @@ -153,8 +153,12 @@ class Spans: for s in self._spans: yield s - def __len__(self): - # this also gets us bool(s) + def __nonzero__(self): # this gets us bool() + return self.len() + + def len(self): + # guess what! python doesn't allow __len__ to return a long, only an + # int. So we stop using len(spans), use spans.len() instead. return sum([length for start,length in self._spans]) def __add__(self, other): @@ -228,7 +232,10 @@ class DataSpans: for (start, data) in other.get_chunks(): self.add(start, data) - def __len__(self): + def __nonzero__(self): # this gets us bool() + return self.len() + + def len(self): # return number of bytes we're holding return sum([len(data) for (start,data) in self.spans]) @@ -239,7 +246,7 @@ class DataSpans: yield i def dump(self): - return "len=%d: %s" % (len(self), + return "len=%d: %s" % (self.len(), ",".join(["[%d-%d]" % (start,start+len(data)-1) for (start,data) in self.spans]) )