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]