This is an automated email from the ASF dual-hosted git repository.

nehapawar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new c9c185a  Validate schema name before permissions to return proper 
message (#8357)
c9c185a is described below

commit c9c185a6c36de65637250ed4e82205f93da87049
Author: Neha Pawar <[email protected]>
AuthorDate: Tue Mar 15 23:52:39 2022 -0700

    Validate schema name before permissions to return proper message (#8357)
    
    * Validate schema name before permissions to return proper message
    
    * Review comments
    
    * Linter
---
 .../api/resources/PinotSchemaRestletResource.java  | 42 ++++++++++------------
 .../api/PinotSchemaRestletResourceTest.java        |  5 +++
 2 files changed, 24 insertions(+), 23 deletions(-)

diff --git 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
index d795aad..0c9461d 100644
--- 
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
+++ 
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
@@ -43,6 +43,7 @@ import javax.ws.rs.core.Context;
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
+import org.apache.commons.lang3.StringUtils;
 import org.apache.pinot.common.exception.SchemaAlreadyExistsException;
 import org.apache.pinot.common.exception.SchemaBackwardIncompatibleException;
 import org.apache.pinot.common.exception.SchemaNotFoundException;
@@ -191,6 +192,7 @@ public class PinotSchemaRestletResource {
       @Context Request request) {
     Schema schema = getSchemaFromMultiPart(multiPart);
     String endpointUrl = request.getRequestURL().toString();
+    validateSchemaName(schema.getSchemaName());
     _accessControlUtils.validatePermission(schema.getSchemaName(), 
AccessType.CREATE, httpHeaders, endpointUrl,
         _accessControlFactory.create());
     return addSchema(schema, override);
@@ -212,6 +214,7 @@ public class PinotSchemaRestletResource {
       @QueryParam("override") boolean override, Schema schema, @Context 
HttpHeaders httpHeaders,
       @Context Request request) {
     String endpointUrl = request.getRequestURL().toString();
+    validateSchemaName(schema.getSchemaName());
     _accessControlUtils.validatePermission(schema.getSchemaName(), 
AccessType.CREATE, httpHeaders, endpointUrl,
         _accessControlFactory.create());
     return addSchema(schema, override);
@@ -229,13 +232,7 @@ public class PinotSchemaRestletResource {
   })
   public String validateSchema(FormDataMultiPart multiPart) {
     Schema schema = getSchemaFromMultiPart(multiPart);
-    try {
-      List<TableConfig> tableConfigs = 
_pinotHelixResourceManager.getTableConfigsForSchema(schema.getSchemaName());
-      SchemaUtils.validate(schema, tableConfigs);
-    } catch (Exception e) {
-      throw new ControllerApplicationException(LOGGER,
-          "Invalid schema: " + schema.getSchemaName() + ". Reason: " + 
e.getMessage(), Response.Status.BAD_REQUEST, e);
-    }
+    validateSchemaInternal(schema);
     return schema.toPrettyJsonString();
   }
 
@@ -251,6 +248,19 @@ public class PinotSchemaRestletResource {
       @ApiResponse(code = 500, message = "Internal error")
   })
   public String validateSchema(Schema schema) {
+    validateSchemaInternal(schema);
+    return schema.toPrettyJsonString();
+  }
+
+  private void validateSchemaName(String schemaName) {
+    if (StringUtils.isBlank(schemaName)) {
+      throw new ControllerApplicationException(LOGGER, "Invalid schema. 
Reason: 'schemaName' should not be null",
+          Response.Status.BAD_REQUEST);
+    }
+  }
+
+  private void validateSchemaInternal(Schema schema) {
+    validateSchemaName(schema.getSchemaName());
     try {
       List<TableConfig> tableConfigs = 
_pinotHelixResourceManager.getTableConfigsForSchema(schema.getSchemaName());
       SchemaUtils.validate(schema, tableConfigs);
@@ -258,7 +268,6 @@ public class PinotSchemaRestletResource {
       throw new ControllerApplicationException(LOGGER,
           "Invalid schema: " + schema.getSchemaName() + ". Reason: " + 
e.getMessage(), Response.Status.BAD_REQUEST, e);
     }
-    return schema.toPrettyJsonString();
   }
 
   /**
@@ -268,13 +277,7 @@ public class PinotSchemaRestletResource {
    */
   private SuccessResponse addSchema(Schema schema, boolean override) {
     String schemaName = schema.getSchemaName();
-    try {
-      List<TableConfig> tableConfigs = 
_pinotHelixResourceManager.getTableConfigsForSchema(schemaName);
-      SchemaUtils.validate(schema, tableConfigs);
-    } catch (Exception e) {
-      throw new ControllerApplicationException(LOGGER,
-          "Cannot add invalid schema: " + schemaName + ". Reason: " + 
e.getMessage(), Response.Status.BAD_REQUEST, e);
-    }
+    validateSchemaInternal(schema);
 
     try {
       _pinotHelixResourceManager.addSchema(schema, override);
@@ -304,14 +307,7 @@ public class PinotSchemaRestletResource {
    * @return
    */
   private SuccessResponse updateSchema(String schemaName, Schema schema, 
boolean reload) {
-    try {
-      List<TableConfig> tableConfigs = 
_pinotHelixResourceManager.getTableConfigsForSchema(schemaName);
-      SchemaUtils.validate(schema, tableConfigs);
-    } catch (Exception e) {
-      throw new ControllerApplicationException(LOGGER,
-          String.format("Cannot add invalid schema: %s. Reason: %s", 
schemaName, e.getMessage()),
-          Response.Status.BAD_REQUEST, e);
-    }
+    validateSchemaInternal(schema);
 
     if (schemaName != null && !schema.getSchemaName().equals(schemaName)) {
       
_controllerMetrics.addMeteredGlobalValue(ControllerMeter.CONTROLLER_SCHEMA_UPLOAD_ERROR,
 1L);
diff --git 
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotSchemaRestletResourceTest.java
 
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotSchemaRestletResourceTest.java
index 2d51a18..7bfe032 100644
--- 
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotSchemaRestletResourceTest.java
+++ 
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotSchemaRestletResourceTest.java
@@ -182,6 +182,11 @@ public class PinotSchemaRestletResourceTest {
     putMethod = ControllerTestUtils.sendMultipartPutRequest(updateSchemaUrl, 
invalidSchemaString);
     Assert.assertEquals(putMethod.getStatusCode(), 400);
 
+    // Update schema with null schema name
+    schema.setSchemaName(null);
+    putMethod = ControllerTestUtils.sendMultipartPutRequest(updateSchemaUrl, 
schema.toSingleLineJsonString());
+    Assert.assertEquals(putMethod.getStatusCode(), 400);
+
     // Update schema with non-matching schema name
     String newSchemaName = "newSchemaName";
     schema.setSchemaName(newSchemaName);

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to