CouchDB
  1. CouchDB
  2. COUCHDB-1319

Headers larger than 4k cannot be retrieved

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.1, 1.1.1
    • Fix Version/s: 1.2, 1.2.1
    • Component/s: None
    • Labels:
      None

      Description

      Our headers start with a <<1>> and then four bytes indicating the length of the header and its checksum. When the header is larger than 4090 bytes it will be split across multiple blocks in the file and will need to be reassembled on read. The reassembly consists of stripping out <<0>> from the beginning of each subsequent block in the remove_block_prefixes/2 function. The bug here is that we tell remove_block_prefixes that we're starting 1 byte into the current block instead of 5, so it ends up removing one or more good bytes from the header and injecting one or more random <<0>>s.

      Headers larger than 4k are very rare and generally require a view group with a huge number of indexes or indexes with fairly large reductions, which explains why this bug has gone undetected until now.

      Patch forthcoming.

        Activity

        Robert Newson made changes -
        Fix Version/s 1.2.1 [ 12319457 ]
        Fix Version/s 1.1.2 [ 12318873 ]
        Hide
        Adam Kocoloski added a comment -

        Yeah ... the important thing is it got reviewed. Pretty sure Bob, Bob, Paul and Filipe all gave it a once-over. And yes, it's a simple bug with a simple test, but I think changes to "core" pretty much always need to be reviewed at this point.

        Show
        Adam Kocoloski added a comment - Yeah ... the important thing is it got reviewed. Pretty sure Bob, Bob, Paul and Filipe all gave it a once-over. And yes, it's a simple bug with a simple test, but I think changes to "core" pretty much always need to be reviewed at this point.
        Hide
        Paul Joseph Davis added a comment -

        It's a straightforward bug and has a test. I wouldn't have even bothered with a ticket.

        Show
        Paul Joseph Davis added a comment - It's a straightforward bug and has a test. I wouldn't have even bothered with a ticket.
        Hide
        Jan Lehnardt added a comment -

        Thanks Adam.

        In the future, it wouldn't hurt to ask the side channels to comment here, so we can all see them

        Show
        Jan Lehnardt added a comment - Thanks Adam. In the future, it wouldn't hurt to ask the side channels to comment here, so we can all see them
        Adam Kocoloski made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Adam Kocoloski made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Adam Kocoloski added a comment -

        Got some positive reviews of the patch in side channels so I've gone ahead and landed it on 1.1.x, 1.2.x and master

        Show
        Adam Kocoloski added a comment - Got some positive reviews of the patch in side channels so I've gone ahead and landed it on 1.1.x, 1.2.x and master
        Hide
        Adam Kocoloski added a comment -

        Added an etap test to the branch.

        Show
        Adam Kocoloski added a comment - Added an etap test to the branch.
        Robert Newson made changes -
        Field Original Value New Value
        Fix Version/s 1.1.2 [ 12318873 ]
        Affects Version/s 1.1.1 [ 12316395 ]
        Hide
        Robert Newson added a comment -

        Mark for fixing in next 1.1.x maintenance release.

        Show
        Robert Newson added a comment - Mark for fixing in next 1.1.x maintenance release.
        Hide
        Adam Kocoloski added a comment -

        http://git-wip-us.apache.org/repos/asf?p=couchdb.git;a=shortlog;h=refs/heads/1319-large-headers-are-corrupted

        Will look into an etap test. I think we may also want to log when load_header crashes. That's not normal.

        Show
        Adam Kocoloski added a comment - http://git-wip-us.apache.org/repos/asf?p=couchdb.git;a=shortlog;h=refs/heads/1319-large-headers-are-corrupted Will look into an etap test. I think we may also want to log when load_header crashes. That's not normal.
        Adam Kocoloski created issue -

          People

          • Assignee:
            Adam Kocoloski
            Reporter:
            Adam Kocoloski
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development