[fix](metadata)Add FE metadata-related file checks

CalvinKirs · Sep 10, 2024 · 1e0544e · 1e0544e
1 parent 90e6486
commit 1e0544e
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 11 deletions.
diff --git a/fe/fe-common/src/main/java/org/apache/doris/common/Config.java b/fe/fe-common/src/main/java/org/apache/doris/common/Config.java
@@ -3052,4 +3052,8 @@ public static int metaServiceRpcRetryTimes() {
     @ConfField(mutable = true, description = {"表示最大锁持有时间，超过该时间会打印告警日志，单位秒",
             "Maximum lock hold time; logs a warning if exceeded"})
     public static long max_lock_hold_threshold_seconds = 10;
+
+    @ConfField(mutable = true, description = {"元数据同步是否开启安全模式",
+        "Is metadata synchronization enabled in safe mode"})
+    public static boolean meta_helper_security_mode = false;
 }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/master/MetaHelper.java b/fe/fe-core/src/main/java/org/apache/doris/master/MetaHelper.java
@@ -18,6 +18,7 @@
 package org.apache.doris.master;
 
 import org.apache.doris.catalog.Env;
+import org.apache.doris.common.Config;
 import org.apache.doris.common.io.IOUtils;
 import org.apache.doris.common.util.HttpURLUtil;
 import org.apache.doris.httpv2.entity.ResponseBody;
@@ -46,7 +47,7 @@ public class MetaHelper {
     public static final String X_IMAGE_MD5 = "X-Image-Md5";
     private static final int BUFFER_BYTES = 8 * 1024;
     private static final int CHECKPOINT_LIMIT_BYTES = 30 * 1024 * 1024;
-    private static final String VALID_FILENAME_REGEX =  "^image\\.\\d+(\\.part)?$";
+    private static final String VALID_FILENAME_REGEX = "^image\\.\\d+(\\.part)?$";
 
 
     public static File getMasterImageDir() {
@@ -58,14 +59,10 @@ public static int getLimit() {
         return CHECKPOINT_LIMIT_BYTES;
     }
 
-    // rename the .PART_SUFFIX file to filename
-    public static File complete(String filename, File dir) throws IOException {
-        // Validate that the filename does not contain illegal path elements
-        checkIsValidFileName(filename);
-
-        File file = new File(dir, filename + MetaHelper.PART_SUFFIX); // Original file with a specific suffix
-        File newFile = new File(dir, filename); // Target file without the suffix
-
+    private static void completeCheck(File dir, File file, File newFile) throws IOException {
+        if (!Config.meta_helper_security_mode) {
+            return;
+        }
         String dirPath = dir.getCanonicalPath(); // Get the canonical path of the directory
         String filePath = file.getCanonicalPath(); // Get the canonical path of the original file
         String newFilePath = newFile.getCanonicalPath(); // Get the canonical path of the new file
@@ -80,6 +77,17 @@ public static File complete(String filename, File dir) throws IOException {
             throw new IOException("Source file does not exist or is not a valid file.");
         }
 
+    }
+
+    // rename the .PART_SUFFIX file to filename
+    public static File complete(String filename, File dir) throws IOException {
+        // Validate that the filename does not contain illegal path elements
+        checkIsValidFileName(filename);
+
+        File file = new File(dir, filename + MetaHelper.PART_SUFFIX); // Original file with a specific suffix
+        File newFile = new File(dir, filename); // Target file without the suffix
+
+        completeCheck(dir, file, newFile);
         // Attempt to rename the file. If it fails, throw an exception
         if (!file.renameTo(newFile)) {
             throw new IOException("Complete file " + filename + " failed");
@@ -91,22 +99,36 @@ public static File complete(String filename, File dir) throws IOException {
     public static File getFile(String filename, File dir) throws IOException {
         checkIsValidFileName(filename);
         File file = new File(dir, filename + MetaHelper.PART_SUFFIX);
+        checkFile(dir, file);
+        return file;
+    }
+
+    private static void checkFile(File dir, File file) throws IOException {
+        if (!Config.meta_helper_security_mode) {
+            return;
+        }
         String dirPath = dir.getCanonicalPath();
         String filePath = file.getCanonicalPath();
 
         if (!filePath.startsWith(dirPath)) {
             throw new SecurityException("File path traversal attempt detected.");
         }
-        return file;
     }
 
+
     private static void checkIsValidFileName(String filename) {
+        if (!Config.meta_helper_security_mode) {
+            return;
+        }
         if (!filename.matches(VALID_FILENAME_REGEX)) {
             throw new IllegalArgumentException("Invalid filename");
         }
     }
 
     private static void checkFile(File file) throws IOException {
+        if (!Config.meta_helper_security_mode) {
+            return;
+        }
         if (!file.getAbsolutePath().startsWith(file.getCanonicalFile().getParent())) {
             throw new IllegalArgumentException("Invalid file path");
         }
@@ -175,7 +197,7 @@ public static void getRemoteFile(String urlStr, int timeout, File file)
             if (out != null) {
                 out.close();
             }
-            if (!md5Matched && file.exists()) {
+            if (!md5Matched && file.exists() & Config.meta_helper_security_mode) {
                 file.delete();
             }
         }

diff --git a/fe/fe-core/src/test/java/org/apache/doris/master/MetaHelperTest.java b/fe/fe-core/src/test/java/org/apache/doris/master/MetaHelperTest.java
@@ -17,6 +17,7 @@
 
 package org.apache.doris.master;
 
+import org.apache.doris.common.Config;
 import org.apache.doris.httpv2.entity.ResponseBody;
 import org.apache.doris.httpv2.rest.RestApiStatusCode;
 import org.apache.doris.persist.StorageInfo;
@@ -25,6 +26,11 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import org.junit.Assert;
 import org.junit.Test;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+
+import java.io.File;
+import java.io.IOException;
 
 public class MetaHelperTest {
 
@@ -49,4 +55,47 @@ private ResponseBody<StorageInfo> buildResponseBody() {
         bodyBefore.setMsg("msg");
         return bodyBefore;
     }
+
+    File tempDir = new File(System.getProperty("java.io.tmpdir"), "tempDir");
+
+    @BeforeEach
+    void setUp() {
+
+        if (tempDir.exists()) {
+            tempDir.delete();
+        }
+        tempDir.mkdir();
+    }
+
+    @Test
+    public void testFile() throws IOException {
+
+        String errorFilename = "testfile.";
+        File errorFileWithSuffix = new File(tempDir, errorFilename);
+        String rightFilename = "image.1";
+        File rightFileWithSuffix = new File(tempDir, rightFilename);
+
+        Config.meta_helper_security_mode = true;
+
+        if (errorFileWithSuffix.exists()) {
+            errorFileWithSuffix.delete();
+        }
+        Assert.assertTrue(errorFileWithSuffix.createNewFile());
+        Assert.assertThrows(IllegalArgumentException.class, () -> MetaHelper.complete(errorFilename, tempDir));
+        Assert.assertThrows(IllegalArgumentException.class, () -> MetaHelper.getFile(errorFilename, tempDir));
+        if (rightFileWithSuffix.exists()) {
+            rightFileWithSuffix.delete();
+        }
+        Assert.assertTrue(rightFileWithSuffix.createNewFile());
+        Assert.assertEquals(rightFileWithSuffix.getName() + ".part", MetaHelper.getFile(rightFilename, tempDir).getName());
+
+    }
+
+    @AfterEach
+    public void tearDown() {
+        if (tempDir.exists()) {
+            tempDir.delete();
+        }
+    }
+
 }