Merge pull request #3932 from vreuland/issue-3931-fix-mvstoretool-dum…

…p-npe

Fix NPE and avoid binary output in MVStoreTool -dump

h2/src/main/org/h2/mvstore/Chunk.java

@@ -219,6 +219,7 @@ public abstract class Chunk<C extends Chunk<C>> {
      *
      * @param buff the source buffer
      * @return the chunk
+     * @throws MVStoreException if {@code buff} does not contain a chunk header
      */
     static String readChunkHeader(ByteBuffer buff) {
         int pos = buff.position();
@@ -232,7 +233,7 @@ static String readChunkHeader(ByteBuffer buff) {
                 return s;
             }
         }
-        return null;
+        throw DataUtils.newMVStoreException(DataUtils.ERROR_FILE_CORRUPT, "Not a valid chunk header");
     }
 
     /**

h2/src/main/org/h2/mvstore/MVStoreTool.java

@@ -33,6 +33,8 @@
  */
 public class MVStoreTool {
 
+    public static final int MAX_NB_FILE_HEADERS_PER_FILE = 2;
+
     /**
      * Runs this tool.
      * Options are case sensitive. Supported options are:
@@ -119,6 +121,7 @@ public static void dump(String fileName, Writer writer, boolean details) {
             int len = Long.toHexString(fileSize).length();
             ByteBuffer buffer = ByteBuffer.allocate(4096);
             long pageCount = 0;
+            int readFileHeaderCount = 0;
             for (long pos = 0; pos < fileSize; ) {
                 buffer.rewind();
                 // Bugfix - An MVStoreException that wraps EOFException is
@@ -134,11 +137,12 @@ public static void dump(String fileName, Writer writer, boolean details) {
                 }
                 buffer.rewind();
                 int headerType = buffer.get();
-                if (headerType == 'H') {
+                if (headerType == 'H' && readFileHeaderCount < MAX_NB_FILE_HEADERS_PER_FILE) {
                     String header = new String(buffer.array(), StandardCharsets.ISO_8859_1).trim();
                     pw.printf("%0" + len + "x fileHeader %s%n",
                             pos, header);
                     pos += blockSize;
+                    readFileHeaderCount++;
                     continue;
                 }
                 if (headerType != 'c') {
@@ -150,6 +154,9 @@ public static void dump(String fileName, Writer writer, boolean details) {
                 try {
                     c = new SFChunk(Chunk.readChunkHeader(buffer));
                 } catch (MVStoreException e) {
+                    // Chunks are not always contiguous (due to chunk compaction/move/drop and space re-use)
+                    // Blocks following a chunk can therefore contain something else than a valid chunk header
+                    // In that case, let's move to the next block
                     pos += blockSize;
                     continue;
                 }

h2/src/test/org/h2/test/store/TestMVStoreTool.java

@@ -5,6 +5,7 @@
  */
 package org.h2.test.store;
 
+import java.io.StringWriter;
 import java.util.Map.Entry;
 import java.util.Random;
 
@@ -22,6 +23,10 @@
  */
 public class TestMVStoreTool extends TestBase {
 
+    public static final String BIG_STRING_WITH_C = new String(new char[3000]).replace("\0", "c");
+    public static final String BIG_STRING_WITH_H = new String(new char[3000]).replace("\0", "H");
+
+
     /**
      * Run just this test.
      *
@@ -37,6 +42,7 @@ public static void main(String... a) throws Exception {
     @Override
     public void test() throws Exception {
         testCompact();
+        testDump();
     }
 
     private void testCompact() {
@@ -148,4 +154,39 @@ private void assertEquals(MVStore a, MVStore b) {
         }
     }
 
+    private void testDump() {
+        String fileName = getBaseDir() + "/testDump.h3";
+        FileUtils.createDirectories(getBaseDir());
+        FileUtils.delete(fileName);
+        // store with a very small page size, to make sure
+        // there are many leaf pages
+        MVStore s = new MVStore.Builder().
+                pageSplitSize(1000).
+                fileName(fileName).autoCommitDisabled().open();
+        s.setRetentionTime(0);
+        MVMap<Integer, String> map = s.openMap("data");
+
+        // Insert some data. Using big strings with "H" and "c" to validate the fix of #3931
+        int nbEntries = 20_000;
+        for (int i = 0; i < nbEntries; i++) {
+            map.put(i, i % 2 == 0 ? BIG_STRING_WITH_C : BIG_STRING_WITH_H);
+        }
+        s.commit();
+        // Let's rewrite the data to trigger some chunk compaction & drop
+        for (int i = 0; i < nbEntries; i++) {
+            map.put(i, i % 2 == 0 ? BIG_STRING_WITH_H : BIG_STRING_WITH_C);
+        }
+        s.commit();
+        s.close();
+        StringWriter dumpWriter = new StringWriter();
+        MVStoreTool.dump(fileName, dumpWriter, true);
+
+        int nbFileHeaders = nbOfOccurrences(dumpWriter.toString(), "fileHeader");
+        assertEquals("Exactly 2 file headers are expected in the dump", 2, nbFileHeaders);
+    }
+
+    private int nbOfOccurrences(String str, String pattern) {
+       return str.split(pattern,-1).length - 1;
+    }
+
 }