Improvement: Add jdbc capabilities for creating and dropping schema

apache · Jun 18, 2024 · b9c7095 · b9c7095
1 parent c0f8be0
commit b9c7095
Show file tree

Hide file tree

Showing 12 changed files with 340 additions and 5 deletions.
diff --git a/.../catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/JdbcCatalog.java b/.../catalog-jdbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/JdbcCatalog.java
@@ -13,6 +13,7 @@
 import com.datastrato.gravitino.connector.CatalogOperations;
 import com.datastrato.gravitino.connector.PropertiesMetadata;
 import com.datastrato.gravitino.connector.PropertyEntry;
+import com.datastrato.gravitino.connector.capability.Capability;
 import java.util.Collections;
 import java.util.Map;
 
@@ -52,6 +53,11 @@ protected CatalogOperations newOps(Map<String, String> config) {
     return ops;
   }
 
+  @Override
+  public Capability newCapability() {
+    return new JdbcCatalogCapability();
+  }
+
   /** @return The {@link JdbcExceptionConverter} to be used by the catalog. */
   protected JdbcExceptionConverter createExceptionConverter() {
     return new JdbcExceptionConverter() {};

diff --git a/...dbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/JdbcCatalogCapability.java b/...dbc-common/src/main/java/com/datastrato/gravitino/catalog/jdbc/JdbcCatalogCapability.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2024 Datastrato Pvt Ltd.
+ * This software is licensed under the Apache License version 2.
+ */
+package com.datastrato.gravitino.catalog.jdbc;
+
+import com.datastrato.gravitino.connector.capability.Capability;
+import com.datastrato.gravitino.connector.capability.CapabilityResult;
+
+public class JdbcCatalogCapability implements Capability {
+  /**
+   * Regular expression explanation: Regex that matches any string that maybe a filename with an
+   * optional extension We adopt a blacklist approach that excludes filename or extension that
+   * contains '.', '/', or '\' ^[^.\/\\]+(\.[^.\/\\]+)?$
+   *
+   * <p>^ - Start of the string
+   *
+   * <p>[^.\/\\]+ - matches any filename string that does not contain '.', '/', or '\'
+   *
+   * <p>(\.[^.\/\\]+)? - matches an optional extension
+   *
+   * <p>$ - End of the string
+   */
+  // We use sqlite name pattern to be the default pattern for JDBC catalog for testing purposes
+  public static final String SQLITE_NAME_PATTERN = "^[^.\\/\\\\]+(\\.[^.\\/\\\\]+)?$";
+
+  @Override
+  public CapabilityResult specificationOnName(Scope scope, String name) {
+    // TODO: Validate the name against reserved words
+    if (!name.matches(SQLITE_NAME_PATTERN)) {
+      return CapabilityResult.unsupported(
+          String.format("The %s name '%s' is illegal.", scope, name));
+    }
+    return CapabilityResult.SUPPORTED;
+  }
+}
diff --git a/...catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/DorisCatalog.java b/...catalog-jdbc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/DorisCatalog.java
@@ -17,6 +17,7 @@
 import com.datastrato.gravitino.catalog.jdbc.operation.JdbcDatabaseOperations;
 import com.datastrato.gravitino.catalog.jdbc.operation.JdbcTableOperations;
 import com.datastrato.gravitino.connector.CatalogOperations;
+import com.datastrato.gravitino.connector.capability.Capability;
 import java.util.Map;
 
 /** Implementation of a Doris catalog in Gravitino. */
@@ -38,6 +39,11 @@ protected CatalogOperations newOps(Map<String, String> config) {
         createJdbcColumnDefaultValueConverter());
   }
 
+  @Override
+  public Capability newCapability() {
+    return new DorisCatalogCapability();
+  }
+
   @Override
   protected JdbcExceptionConverter createExceptionConverter() {
     return new DorisExceptionConverter();

diff --git a/...bc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/DorisCatalogCapability.java b/...bc-doris/src/main/java/com/datastrato/gravitino/catalog/doris/DorisCatalogCapability.java
@@ -0,0 +1,13 @@
+/*
+ * Copyright 2024 Datastrato Pvt Ltd.
+ * This software is licensed under the Apache License version 2.
+ */
+package com.datastrato.gravitino.catalog.doris;
+
+import com.datastrato.gravitino.connector.capability.Capability;
+
+public class DorisCatalogCapability implements Capability {
+  // Doris best practice mention that the name should be in lowercase, separated by underscores
+  // https://doris.apache.org/docs/2.0/table-design/best-practice/
+  // We can use the more general DEFAULT_NAME_PATTERN for Doris and update as needed in the future
+}
diff --git a/...src/test/java/com/datastrato/gravitino/catalog/doris/integration/test/CatalogDorisIT.java b/...src/test/java/com/datastrato/gravitino/catalog/doris/integration/test/CatalogDorisIT.java
@@ -30,14 +30,17 @@
 import com.datastrato.gravitino.rel.indexes.Index;
 import com.datastrato.gravitino.rel.indexes.Indexes;
 import com.datastrato.gravitino.rel.types.Types;
+import com.datastrato.gravitino.utils.RandomNameUtils;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
+import org.apache.commons.lang3.StringUtils;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
@@ -256,6 +259,65 @@ void testDropDorisSchema() {
         });
   }
 
+  @Test
+  void testSchemaWithIllegalName() {
+    SupportsSchemas schemas = catalog.asSchemas();
+    String databaseName = RandomNameUtils.genRandomName("it_db");
+    Map<String, String> properties = new HashMap<>();
+    String comment = "comment";
+
+    // should throw an exception with string that might contain SQL injection
+    String sqlInjection = databaseName + "`; DROP TABLE important_table; -- ";
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          schemas.createSchema(sqlInjection, comment, properties);
+        });
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          schemas.dropSchema(sqlInjection, false);
+        });
+
+    String sqlInjection1 = databaseName + "`; SLEEP(10); -- ";
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          schemas.createSchema(sqlInjection1, comment, properties);
+        });
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          schemas.dropSchema(sqlInjection1, false);
+        });
+
+    String sqlInjection2 =
+        databaseName + "`; UPDATE Users SET password = 'newpassword' WHERE username = 'admin'; -- ";
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          schemas.createSchema(sqlInjection2, comment, properties);
+        });
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          schemas.dropSchema(sqlInjection2, false);
+        });
+
+    // should throw an exception with input that has more than 64 characters
+    String invalidInput = StringUtils.repeat("a", 65);
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          schemas.createSchema(invalidInput, comment, properties);
+        });
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          schemas.dropSchema(invalidInput, false);
+        });
+  }
+
   @Test
   void testDorisTableBasicOperation() {
     // create a table

diff --git a/...catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/MysqlCatalog.java b/...catalog-jdbc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/MysqlCatalog.java
@@ -18,6 +18,7 @@
 import com.datastrato.gravitino.catalog.mysql.operation.MysqlTableOperations;
 import com.datastrato.gravitino.connector.CatalogOperations;
 import com.datastrato.gravitino.connector.PropertiesMetadata;
+import com.datastrato.gravitino.connector.capability.Capability;
 import java.util.Map;
 
 /** Implementation of a Mysql catalog in Gravitino. */
@@ -42,6 +43,11 @@ protected CatalogOperations newOps(Map<String, String> config) {
         createJdbcColumnDefaultValueConverter());
   }
 
+  @Override
+  public Capability newCapability() {
+    return new MysqlCatalogCapability();
+  }
+
   @Override
   protected JdbcExceptionConverter createExceptionConverter() {
     return new MysqlExceptionConverter();

diff --git a/...bc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/MysqlCatalogCapability.java b/...bc-mysql/src/main/java/com/datastrato/gravitino/catalog/mysql/MysqlCatalogCapability.java
@@ -0,0 +1,37 @@
+/*
+ * Copyright 2024 Datastrato Pvt Ltd.
+ * This software is licensed under the Apache License version 2.
+ */
+package com.datastrato.gravitino.catalog.mysql;
+
+import com.datastrato.gravitino.connector.capability.Capability;
+import com.datastrato.gravitino.connector.capability.CapabilityResult;
+
+public class MysqlCatalogCapability implements Capability {
+  /**
+   * Regular expression explanation: ^[\w\p{L}-$/=]{1,64}$
+   *
+   * <p>^ - Start of the string
+   *
+   * <p>[\w\p{L}-$/=]{1,64} - Consist of 1 to 64 characters of letters (both cases), digits,
+   * underscores, any kind of letter from any language, hyphens, dollar signs, slashes or equal
+   * signs
+   *
+   * <p>\w - matches [a-zA-Z0-9_]
+   *
+   * <p>\p{L} - matches any kind of letter from any language
+   *
+   * <p>$ - End of the string
+   */
+  public static final String MYSQL_NAME_PATTERN = "^[\\w\\p{L}-$/=]{1,64}$";
+
+  @Override
+  public CapabilityResult specificationOnName(Scope scope, String name) {
+    // TODO: Validate the name against reserved words
+    if (!name.matches(MYSQL_NAME_PATTERN)) {
+      return CapabilityResult.unsupported(
+          String.format("The %s name '%s' is illegal.", scope, name));
+    }
+    return CapabilityResult.SUPPORTED;
+  }
+}
diff --git a/...src/test/java/com/datastrato/gravitino/catalog/mysql/integration/test/CatalogMysqlIT.java b/...src/test/java/com/datastrato/gravitino/catalog/mysql/integration/test/CatalogMysqlIT.java
@@ -43,13 +43,15 @@
 import com.datastrato.gravitino.rel.indexes.Indexes;
 import com.datastrato.gravitino.rel.types.Decimal;
 import com.datastrato.gravitino.rel.types.Types;
+import com.datastrato.gravitino.utils.RandomNameUtils;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import java.io.IOException;
 import java.sql.SQLException;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
@@ -1350,8 +1352,68 @@ void testNameSpec() {
 
     Assertions.assertTrue(catalog.asTableCatalog().dropTable(tableIdent));
     Assertions.assertFalse(catalog.asTableCatalog().tableExists(tableIdent));
-    Assertions.assertFalse(catalog.asTableCatalog().purgeTable(tableIdent));
+    Assertions.assertThrows(
+        UnsupportedOperationException.class,
+        () -> {
+          catalog.asTableCatalog().purgeTable(tableIdent);
+        });
     catalog.asSchemas().dropSchema(testSchemaName, true);
+
+    // sql injection
+    String schemaName = RandomNameUtils.genRandomName("ct_db");
+    Map<String, String> properties = new HashMap<>();
+    String comment = null;
+
+    // should throw an exception with string that might contain SQL injection
+    String sqlInjection = schemaName + "`; DROP TABLE important_table; -- ";
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          catalog.asSchemas().createSchema(sqlInjection, comment, properties);
+        });
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          catalog.asSchemas().dropSchema(sqlInjection, false);
+        });
+
+    String sqlInjection1 = schemaName + "`; SLEEP(10); -- ";
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          catalog.asSchemas().createSchema(sqlInjection1, comment, properties);
+        });
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          catalog.asSchemas().dropSchema(sqlInjection1, false);
+        });
+
+    String sqlInjection2 =
+        schemaName + "`; UPDATE Users SET password = 'newpassword' WHERE username = 'admin'; -- ";
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          catalog.asSchemas().createSchema(sqlInjection2, comment, properties);
+        });
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          catalog.asSchemas().dropSchema(sqlInjection2, false);
+        });
+
+    // should throw an exception with input that has more than 64 characters
+    String invalidInput = StringUtils.repeat("a", 65);
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          catalog.asSchemas().createSchema(invalidInput, comment, properties);
+        });
+    Assertions.assertThrows(
+        IllegalArgumentException.class,
+        () -> {
+          catalog.asSchemas().dropSchema(invalidInput, false);
+        });
   }
 
   @Test

diff --git a/...stgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/PostgreSqlCatalog.java b/...stgresql/src/main/java/com/datastrato/gravitino/catalog/postgresql/PostgreSqlCatalog.java
@@ -16,6 +16,7 @@
 import com.datastrato.gravitino.catalog.postgresql.operation.PostgreSqlSchemaOperations;
 import com.datastrato.gravitino.catalog.postgresql.operation.PostgreSqlTableOperations;
 import com.datastrato.gravitino.connector.CatalogOperations;
+import com.datastrato.gravitino.connector.capability.Capability;
 import java.util.Map;
 
 public class PostgreSqlCatalog extends JdbcCatalog {
@@ -36,6 +37,11 @@ protected CatalogOperations newOps(Map<String, String> config) {
         createJdbcColumnDefaultValueConverter());
   }
 
+  @Override
+  public Capability newCapability() {
+    return new PostgreSqlCatalogCapability();
+  }
+
   @Override
   protected JdbcExceptionConverter createExceptionConverter() {
     return new PostgreSqlExceptionConverter();

diff --git a/...rc/main/java/com/datastrato/gravitino/catalog/postgresql/PostgreSqlCatalogCapability.java b/...rc/main/java/com/datastrato/gravitino/catalog/postgresql/PostgreSqlCatalogCapability.java
@@ -0,0 +1,33 @@
+/*
+ * Copyright 2024 Datastrato Pvt Ltd.
+ * This software is licensed under the Apache License version 2.
+ */
+package com.datastrato.gravitino.catalog.postgresql;
+
+import com.datastrato.gravitino.connector.capability.Capability;
+import com.datastrato.gravitino.connector.capability.CapabilityResult;
+
+public class PostgreSqlCatalogCapability implements Capability {
+  /**
+   * Regular expression explanation: ^[_a-zA-Z\p{L}/][\w\p{L}-$/=]{0,62}$
+   *
+   * <p>^[_a-zA-Z\p{L}/] - Start with an underscore, a letter, or a letter from any language
+   *
+   * <p>[\w\p{L}-$/=]{0,62} - Consist of 0 to 62 characters (making the total length at most 63) of
+   * letters (both cases), digits, underscores, any kind of letter from any language, hyphens,
+   * dollar signs, slashes or equal signs
+   *
+   * <p>$ - End of the string
+   */
+  public static final String POSTGRESQL_NAME_PATTERN = "^[_a-zA-Z\\p{L}/][\\w\\p{L}-$/=]{0,62}$";
+
+  @Override
+  public CapabilityResult specificationOnName(Scope scope, String name) {
+    // TODO: Validate the name against reserved words
+    if (!name.matches(POSTGRESQL_NAME_PATTERN)) {
+      return CapabilityResult.unsupported(
+              String.format("The %s name '%s' is illegal.", scope, name));
+    }
+    return CapabilityResult.SUPPORTED;
+  }
+}