You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2026/08/04 22:40:06 UTC
(tinkerpop) 08/10: Add integrity checks to TinkerStorageGraph storage format
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch tinkergraph-storage
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 11589aa9d131ebc950e705954d3bf74ea9ad4a4f
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Tue Aug 4 17:29:56 2026 +0000
Add integrity checks to TinkerStorageGraph storage format
Storage files could silently misread corrupt data: a bit-flip inside a frame
went undetected, and a garbage frame length was indistinguishable from a
truncated trailing append. Each frame now carries a CRC32, and every file
starts with a magic + version header. On read, a complete frame with a bad
CRC or a file with bad magic / an unknown version fails loudly, while a short
trailing frame is still tolerated as an interrupted append. The payload
allocation is bounded by the remaining file bytes, so a corrupt length can no
longer trigger a huge allocation.
The per-record version byte is dropped in favor of the file header. This
changes the on-disk format, which is safe as the storage feature is unreleased.
Assisted-by: Claude Code:claude-opus-4-8
---
.../structure/storage/GraphBinaryStorage.java | 119 ++++++++++++++++-----
.../structure/storage/GraphBinaryStorageTest.java | 70 +++++++++++-
2 files changed, 159 insertions(+), 30 deletions(-)
diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorage.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorage.java
index 074b67bc34..715dc48c74 100644
--- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorage.java
+++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorage.java
@@ -49,10 +49,12 @@ import java.nio.file.AtomicMoveNotSupportedException;
import java.nio.file.Files;
import java.nio.file.StandardCopyOption;
import java.nio.file.StandardOpenOption;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.Map;
+import java.util.zip.CRC32;
/**
* A durable {@link TinkerStorage} engine that persists a {@code TinkerStorageGraph} as an append-only commit log
@@ -76,10 +78,22 @@ import java.util.Map;
public final class GraphBinaryStorage implements TinkerStorage {
/**
- * Version byte prefixing every record, allowing the on-disk format to evolve.
+ * Magic bytes ("TGSB" — TinkerGraph Storage Binary) at the start of every storage file, so a file can be
+ * identified as one written by this engine (and a foreign or corrupt file rejected) before any record is read.
+ */
+ static final byte[] MAGIC = { 'T', 'G', 'S', 'B' };
+
+ /**
+ * On-disk format version, written once in each file's header after {@link #MAGIC}. A future format bump is
+ * detected here so old files can be rejected (or, later, migrated) rather than misread.
*/
static final byte FORMAT_VERSION = 1;
+ /**
+ * Bytes of the fixed file header: {@link #MAGIC} followed by the one-byte {@link #FORMAT_VERSION}.
+ */
+ static final int HEADER_SIZE = MAGIC.length + 1;
+
private static final byte OP_PUT_VERTEX = 1;
private static final byte OP_DEL_VERTEX = 2;
private static final byte OP_PUT_EDGE = 3;
@@ -164,16 +178,15 @@ public final class GraphBinaryStorage implements TinkerStorage {
* Read every record in a file, folding puts and deletes into the supplied maps.
*/
private void foldRecords(final File file, final Map<Object, DetachedVertex> vertices, final Map<Object, DetachedEdge> edges) {
+ final long fileLength = file.length();
try (final DataInputStream in = new DataInputStream(new java.io.BufferedInputStream(new FileInputStream(file)))) {
+ long remaining = readAndVerifyHeader(in, file, fileLength);
while (true) {
- final byte[] record;
- try {
- record = readFrame(in);
- } catch (EOFException eof) {
- break;
- }
+ final byte[] record = readFrame(in, remaining);
if (record == null)
break;
+ // account for the header (length + crc) and payload just consumed
+ remaining -= 2L * Integer.BYTES + record.length;
applyRecord(record, vertices, edges);
}
} catch (IOException ex) {
@@ -181,11 +194,29 @@ public final class GraphBinaryStorage implements TinkerStorage {
}
}
+ /**
+ * Read and validate the fixed file header, returning the number of record bytes that follow it. An empty file
+ * (freshly created, no header yet) is treated as having no records.
+ */
+ private long readAndVerifyHeader(final DataInputStream in, final File file, final long fileLength) throws IOException {
+ if (fileLength == 0)
+ return 0;
+ if (fileLength < HEADER_SIZE)
+ throw new IOException(String.format("Corrupt storage file %s: shorter than its %d-byte header", file, HEADER_SIZE));
+ final byte[] magic = new byte[MAGIC.length];
+ readFully(in, magic);
+ if (!Arrays.equals(magic, MAGIC))
+ throw new IOException(String.format("%s is not a TinkerGraph storage file (bad magic)", file));
+ final byte version = in.readByte();
+ if (version != FORMAT_VERSION)
+ throw new IOException(String.format(
+ "Unsupported storage format version %d in %s (expected %d)", version, file, FORMAT_VERSION));
+ return fileLength - HEADER_SIZE;
+ }
+
private void applyRecord(final byte[] record, final Map<Object, DetachedVertex> vertices, final Map<Object, DetachedEdge> edges) throws IOException {
+ // the format version is validated once per file in readAndVerifyHeader, so records no longer repeat it
final ByteBufferBuffer buffer = new ByteBufferBuffer(record);
- final byte version = buffer.readByte();
- if (version != FORMAT_VERSION)
- throw new IOException(String.format("Unsupported storage record version %d (expected %d)", version, FORMAT_VERSION));
buffer.readLong(); // txVersion, retained for diagnostics/future use
final int entryCount = buffer.readInt();
for (int i = 0; i < entryCount; i++) {
@@ -225,21 +256,21 @@ public final class GraphBinaryStorage implements TinkerStorage {
try {
final byte[] frame = encodeRecord(txVersion, changedVertices, changedEdges);
writeFrame(logOut, frame);
- logBytesSinceCompaction += Integer.BYTES + frame.length; // length prefix + payload
+ logBytesSinceCompaction += 2L * Integer.BYTES + frame.length; // length + crc prefixes + payload
} catch (IOException ex) {
throw new UncheckedIOException("Could not append transaction to storage log", ex);
}
}
/**
- * Serialize a commit record: version byte, txVersion, entry count, then each entry as an op byte followed by
- * either the serialized element (put) or the serialized id (delete).
+ * Serialize a commit record: txVersion, entry count, then each entry as an op byte followed by either the
+ * serialized element (put) or the serialized id (delete). The format version lives in the file header, not the
+ * record.
*/
private byte[] encodeRecord(final long txVersion,
final Collection<TinkerStorageMutation<TinkerVertex>> changedVertices,
final Collection<TinkerStorageMutation<TinkerEdge>> changedEdges) throws IOException {
final ByteBufferBuffer buffer = new ByteBufferBuffer();
- buffer.writeByte(FORMAT_VERSION);
buffer.writeLong(txVersion);
buffer.writeInt(changedVertices.size() + changedEdges.size());
for (final TinkerStorageMutation<TinkerVertex> m : changedVertices) {
@@ -296,6 +327,7 @@ public final class GraphBinaryStorage implements TinkerStorage {
final File tmp = new File(directory, SNAPSHOT_FILE + ".tmp");
try (final FileOutputStream fos = new FileOutputStream(tmp);
final DataOutputStream out = new DataOutputStream(new BufferedOutputStream(fos))) {
+ writeHeader(out);
writeSnapshot(graph, out);
out.flush();
// force the snapshot's bytes to the device before it is renamed into place
@@ -380,7 +412,6 @@ public final class GraphBinaryStorage implements TinkerStorage {
*/
private void writeElementFrame(final DataOutputStream out, final byte op, final Object element) throws IOException {
final ByteBufferBuffer buffer = new ByteBufferBuffer();
- buffer.writeByte(FORMAT_VERSION);
buffer.writeLong(0L); // snapshot records have no single tx version
buffer.writeInt(1);
buffer.writeByte(op);
@@ -397,15 +428,26 @@ public final class GraphBinaryStorage implements TinkerStorage {
private void ensureLogOpen() {
if (logOut == null) {
try {
+ final boolean freshFile = !logFile.exists() || logFile.length() == 0;
// retain the FileOutputStream so flush() can reach its FileDescriptor for fsync
logFos = new FileOutputStream(logFile, true);
logOut = new DataOutputStream(new BufferedOutputStream(logFos));
+ if (freshFile)
+ writeHeader(logOut);
} catch (IOException ex) {
throw new UncheckedIOException("Could not open storage log for append", ex);
}
}
}
+ /**
+ * Write the fixed file header ({@link #MAGIC} + {@link #FORMAT_VERSION}) at the start of a storage file.
+ */
+ private static void writeHeader(final DataOutputStream out) throws IOException {
+ out.write(MAGIC);
+ out.writeByte(FORMAT_VERSION);
+ }
+
private void closeLog() {
if (logOut != null) {
try {
@@ -421,33 +463,54 @@ public final class GraphBinaryStorage implements TinkerStorage {
}
/**
- * Write a length-prefixed frame: a 4-byte big-endian length followed by the payload.
+ * Write a framed record: a 4-byte big-endian payload length, a 4-byte CRC32 of the payload, then the payload.
+ * The checksum lets a reader tell a bit-flip inside a complete frame (corruption) from a short final frame left
+ * by an interrupted append (truncation).
*/
private static void writeFrame(final DataOutputStream out, final byte[] payload) throws IOException {
+ final CRC32 crc = new CRC32();
+ crc.update(payload);
out.writeInt(payload.length);
+ out.writeInt((int) crc.getValue());
out.write(payload);
}
/**
- * Read a length-prefixed frame, or return {@code null} on a clean end of file. A truncated final frame (from a
- * crash mid-append) is treated as end of file so earlier committed records still load.
+ * Read a framed record, or return {@code null} at end of the readable log. A frame that is only partially present
+ * — the file ends inside the header or payload — is treated as an interrupted trailing append (truncation) and
+ * ends reading so earlier committed records still load. A frame that is fully present but whose stored CRC does
+ * not match its payload is genuine corruption and is raised, rather than silently dropping it and everything
+ * after it.
+ *
+ * @param remaining bytes left in the file at the current position; used to distinguish a short trailing frame
+ * (truncation) from a complete frame, and to bound the payload allocation against a garbage length
*/
- private static byte[] readFrame(final DataInputStream in) throws IOException {
- final int length;
- try {
- length = in.readInt();
- } catch (EOFException eof) {
- return null;
- }
+ private static byte[] readFrame(final DataInputStream in, final long remaining) throws IOException {
+ if (remaining == 0)
+ return null; // clean end of file, exactly on a frame boundary
+ if (remaining < 2L * Integer.BYTES)
+ return null; // not even a full header left — interrupted append
+
+ final int length = in.readInt();
+ final int storedCrc = in.readInt();
if (length < 0)
- throw new IOException("Corrupt storage frame length: " + length);
+ throw new IOException("Corrupt storage frame: negative payload length " + length);
+ if ((long) length > remaining - 2L * Integer.BYTES)
+ return null; // frame claims more bytes than remain — truncated trailing append
+
final byte[] payload = new byte[length];
try {
readFully(in, payload);
} catch (EOFException eof) {
- // partial trailing frame from an interrupted append — stop here
- return null;
+ return null; // partial trailing payload from an interrupted append
}
+
+ final CRC32 crc = new CRC32();
+ crc.update(payload);
+ if ((int) crc.getValue() != storedCrc)
+ throw new IOException(String.format(
+ "Corrupt storage frame: CRC mismatch (stored %08x, computed %08x) in a fully-present %d-byte record",
+ storedCrc, (int) crc.getValue(), length));
return payload;
}
diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorageTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorageTest.java
index 2d6f9952d5..34e67ab93d 100644
--- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorageTest.java
+++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/storage/GraphBinaryStorageTest.java
@@ -33,6 +33,7 @@ import java.util.Iterator;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
/**
* Runs the shared {@link AbstractTinkerStorageConformanceTest} suite against the {@link GraphBinaryStorage} engine and
@@ -135,12 +136,14 @@ public class GraphBinaryStorageTest extends AbstractTinkerStorageConformanceTest
}
/**
- * Count the length-prefixed frames in a storage file: each frame is a 4-byte big-endian length followed by that
- * many payload bytes.
+ * Count the framed records in a storage file: a fixed header ({@code HEADER_SIZE} bytes) followed by frames of a
+ * 4-byte big-endian payload length, a 4-byte CRC, then that many payload bytes.
*/
private static int countFrames(final File file) throws Exception {
int frames = 0;
try (final DataInputStream in = new DataInputStream(new java.io.BufferedInputStream(new java.io.FileInputStream(file)))) {
+ final long headerSkipped = in.skip(GraphBinaryStorage.HEADER_SIZE);
+ if (headerSkipped < GraphBinaryStorage.HEADER_SIZE) return 0;
while (true) {
final int len;
try {
@@ -148,6 +151,7 @@ public class GraphBinaryStorageTest extends AbstractTinkerStorageConformanceTest
} catch (java.io.EOFException eof) {
break;
}
+ in.readInt(); // CRC
final long skipped = in.skip(len);
if (skipped < len) break;
frames++;
@@ -241,6 +245,68 @@ public class GraphBinaryStorageTest extends AbstractTinkerStorageConformanceTest
graph.close();
}
+ @Test
+ public void shouldFailOnCorruptFrameWithBadCrc() throws Exception {
+ TinkerStorageGraph graph = open();
+ final String location = graph.configuration().getString(TinkerGraph.GREMLIN_TINKERGRAPH_GRAPH_LOCATION);
+ graph.addVertex(T.id, 1, "value", 1);
+ graph.tx().commit();
+ graph.addVertex(T.id, 2, "value", 2);
+ graph.tx().commit();
+ graph.tx().close();
+
+ // preserve the raw log, then reconstruct it with a bit flipped inside the first frame's payload — a complete
+ // frame whose CRC no longer matches (distinct from a short trailing frame, which is tolerated as truncation)
+ final File logFile = new File(location, GraphBinaryStorage.LOG_FILE);
+ final byte[] log = Files.readAllBytes(logFile.toPath());
+ graph.close();
+ // header, then first frame's 4-byte length + 4-byte CRC, then payload — flip the first payload byte
+ final int firstPayloadByte = GraphBinaryStorage.HEADER_SIZE + 2 * Integer.BYTES;
+ log[firstPayloadByte] ^= 0x01;
+ Files.deleteIfExists(new File(location, GraphBinaryStorage.SNAPSHOT_FILE).toPath());
+ Files.write(logFile.toPath(), log);
+
+ try {
+ open();
+ fail("expected reopen to fail on a CRC mismatch");
+ } catch (Exception expected) {
+ assertTrue("cause should report corruption: " + rootMessage(expected),
+ rootMessage(expected).contains("CRC mismatch"));
+ }
+ }
+
+ @Test
+ public void shouldFailOnForeignFileWithBadMagic() throws Exception {
+ TinkerStorageGraph graph = open();
+ final String location = graph.configuration().getString(TinkerGraph.GREMLIN_TINKERGRAPH_GRAPH_LOCATION);
+ graph.addVertex(T.id, 1);
+ graph.tx().commit();
+ graph.tx().close();
+ final File logFile = new File(location, GraphBinaryStorage.LOG_FILE);
+ final byte[] log = Files.readAllBytes(logFile.toPath());
+ graph.close();
+
+ // corrupt the magic so the file no longer identifies as a TinkerGraph storage file
+ log[0] ^= 0xFF;
+ Files.deleteIfExists(new File(location, GraphBinaryStorage.SNAPSHOT_FILE).toPath());
+ Files.write(logFile.toPath(), log);
+
+ try {
+ open();
+ fail("expected reopen to fail on bad magic");
+ } catch (Exception expected) {
+ assertTrue("cause should report a bad storage file: " + rootMessage(expected),
+ rootMessage(expected).contains("not a TinkerGraph storage file"));
+ }
+ }
+
+ private static String rootMessage(final Throwable t) {
+ Throwable cur = t;
+ while (cur.getCause() != null && cur.getCause() != cur)
+ cur = cur.getCause();
+ return String.valueOf(cur.getMessage());
+ }
+
private static long countOf(final Iterator<?> it) {
long count = 0;
while (it.hasNext()) {