[ISSUE #9328] Fix ConfigManager.persist() not truncating stale data after write

When ConfigManager.persist() writes config data using RandomAccessFile,
it does not truncate the file after writing. If the new content is shorter
than the previous content, stale trailing bytes remain in the file. After
an unexpected power cut during a write, the file may contain a mix of
partial new data and old trailing data, producing invalid JSON that causes
decode() to fail on restart and leads to NullPointerException.

Fix: call randomAccessFile.setLength(data.length) after writing to ensure
the file is truncated to the exact size of the new content.

Made-with: Cursor

Author: mtlyzhang

common/src/main/java/org/apache/rocketmq/common/ConfigManager.java

@@ -111,7 +111,9 @@ public synchronized void persist() {
                 }
 
                 try (RandomAccessFile randomAccessFile = new RandomAccessFile(config, "rw")) {
-                    randomAccessFile.write(jsonString.getBytes(StandardCharsets.UTF_8));
+                    byte[] data = jsonString.getBytes(StandardCharsets.UTF_8);
+                    randomAccessFile.write(data);
+                    randomAccessFile.setLength(data.length);
                     randomAccessFile.getChannel().force(true);
                     // sync the directory, ensure that the config file is visible
                     MixAll.fsyncDirectory(Paths.get(configFile.getParent()));

common/src/test/java/org/apache/rocketmq/common/ConfigManagerTest.java

@@ -17,7 +17,10 @@
 
 import java.io.File;
 import java.io.PrintWriter;
+import java.io.RandomAccessFile;
 import java.lang.reflect.Method;
+import java.nio.charset.StandardCharsets;
+import org.junit.After;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
@@ -27,6 +30,12 @@ public class ConfigManagerTest {
     private static final String PATH_FILE = System.getProperty("java.io.tmpdir") + File.separator + "org.apache.rocketmq.common.ConfigManagerTest";
     private static final String CONTENT_ENCODE = "Encode content for ConfigManager";
 
+    @After
+    public void cleanup() {
+        new File(PATH_FILE).delete();
+        new File(PATH_FILE + ".bak").delete();
+    }
+
     @Test
     public void testLoad() throws Exception {
         ConfigManager testConfigManager = buildTestConfigManager();
@@ -42,7 +51,6 @@ public void testLoad() throws Exception {
     public void testLoadBak() throws Exception {
         ConfigManager testConfigManager = buildTestConfigManager();
         File file = createAndWriteFile(testConfigManager.configFilePath() + ".bak");
-        // invoke private method "loadBak()"
         Method declaredMethod = ConfigManager.class.getDeclaredMethod("loadBak");
         declaredMethod.setAccessible(true);
         Boolean loadBakResult = (Boolean) declaredMethod.invoke(testConfigManager);
@@ -62,6 +70,50 @@ public void testPersist() throws Exception {
         assertEquals(CONTENT_ENCODE, MixAll.file2String(file));
     }
 
+    @Test
+    public void testPersistTruncatesStaleData() throws Exception {
+        String configPath = PATH_FILE;
+        File dir = new File(new File(configPath).getParent());
+        if (!dir.exists()) {
+            dir.mkdirs();
+        }
+
+        String longContent = "THIS_IS_A_VERY_LONG_OLD_CONTENT_THAT_SHOULD_BE_TRUNCATED_AFTER_PERSIST";
+        try (RandomAccessFile raf = new RandomAccessFile(configPath, "rw")) {
+            raf.write(longContent.getBytes(StandardCharsets.UTF_8));
+            raf.getChannel().force(true);
+        }
+        assertEquals(longContent, MixAll.file2String(new File(configPath)));
+
+        ConfigManager testConfigManager = buildTestConfigManager();
+        testConfigManager.persist();
+
+        String afterPersist = MixAll.file2String(new File(configPath));
+        assertEquals(
+            "persist() should truncate stale trailing data from previous writes",
+            CONTENT_ENCODE, afterPersist);
+    }
+
+    @Test
+    public void testLoadCorruptedFileWithValidBackup() throws Exception {
+        ConfigManager testConfigManager = buildTestConfigManager();
+        createAndWriteFile(testConfigManager.configFilePath(), "{corrupted!!!");
+        createAndWriteFile(testConfigManager.configFilePath() + ".bak", CONTENT_ENCODE);
+
+        assertTrue("Should succeed by falling back to valid .bak file",
+            testConfigManager.load());
+    }
+
+    @Test
+    public void testLoadFirstTimeNoFilesExist() throws Exception {
+        ConfigManager testConfigManager = buildTestConfigManager();
+        new File(testConfigManager.configFilePath()).delete();
+        new File(testConfigManager.configFilePath() + ".bak").delete();
+
+        assertTrue("First-time startup with no files should succeed",
+            testConfigManager.load());
+    }
+
     private ConfigManager buildTestConfigManager() {
         return new ConfigManager() {
             @Override
@@ -87,14 +139,19 @@ public String encode(boolean prettyFormat) {
     }
 
     private File createAndWriteFile(String fileName) throws Exception {
+        return createAndWriteFile(fileName, "TestForConfigManager");
+    }
+
+    private File createAndWriteFile(String fileName, String content) throws Exception {
         File file = new File(fileName);
         if (file.exists()) {
             file.delete();
         }
+        file.getParentFile().mkdirs();
         file.createNewFile();
         PrintWriter out = new PrintWriter(fileName);
-        out.write("TestForConfigManager");
+        out.write(content);
         out.close();
         return file;
     }
-}
+}