Skip to content

GetFilters desyncs on padded filter handles, so tables written by vanilla Bedrock fail to open #12

Description

@NiclasOlofsson

Summary

Database.Open() throws ArgumentException: An item with the same key has already been added. Key: on any table whose metaindex carries a bloom filter entry, which is what vanilla Bedrock and BDS write. Tables written by PocketMine-MP have no filter and an empty metaindex, so they never hit this and the bug stays invisible until you open a world saved by the real game.

System.ArgumentException: An item with the same key has already been added. Key:
   at System.Collections.Generic.Dictionary`2.Add(TKey key, TValue value)
   at MiNET.LevelDB.Table.GetFilters()
   at MiNET.LevelDB.Table.Initialize()
   at MiNET.LevelDB.Manifest.Load(LogReader reader)
   at MiNET.LevelDB.Database.Open()

Worlds that reproduce it

Four worlds in the public NetherGames asset drop, https://github.com/NetherGamesMC/assets (CC BY 4.0), all under Skywars/arenas/:

  • SW-Azure
  • SW-Overcomplicated
  • SW-Sentiment
  • SWD-Stratosphere

Every other world in that repository opens. The four that fail are the ones last written by a Bedrock client or BDS rather than by PocketMine, which is the same split as filter present versus filter absent.

What the data actually says

SW-Azure/db/000005.ldb, footer points the metaindex at offset 410093, size 46, compression type 4. Decompressed it is 65 bytes:

00221466696c7465722e6c6576656c64622e4275696c74696e426c6f6f6d46696c74657232
fcf816ec8a0200000000000000000000000000000000000001000000

Decoded: one entry, shared=0, nonShared=34, valueLen=20, key filter.leveldb.BuiltinBloomFilter2, value 20 bytes. The first 6 bytes of the value are the block handle (offset 375932, length 34156) and the remaining 14 are zero. The entry region ends at 57, followed by a one-entry restart array and the restart count, 65 total. The block is well formed and nothing is corrupt.

The padding is BlockHandle::kMaxEncodedLength. Mojang's fork encodes the handle into a maximum-size buffer and stores it without trimming.

Why that is legal

A metaindex value is a length-prefixed opaque string. Upstream LevelDB decodes the handle out of the value slice, not out of the block stream, so trailing bytes are never looked at:

https://github.com/google/leveldb/blob/main/table/table.cc Table::ReadMeta seeks to the filter key and calls ReadFilter(iter->value()), and the same file states the principle for the index block: "We intentionally allow extra stuff in index_value so that we can add more features in the future."

Cause

MiNET.LevelDB/Table.cs, GetFilters():

ulong size = reader.ReadVarLong();
...
ReadOnlySpan<byte> keyData = reader.Read(shared, nonShared);
var handle = BlockHandle.ReadBlockHandle(ref reader);
result.Add(Encoding.UTF8.GetString(keyData), handle);

size is read but never used to advance. ReadBlockHandle consumes only the two varints, so the 14 padding bytes stay in the reader and are parsed as further entries: shared=0, nonShared=0, valueLen=0 yields an empty key, twice, and the second Add throws.

Reading the handle from a slice bounded by size, and continuing the loop at value start plus size, matches what upstream does and fixes it.

Also in the same loop

if (shared != 0) throw new Exception(...) means a metaindex that uses key prefix sharing fails outright. It has not bitten yet because a metaindex normally holds a single entry, but it is the same assumption.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions