#1540 closed defect (fixed)

MDMF: exception in unpack_checkstring being dropped

Reported by: davidsarah Owned by: davidsarah
Priority: major Milestone: 1.9.0
Component: code-mutable Version: 1.9.0a1
Keywords: error mdmf review-needed Cc:
Launchpad Bug:

Description

Split from #1534.

davidsarah wrote:

layout.py defines:

def unpack_checkstring(checkstring):
    cs_len = struct.calcsize(PREFIX)
    version, seqnum, root_hash, IV = struct.unpack(PREFIX, checkstring[:cs_len])
    if version != 0: # TODO: just ignore the share
        raise UnknownVersionError("got mutable share version %d, but I only understand version 0" % version)
    return (seqnum, root_hash, IV)

but it is called by src/allmydata/mutable/publish.py@5231#L1105 in a context that doesn't seem to be specific to SDMF. Why doesn't that raise an UnknownVersionError?

kevan wrote:

The caller in publish handles the case in which the remote write fails (e.g., because of an uncoordinated write error, share corruption, or something else). It's possible that we don't have tests to exercise that, or that the exception is getting lost in the deferred chain somewhere. I'll take a look at it.

[...]

It seems that both of these contribute to the missing exception. http://tahoe-lafs.org/trac/tahoe-lafs/attachment/ticket/1534/1534-dropped-error-and-tests.darcs.patch tweaks the control flow in the publisher a little bit so the exceptions don't get hidden, and then adds a test to exercise the code for MDMF files. UnknownVersionError still doesn't get raised, though, since the MDMF checkstring is shorter than the SDMF checkstring and causes struct.unpack to give up before the version can be extracted and checked. I'll work on a patch to make the new test pass.

Attachments (1)

fix-unpack-checkstring.darcs.patch (25.2 KB) - added by kevan at 2011-09-25T00:45:20Z.
break unpack_checkstring into unpack_sdmf_checkstring and unpack_mdmf_checkstring, misc. other cleanups

Download all attachments as: .zip

Change History (6)

comment:1 Changed at 2011-09-23T21:17:50Z by david-sarah@…

In 4af626a798c3cfa9:

(The changeset message doesn't reference this ticket)

Changed at 2011-09-25T00:45:20Z by kevan

break unpack_checkstring into unpack_sdmf_checkstring and unpack_mdmf_checkstring, misc. other cleanups

comment:2 Changed at 2011-09-25T00:46:05Z by kevan

  • Keywords review-needed added
  • Owner kevan deleted

comment:3 Changed at 2011-09-25T02:00:40Z by davidsarah

  • Owner set to davidsarah
  • Status changed from new to assigned

comment:4 Changed at 2011-09-25T04:56:01Z by kevan@…

  • Resolution set to fixed
  • Status changed from assigned to closed

In a911e15783e6fca7:

mutable/publish: use unpack_mdmf_checkstring and unpack_sdmf_checkstring instead of unpack_checkstring. fixes #1540

comment:5 Changed at 2011-09-25T04:56:01Z by david-sarah@…

In c88adf0ac0e10000:

(The changeset message doesn't reference this ticket)

Note: See TracTickets for help on using tickets.