Validates each property found is an accumulo property

rsingh433 · Jan 5, 2024 · 0b6a126 · 0b6a126
1 parent 7ab4ebc
commit 0b6a126
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 4 deletions.
diff --git a/shell/src/main/java/org/apache/accumulo/shell/commands/ConfigCommand.java b/shell/src/main/java/org/apache/accumulo/shell/commands/ConfigCommand.java
@@ -380,16 +380,30 @@ public int execute(final String fullCommand, final CommandLine cl, final Shell s
   }
 
   private void modifyPropertiesFromFile(CommandLine cl, Shell shellState, String filename)
-      throws AccumuloException, AccumuloSecurityException, TableNotFoundException, IOException,
-      NamespaceNotFoundException {
+      throws AccumuloException, AccumuloSecurityException, IOException, NamespaceNotFoundException {
     PropertiesConfiguration fileProperties = readPropertiesFromFile(filename);
 
     Map<String,String> propertiesMap = new HashMap<>();
     Iterator<String> keysIterator = fileProperties.getKeys();
+    boolean foundErrors = false;
     while (keysIterator.hasNext()) {
       String key = keysIterator.next();
       String value = fileProperties.getString(key);
-      propertiesMap.put(key, value);
+      if (!Property.isValidPropertyKey(key)) {
+        Shell.log.error("Property: \"{}\" is invalid", key);
+        foundErrors = true;
+      } else if (!Property.isValidProperty(key, value)) {
+        Shell.log.error("Property: \"{}\" has an invalid value: \"{}\"", key, value);
+        foundErrors = true;
+      } else {
+        propertiesMap.put(key, value);
+      }
+    }
+    // Error for the whole file as this should be an atomic properties update
+    if (foundErrors) {
+      Shell.log.error("Property file {} contains invalid properties", filename);
+      throw new AccumuloException("InvalidPropertyFile: " + filename);
+
     }
 
     Consumer<Map<String,String>> propertyModifier = currProps -> {

diff --git a/test/src/main/java/org/apache/accumulo/test/shell/ShellIT.java b/test/src/main/java/org/apache/accumulo/test/shell/ShellIT.java
@@ -570,7 +570,7 @@ public void propFileNotFoundTest() throws IOException {
 
   @Test
   public void setpropsViaFile() throws Exception {
-    // execfile
+
     File file = File.createTempFile("propFile", ".conf",
         new File("src/main/resources/org/apache/accumulo/test/"));
     PrintWriter writer = new PrintWriter(file.getAbsolutePath());
@@ -580,6 +580,18 @@ public void setpropsViaFile() throws Exception {
     file.deleteOnExit();
   }
 
+  @Test
+  public void invalidPropFileTest() throws Exception {
+    File file = File.createTempFile("invalidPropFile", ".conf",
+        new File("src/main/resources/org/apache/accumulo/test/"));
+    PrintWriter writer = new PrintWriter(file.getAbsolutePath());
+    writer.println("this is not a valid property file");
+    writer.close();
+    exec("config --propFile " + file.getAbsolutePath(), false,
+        "InvalidPropertyFile: " + file.getAbsolutePath());
+    file.deleteOnExit();
+  }
+
   @Test
   public void setIterTest() throws IOException {
     Shell.log.debug("Starting setiter test --------------------------");