This is an automated email from the ASF dual-hosted git repository. github-actions[bot] pushed a commit to branch cherry-pick-29a566bc-to-branch-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit 8c823414b488f1a19c56e9ea0166a03a75f3d57f Author: Fayupable <[email protected]> AuthorDate: Wed Sep 2 11:39:27 2026 +0300 [#12788] fix(server): reject null request body before validate() in create/register/add operations (#12791) ### What changes were proposed in this pull request? Several `create`/`register`/`add` REST operations called `request.validate()` without first checking for a null request body, so an empty or JSON `null` body dereferenced the request and returned HTTP 500 instead of a proper 400. Applied the existing `createMetalake` pattern (null-check first, then route an `IllegalArgumentException` through the corresponding exception handler) to: - Catalog, Schema, Table, Fileset, Topic, Policy, and Tag creation - Function, Model, and JobTemplate registration - User, Group, and Role creation - Bulk user and group creation Also updated a pre-existing test in `TestGroupOperations` that had asserted the old 500 status as expected behavior, and added a `WithNullRequest` test for each fixed operation, matching the `assertNullRequestBodyRejected` pattern from #12770. ### Why are the changes needed? A null request body currently returns HTTP 500 with an internal error response for these operations, instead of a structured 400. This is inconsistent with the already-fixed `alter`/`createView` operations (#12769, #12770) and with `createMetalake`, and it leaks an internal error to the caller for what is really a bad request. Fix: #12788 ### Does this PR introduce _any_ user-facing change? Yes. A `create`/`register`/`add` request with a null or empty body now returns HTTP 400 with error code `1001`, error type `IllegalArgumentException`, and a message stating the request body cannot be null, instead of HTTP 500. ### How was this patch tested? Added a `testXxxWithNullRequest()` test for each of the 14 fixed operations, verifying HTTP 400, error code `1001`, error type `IllegalArgumentException`, and the error message, using the shared `assertNullRequestBodyRejected` helper. Ran `:server:test` scoped to the 14 affected test classes: 176 tests, all passing. # Conflicts: # server/src/main/java/org/apache/gravitino/server/web/rest/BulkOperations.java # server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java # server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java # server/src/main/java/org/apache/gravitino/server/web/rest/FunctionOperations.java # server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java # server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java # server/src/main/java/org/apache/gravitino/server/web/rest/ModelOperations.java # server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java # server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java # server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.java # server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java # server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java # server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.java # server/src/main/java/org/apache/gravitino/server/web/rest/UserOperations.java # server/src/test/java/org/apache/gravitino/server/web/rest/TestBulkOperations.java # server/src/test/java/org/apache/gravitino/server/web/rest/TestGroupOperations.java --- .../gravitino/server/web/rest/BulkOperations.java | 301 +++++++++++++++++ .../server/web/rest/CatalogOperations.java | 13 + .../server/web/rest/FilesetOperations.java | 13 + .../server/web/rest/FunctionOperations.java | 13 + .../gravitino/server/web/rest/GroupOperations.java | 13 + .../gravitino/server/web/rest/JobOperations.java | 14 + .../gravitino/server/web/rest/ModelOperations.java | 14 + .../server/web/rest/PolicyOperations.java | 13 + .../gravitino/server/web/rest/RoleOperations.java | 13 + .../server/web/rest/SchemaOperations.java | 14 + .../gravitino/server/web/rest/TableOperations.java | 14 + .../gravitino/server/web/rest/TagOperations.java | 13 + .../gravitino/server/web/rest/TopicOperations.java | 13 + .../gravitino/server/web/rest/UserOperations.java | 13 + .../server/web/rest/TestBulkOperations.java | 368 +++++++++++++++++++++ .../server/web/rest/TestCatalogOperations.java | 11 + .../server/web/rest/TestFilesetOperations.java | 11 + .../server/web/rest/TestFunctionOperations.java | 11 + .../server/web/rest/TestGroupOperations.java | 57 ++++ .../server/web/rest/TestJobOperations.java | 16 + .../server/web/rest/TestModelOperations.java | 11 + .../server/web/rest/TestPolicyOperations.java | 11 + .../server/web/rest/TestRoleOperations.java | 11 + .../server/web/rest/TestSchemaOperations.java | 11 + .../server/web/rest/TestTableOperations.java | 11 + .../server/web/rest/TestTagOperations.java | 11 + .../server/web/rest/TestTopicOperations.java | 11 + .../server/web/rest/TestUserOperations.java | 11 + 28 files changed, 1036 insertions(+) diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/BulkOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/BulkOperations.java new file mode 100644 index 0000000000..779b463dde --- /dev/null +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/BulkOperations.java @@ -0,0 +1,301 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.server.web.rest; + +import com.codahale.metrics.annotation.ResponseMetered; +import com.codahale.metrics.annotation.Timed; +import java.util.Arrays; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; +import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.POST; +import javax.ws.rs.Path; +import javax.ws.rs.PathParam; +import javax.ws.rs.Produces; +import javax.ws.rs.core.Context; +import javax.ws.rs.core.Response; +import org.apache.gravitino.Entity; +import org.apache.gravitino.GravitinoEnv; +import org.apache.gravitino.MetadataObject; +import org.apache.gravitino.MetadataObjects; +import org.apache.gravitino.authorization.AccessControlDispatcher; +import org.apache.gravitino.authorization.Group; +import org.apache.gravitino.authorization.Owner; +import org.apache.gravitino.authorization.OwnerDispatcher; +import org.apache.gravitino.authorization.User; +import org.apache.gravitino.bulk.BulkItemResult; +import org.apache.gravitino.bulk.BulkManager; +import org.apache.gravitino.bulk.GroupAdd; +import org.apache.gravitino.bulk.UserAdd; +import org.apache.gravitino.dto.authorization.GroupDTO; +import org.apache.gravitino.dto.authorization.UserDTO; +import org.apache.gravitino.dto.requests.BulkGroupAddRequest; +import org.apache.gravitino.dto.requests.BulkRemoveRequest; +import org.apache.gravitino.dto.requests.BulkUserAddRequest; +import org.apache.gravitino.dto.responses.BulkError; +import org.apache.gravitino.dto.responses.BulkGroupResponse; +import org.apache.gravitino.dto.responses.BulkRemoveResponse; +import org.apache.gravitino.dto.responses.BulkSummary; +import org.apache.gravitino.dto.responses.BulkUserResponse; +import org.apache.gravitino.dto.util.DTOConverters; +import org.apache.gravitino.metalake.MetalakeManager; +import org.apache.gravitino.metrics.MetricNames; +import org.apache.gravitino.server.authorization.NameBindings; +import org.apache.gravitino.server.authorization.annotations.AuthorizationExpression; +import org.apache.gravitino.server.authorization.annotations.AuthorizationMetadata; +import org.apache.gravitino.server.web.Utils; + +/** Provides best-effort bulk APIs for metalake access-control entities. */ [email protected] +@Path("/bulk/metalakes/{metalake}") +public class BulkOperations { + + private static final String USERS_FIELD_NAME = "users"; + private static final String GROUPS_FIELD_NAME = "groups"; + private static final String NAMES_FIELD_NAME = "names"; + + private final BulkManager bulkManager; + private final AccessControlDispatcher accessControlDispatcher; + private final OwnerDispatcher ownerDispatcher; + + @Context private HttpServletRequest httpRequest; + + /** Creates a new bulk operations resource. */ + public BulkOperations() { + this.bulkManager = GravitinoEnv.getInstance().bulkManager(); + this.accessControlDispatcher = GravitinoEnv.getInstance().accessControlDispatcher(); + this.ownerDispatcher = GravitinoEnv.getInstance().ownerDispatcher(); + } + + /** + * Adds users in bulk. + * + * @param metalake The metalake name. + * @param request The bulk user add request. + * @return The bulk user response. + */ + @POST + @Path("users/add") + @Produces("application/vnd.gravitino.v1+json") + @Timed(name = "bulk-add-user." + MetricNames.HTTP_PROCESS_DURATION, absolute = true) + @ResponseMetered(name = "bulk-add-user", absolute = true) + @AuthorizationExpression(expression = "METALAKE::OWNER || METALAKE::MANAGE_USERS") + public Response addUsers( + @PathParam("metalake") @AuthorizationMetadata(type = Entity.EntityType.METALAKE) + String metalake, + BulkUserAddRequest request) { + if (request == null) { + return ExceptionHandlers.handleUserException( + OperationType.ADD, + "", + metalake, + new IllegalArgumentException("Request body cannot be null")); + } + + try { + return Utils.doAs( + httpRequest, + () -> { + request.validate(); + bulkManager.checkBulkSize(USERS_FIELD_NAME, request.getUsers().length); + MetalakeManager.checkMetalakeInUse(metalake); + List<BulkItemResult<User>> results = + accessControlDispatcher.addUsers( + metalake, + Arrays.stream(request.getUsers()) + .map( + user -> + new UserAdd( + user.getName(), user.getExternalId(), user.getEnabled())) + .collect(Collectors.toList())); + UserDTO[] users = + results.stream() + .filter(BulkItemResult::succeeded) + .map(result -> DTOConverters.toDTO(result.value().get())) + .toArray(UserDTO[]::new); + BulkError[] errors = + results.stream() + .filter(result -> !result.succeeded()) + .map(bulkManager::toBulkError) + .toArray(BulkError[]::new); + return Utils.ok( + new BulkUserResponse( + users, errors, new BulkSummary(results.size(), users.length, errors.length))); + }); + } catch (Exception e) { + return ExceptionHandlers.handleUserException(OperationType.ADD, "", metalake, e); + } + } + + /** + * Removes users in bulk. + * + * @param metalake The metalake name. + * @param request The bulk remove request. + * @return The bulk remove response. + */ + @POST + @Path("users/remove") + @Produces("application/vnd.gravitino.v1+json") + @Timed(name = "bulk-remove-user." + MetricNames.HTTP_PROCESS_DURATION, absolute = true) + @ResponseMetered(name = "bulk-remove-user", absolute = true) + @AuthorizationExpression(expression = "METALAKE::OWNER || METALAKE::MANAGE_USERS") + public Response removeUsers( + @PathParam("metalake") @AuthorizationMetadata(type = Entity.EntityType.METALAKE) + String metalake, + BulkRemoveRequest request) { + try { + return Utils.doAs( + httpRequest, + () -> { + request.validate(); + bulkManager.checkBulkSize(NAMES_FIELD_NAME, request.getNames().length); + MetalakeManager.checkMetalakeInUse(metalake); + Optional<Owner> metalakeOwner = + ownerDispatcher.getOwner( + metalake, MetadataObjects.of(null, metalake, MetadataObject.Type.METALAKE)); + List<BulkItemResult<String>> results = + accessControlDispatcher.removeUsers( + metalake, Arrays.asList(request.getNames()), metalakeOwner); + String[] names = + results.stream() + .filter(BulkItemResult::succeeded) + .map(BulkItemResult::name) + .toArray(String[]::new); + BulkError[] errors = + results.stream() + .filter(result -> !result.succeeded()) + .map(bulkManager::toBulkError) + .toArray(BulkError[]::new); + return Utils.ok( + new BulkRemoveResponse( + names, errors, new BulkSummary(results.size(), names.length, errors.length))); + }); + } catch (Exception e) { + return ExceptionHandlers.handleUserException(OperationType.REMOVE, "", metalake, e); + } + } + + /** + * Adds groups in bulk. + * + * @param metalake The metalake name. + * @param request The bulk group add request. + * @return The bulk group response. + */ + @POST + @Path("groups/add") + @Produces("application/vnd.gravitino.v1+json") + @Timed(name = "bulk-add-group." + MetricNames.HTTP_PROCESS_DURATION, absolute = true) + @ResponseMetered(name = "bulk-add-group", absolute = true) + @AuthorizationExpression(expression = "METALAKE::OWNER || METALAKE::MANAGE_GROUPS") + public Response addGroups( + @PathParam("metalake") @AuthorizationMetadata(type = Entity.EntityType.METALAKE) + String metalake, + BulkGroupAddRequest request) { + if (request == null) { + return ExceptionHandlers.handleGroupException( + OperationType.ADD, + "", + metalake, + new IllegalArgumentException("Request body cannot be null")); + } + + try { + return Utils.doAs( + httpRequest, + () -> { + request.validate(); + bulkManager.checkBulkSize(GROUPS_FIELD_NAME, request.getGroups().length); + MetalakeManager.checkMetalakeInUse(metalake); + List<BulkItemResult<Group>> results = + accessControlDispatcher.addGroups( + metalake, + Arrays.stream(request.getGroups()) + .map(group -> new GroupAdd(group.getName(), group.getExternalId())) + .collect(Collectors.toList())); + GroupDTO[] groups = + results.stream() + .filter(BulkItemResult::succeeded) + .map(result -> DTOConverters.toDTO(result.value().get())) + .toArray(GroupDTO[]::new); + BulkError[] errors = + results.stream() + .filter(result -> !result.succeeded()) + .map(bulkManager::toBulkError) + .toArray(BulkError[]::new); + return Utils.ok( + new BulkGroupResponse( + groups, errors, new BulkSummary(results.size(), groups.length, errors.length))); + }); + } catch (Exception e) { + return ExceptionHandlers.handleGroupException(OperationType.ADD, "", metalake, e); + } + } + + /** + * Removes groups in bulk. + * + * @param metalake The metalake name. + * @param request The bulk remove request. + * @return The bulk remove response. + */ + @POST + @Path("groups/remove") + @Produces("application/vnd.gravitino.v1+json") + @Timed(name = "bulk-remove-group." + MetricNames.HTTP_PROCESS_DURATION, absolute = true) + @ResponseMetered(name = "bulk-remove-group", absolute = true) + @AuthorizationExpression(expression = "METALAKE::OWNER || METALAKE::MANAGE_GROUPS") + public Response removeGroups( + @PathParam("metalake") @AuthorizationMetadata(type = Entity.EntityType.METALAKE) + String metalake, + BulkRemoveRequest request) { + try { + return Utils.doAs( + httpRequest, + () -> { + request.validate(); + bulkManager.checkBulkSize(NAMES_FIELD_NAME, request.getNames().length); + MetalakeManager.checkMetalakeInUse(metalake); + Optional<Owner> metalakeOwner = + ownerDispatcher.getOwner( + metalake, MetadataObjects.of(null, metalake, MetadataObject.Type.METALAKE)); + List<BulkItemResult<String>> results = + accessControlDispatcher.removeGroups( + metalake, Arrays.asList(request.getNames()), metalakeOwner); + String[] names = + results.stream() + .filter(BulkItemResult::succeeded) + .map(BulkItemResult::name) + .toArray(String[]::new); + BulkError[] errors = + results.stream() + .filter(result -> !result.succeeded()) + .map(bulkManager::toBulkError) + .toArray(BulkError[]::new); + return Utils.ok( + new BulkRemoveResponse( + names, errors, new BulkSummary(results.size(), names.length, errors.length))); + }); + } catch (Exception e) { + return ExceptionHandlers.handleGroupException(OperationType.REMOVE, "", metalake, e); + } + } +} diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java index 764fd8e0c0..7b077834ca 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java @@ -141,6 +141,19 @@ public class CatalogOperations { @PathParam("metalake") @AuthorizationMetadata(type = Entity.EntityType.METALAKE) String metalake, CatalogCreateRequest request) { +<<<<<<< HEAD +======= + if (request == null) { + LOG.warn("Received create catalog request with null request body"); + return ExceptionHandlers.handleCatalogException( + OperationType.CREATE, + "", + metalake, + new IllegalArgumentException("Request body cannot be null")); + } + + String catalogName = request.getName(); +>>>>>>> 29a566bc6 ([#12788] fix(server): reject null request body before validate() in create/register/add operations (#12791)) LOG.info("Received create catalog request for metalake: {}", metalake); try { return Utils.doAs( diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java index 1e534675df..f2c6e8a896 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/FilesetOperations.java @@ -144,6 +144,19 @@ public class FilesetOperations { @PathParam("catalog") @AuthorizationMetadata(type = Entity.EntityType.CATALOG) String catalog, @PathParam("schema") @AuthorizationMetadata(type = Entity.EntityType.SCHEMA) String schema, FilesetCreateRequest request) { +<<<<<<< HEAD +======= + if (request == null) { + LOG.warn("Received create fileset request with null request body"); + return ExceptionHandlers.handleFilesetException( + OperationType.CREATE, + "", + schema, + new IllegalArgumentException("Request body cannot be null")); + } + + String filesetName = request.getName(); +>>>>>>> 29a566bc6 ([#12788] fix(server): reject null request body before validate() in create/register/add operations (#12791)) LOG.info( "Received create fileset request: {}.{}.{}.{}", metalake, diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/FunctionOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/FunctionOperations.java index adc62d937c..b589f3a347 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/FunctionOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/FunctionOperations.java @@ -160,6 +160,19 @@ public class FunctionOperations { @PathParam("catalog") @AuthorizationMetadata(type = Entity.EntityType.CATALOG) String catalog, @PathParam("schema") @AuthorizationMetadata(type = Entity.EntityType.SCHEMA) String schema, FunctionRegisterRequest request) { +<<<<<<< HEAD +======= + if (request == null) { + LOG.warn("Received register function request with null request body"); + return ExceptionHandlers.handleFunctionException( + OperationType.REGISTER, + "", + schema, + new IllegalArgumentException("Request body cannot be null")); + } + + String functionName = request.getName(); +>>>>>>> 29a566bc6 ([#12788] fix(server): reject null request body before validate() in create/register/add operations (#12791)) LOG.info( "Received register function request: {}.{}.{}.{}", metalake, diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java index 8ed1850650..d73f9a46a5 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/GroupOperations.java @@ -112,6 +112,19 @@ public class GroupOperations { @PathParam("metalake") @AuthorizationMetadata(type = Entity.EntityType.METALAKE) String metalake, GroupAddRequest request) { +<<<<<<< HEAD +======= + if (request == null) { + LOG.warn("Received add group request with null request body"); + return ExceptionHandlers.handleGroupException( + OperationType.ADD, + "", + metalake, + new IllegalArgumentException("Request body cannot be null")); + } + + String groupName = request.getName(); +>>>>>>> 29a566bc6 ([#12788] fix(server): reject null request body before validate() in create/register/add operations (#12791)) try { return Utils.doAs( httpRequest, diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java index a1b456ed98..a6c97200dc 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/JobOperations.java @@ -150,6 +150,20 @@ public class JobOperations { @PathParam("metalake") @AuthorizationMetadata(type = Entity.EntityType.METALAKE) String metalake, JobTemplateRegisterRequest request) { +<<<<<<< HEAD +======= + if (request == null) { + LOG.warn("Received register job template request with null request body"); + return ExceptionHandlers.handleJobTemplateException( + OperationType.REGISTER, + "", + metalake, + new IllegalArgumentException("Request body cannot be null")); + } + + String jobTemplateName = + request.getJobTemplate() == null ? "" : request.getJobTemplate().name(); +>>>>>>> 29a566bc6 ([#12788] fix(server): reject null request body before validate() in create/register/add operations (#12791)) LOG.info( "Received request to register job template {} in metalake: {}", request.getJobTemplate().name(), diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/ModelOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/ModelOperations.java index f4d7741f49..d252b5d165 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/ModelOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/ModelOperations.java @@ -172,12 +172,26 @@ public class ModelOperations { @PathParam("catalog") @AuthorizationMetadata(type = Entity.EntityType.CATALOG) String catalog, @PathParam("schema") @AuthorizationMetadata(type = Entity.EntityType.SCHEMA) String schema, ModelRegisterRequest request) { +<<<<<<< HEAD LOG.info( "Received register model request: {}.{}.{}.{}", metalake, catalog, schema, request.getName()); +======= + if (request == null) { + LOG.warn("Received register model request with null request body"); + return ExceptionHandlers.handleModelException( + OperationType.REGISTER, + "", + schema, + new IllegalArgumentException("Request body cannot be null")); + } + + String modelName = request.getName(); + LOG.info("Received register model request: {}.{}.{}.{}", metalake, catalog, schema, modelName); +>>>>>>> 29a566bc6 ([#12788] fix(server): reject null request body before validate() in create/register/add operations (#12791)) try { return Utils.doAs( diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java index 54be773aa0..3119507490 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/PolicyOperations.java @@ -141,6 +141,19 @@ public class PolicyOperations { @PathParam("metalake") @AuthorizationMetadata(type = Entity.EntityType.METALAKE) String metalake, PolicyCreateRequest request) { +<<<<<<< HEAD +======= + if (request == null) { + LOG.warn("Received create policy request with null request body"); + return ExceptionHandlers.handlePolicyException( + OperationType.CREATE, + "", + metalake, + new IllegalArgumentException("Request body cannot be null")); + } + + String policyName = request.getName(); +>>>>>>> 29a566bc6 ([#12788] fix(server): reject null request body before validate() in create/register/add operations (#12791)) LOG.info("Received create policy request under metalake: {}", metalake); try { diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java index 684ac03b6f..b97e359996 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java @@ -140,6 +140,19 @@ public class RoleOperations { @PathParam("metalake") @AuthorizationMetadata(type = Entity.EntityType.METALAKE) String metalake, RoleCreateRequest request) { +<<<<<<< HEAD +======= + if (request == null) { + LOG.warn("Received create role request with null request body"); + return ExceptionHandlers.handleRoleException( + OperationType.CREATE, + "", + metalake, + new IllegalArgumentException("Request body cannot be null")); + } + + String roleName = request.getName(); +>>>>>>> 29a566bc6 ([#12788] fix(server): reject null request body before validate() in create/register/add operations (#12791)) try { return Utils.doAs( diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.java index 70ff20d448..fc45a218fd 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/SchemaOperations.java @@ -141,7 +141,21 @@ public class SchemaOperations { @PathParam("catalog") @AuthorizationMetadata(type = Entity.EntityType.CATALOG) String catalog, @AuthorizationRequest(type = AuthorizationRequest.RequestType.CREATE_SCHEMA) SchemaCreateRequest request) { +<<<<<<< HEAD LOG.info("Received create schema request: {}.{}.{}", metalake, catalog, request.getName()); +======= + if (request == null) { + LOG.warn("Received create schema request with null request body"); + return ExceptionHandlers.handleSchemaException( + OperationType.CREATE, + "", + catalog, + new IllegalArgumentException("Request body cannot be null")); + } + + String schemaName = request.getName(); + LOG.info("Received create schema request: {}.{}.{}", metalake, catalog, schemaName); +>>>>>>> 29a566bc6 ([#12788] fix(server): reject null request body before validate() in create/register/add operations (#12791)) try { return Utils.doAs( httpRequest, diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java index 9d653e75d2..4cc53b4ce6 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/TableOperations.java @@ -129,8 +129,22 @@ public class TableOperations { @PathParam("catalog") @AuthorizationMetadata(type = Entity.EntityType.CATALOG) String catalog, @PathParam("schema") @AuthorizationMetadata(type = Entity.EntityType.SCHEMA) String schema, TableCreateRequest request) { +<<<<<<< HEAD LOG.info( "Received create table request: {}.{}.{}.{}", metalake, catalog, schema, request.getName()); +======= + if (request == null) { + LOG.warn("Received create table request with null request body"); + return ExceptionHandlers.handleTableException( + OperationType.CREATE, + "", + schema, + new IllegalArgumentException("Request body cannot be null")); + } + + String tableName = request.getName(); + LOG.info("Received create table request: {}.{}.{}.{}", metalake, catalog, schema, tableName); +>>>>>>> 29a566bc6 ([#12788] fix(server): reject null request body before validate() in create/register/add operations (#12791)) try { return Utils.doAs( httpRequest, diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java index 7886856173..ebddbead98 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/TagOperations.java @@ -148,6 +148,19 @@ public class TagOperations { @PathParam("metalake") @AuthorizationMetadata(type = Entity.EntityType.METALAKE) String metalake, TagCreateRequest request) { +<<<<<<< HEAD +======= + if (request == null) { + LOG.warn("Received create tag request with null request body"); + return ExceptionHandlers.handleTagException( + OperationType.CREATE, + "", + metalake, + new IllegalArgumentException("Request body cannot be null")); + } + + String tagName = request.getName(); +>>>>>>> 29a566bc6 ([#12788] fix(server): reject null request body before validate() in create/register/add operations (#12791)) LOG.info("Received create tag request under metalake: {}", metalake); try { diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.java index 214e184ceb..7b1fe977d8 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/TopicOperations.java @@ -124,6 +124,19 @@ public class TopicOperations { @PathParam("catalog") @AuthorizationMetadata(type = Entity.EntityType.CATALOG) String catalog, @PathParam("schema") @AuthorizationMetadata(type = Entity.EntityType.SCHEMA) String schema, TopicCreateRequest request) { +<<<<<<< HEAD +======= + if (request == null) { + LOG.warn("Received create topic request with null request body"); + return ExceptionHandlers.handleTopicException( + OperationType.CREATE, + "", + schema, + new IllegalArgumentException("Request body cannot be null")); + } + + String topicName = request.getName(); +>>>>>>> 29a566bc6 ([#12788] fix(server): reject null request body before validate() in create/register/add operations (#12791)) LOG.info("Received create topic request: {}.{}.{}", metalake, catalog, schema); try { return Utils.doAs( diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/UserOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/UserOperations.java index 5bce5e8fc9..6cd4ff912e 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/UserOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/UserOperations.java @@ -153,6 +153,19 @@ public class UserOperations { @PathParam("metalake") @AuthorizationMetadata(type = Entity.EntityType.METALAKE) String metalake, UserAddRequest request) { +<<<<<<< HEAD +======= + if (request == null) { + LOG.warn("Received add user request with null request body"); + return ExceptionHandlers.handleUserException( + OperationType.ADD, + "", + metalake, + new IllegalArgumentException("Request body cannot be null")); + } + + String userName = request.getName(); +>>>>>>> 29a566bc6 ([#12788] fix(server): reject null request body before validate() in create/register/add operations (#12791)) try { return Utils.doAs( httpRequest, diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestBulkOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestBulkOperations.java new file mode 100644 index 0000000000..1d843b4728 --- /dev/null +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestBulkOperations.java @@ -0,0 +1,368 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.gravitino.server.web.rest; + +import static org.apache.gravitino.Configs.TREE_LOCK_CLEAN_INTERVAL; +import static org.apache.gravitino.Configs.TREE_LOCK_MAX_NODE_IN_MEMORY; +import static org.apache.gravitino.Configs.TREE_LOCK_MIN_NODE_IN_MEMORY; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.RETURNS_DEFAULTS; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.time.Instant; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.client.Entity; +import javax.ws.rs.core.Application; +import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.Response; +import org.apache.commons.lang3.reflect.FieldUtils; +import org.apache.gravitino.Config; +import org.apache.gravitino.EntityStore; +import org.apache.gravitino.GravitinoEnv; +import org.apache.gravitino.authorization.AccessControlManager; +import org.apache.gravitino.authorization.Group; +import org.apache.gravitino.authorization.OwnerDispatcher; +import org.apache.gravitino.authorization.User; +import org.apache.gravitino.bulk.BulkItemResult; +import org.apache.gravitino.bulk.BulkManager; +import org.apache.gravitino.bulk.GroupAdd; +import org.apache.gravitino.bulk.UserAdd; +import org.apache.gravitino.config.ConfigEntry; +import org.apache.gravitino.connector.PropertiesMetadata; +import org.apache.gravitino.dto.requests.BulkGroupAddRequest; +import org.apache.gravitino.dto.requests.BulkRemoveRequest; +import org.apache.gravitino.dto.requests.BulkUserAddRequest; +import org.apache.gravitino.dto.requests.GroupAddRequest; +import org.apache.gravitino.dto.requests.UserAddRequest; +import org.apache.gravitino.dto.responses.BulkGroupResponse; +import org.apache.gravitino.dto.responses.BulkRemoveResponse; +import org.apache.gravitino.dto.responses.BulkUserResponse; +import org.apache.gravitino.dto.responses.ErrorConstants; +import org.apache.gravitino.dto.responses.ErrorResponse; +import org.apache.gravitino.exceptions.GroupAlreadyExistsException; +import org.apache.gravitino.exceptions.NoSuchGroupException; +import org.apache.gravitino.exceptions.NoSuchUserException; +import org.apache.gravitino.exceptions.UserAlreadyExistsException; +import org.apache.gravitino.lock.LockManager; +import org.apache.gravitino.meta.AuditInfo; +import org.apache.gravitino.meta.BaseMetalake; +import org.apache.gravitino.meta.GroupEntity; +import org.apache.gravitino.meta.UserEntity; +import org.apache.gravitino.rest.RESTUtils; +import org.glassfish.hk2.utilities.binding.AbstractBinder; +import org.glassfish.jersey.server.ResourceConfig; +import org.glassfish.jersey.test.TestProperties; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; + +public class TestBulkOperations extends BaseOperationsTest { + + private static final AccessControlManager manager = mock(AccessControlManager.class); + private static final EntityStore entityStore = mock(EntityStore.class); + private static final OwnerDispatcher ownerDispatcher = mock(OwnerDispatcher.class); + private static BulkOperations bulkOperations; + + private static class MockServletRequestFactory extends ServletRequestFactoryBase { + @Override + public HttpServletRequest get() { + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getRemoteUser()).thenReturn(null); + return request; + } + } + + @BeforeAll + public static void setup() throws IllegalAccessException { + Config config = + mock( + Config.class, + invocation -> { + if ("get".equals(invocation.getMethod().getName()) + && invocation.getArguments().length == 1 + && invocation.getArgument(0) instanceof ConfigEntry) { + ConfigEntry<?> entry = invocation.getArgument(0); + return entry.getDefaultValue(); + } + return RETURNS_DEFAULTS.answer(invocation); + }); + doReturn(100000L).when(config).get(TREE_LOCK_MAX_NODE_IN_MEMORY); + doReturn(1000L).when(config).get(TREE_LOCK_MIN_NODE_IN_MEMORY); + doReturn(36000L).when(config).get(TREE_LOCK_CLEAN_INTERVAL); + doReturn(2).when(config).get(org.apache.gravitino.Configs.BULK_MAX_ITEMS); + FieldUtils.writeField(GravitinoEnv.getInstance(), "config", config, true); + FieldUtils.writeField(GravitinoEnv.getInstance(), "lockManager", new LockManager(config), true); + FieldUtils.writeField(GravitinoEnv.getInstance(), "accessControlDispatcher", manager, true); + FieldUtils.writeField(GravitinoEnv.getInstance(), "ownerDispatcher", ownerDispatcher, true); + FieldUtils.writeField(GravitinoEnv.getInstance(), "bulkManager", new BulkManager(config), true); + FieldUtils.writeField(GravitinoEnv.getInstance(), "entityStore", entityStore, true); + bulkOperations = new BulkOperations(); + } + + @BeforeEach + public void resetMocks() throws IOException { + Mockito.reset(manager, entityStore, ownerDispatcher); + BaseMetalake metalake = mock(BaseMetalake.class); + PropertiesMetadata propertiesMetadata = mock(PropertiesMetadata.class); + when(propertiesMetadata.getOrDefault(any(), any())).thenReturn(true); + when(metalake.propertiesMetadata()).thenReturn(propertiesMetadata); + when(entityStore.get(any(), any(), any())).thenReturn(metalake); + when(ownerDispatcher.getOwner(any(), any())).thenReturn(Optional.empty()); + } + + @Override + protected Application configure() { + try { + forceSet( + TestProperties.CONTAINER_PORT, String.valueOf(RESTUtils.findAvailablePort(2000, 3000))); + } catch (IOException e) { + throw new RuntimeException(e); + } + + ResourceConfig resourceConfig = new ResourceConfig(); + resourceConfig.register(bulkOperations); + resourceConfig.register( + new AbstractBinder() { + @Override + protected void configure() { + bindFactory(MockServletRequestFactory.class).to(HttpServletRequest.class); + } + }); + + return resourceConfig; + } + + @Test + public void testBulkAddUsersWithNullRequest() { + Response resp = + target("/bulk/metalakes/metalake1/users/add") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + + @Test + public void testBulkAddUsersBestEffort() { + User user1 = buildUser("user1"); + when(manager.addUsers(any(), any())) + .thenReturn( + Arrays.asList( + BulkItemResult.success(0, "user1", user1), + BulkItemResult.failure( + 1, "user2", new UserAlreadyExistsException("User already exists: user2")))); + + BulkUserAddRequest request = + new BulkUserAddRequest( + new UserAddRequest[] { + new UserAddRequest("user1", "ext-user1", false), new UserAddRequest("user2") + }); + Response response = + target("/bulk/metalakes/metalake1/users/add") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + BulkUserResponse bulkResponse = response.readEntity(BulkUserResponse.class); + Assertions.assertEquals(1, bulkResponse.getUsers().length); + Assertions.assertEquals("user1", bulkResponse.getUsers()[0].name()); + Assertions.assertEquals(1, bulkResponse.getErrors().length); + Assertions.assertEquals(1, bulkResponse.getErrors()[0].getIndex()); + Assertions.assertEquals("user2", bulkResponse.getErrors()[0].getName()); + Assertions.assertEquals( + ErrorConstants.ALREADY_EXISTS_CODE, bulkResponse.getErrors()[0].getCode()); + Assertions.assertEquals(2, bulkResponse.getSummary().getTotal()); + Assertions.assertEquals(1, bulkResponse.getSummary().getSucceeded()); + Assertions.assertEquals(1, bulkResponse.getSummary().getFailed()); + + ArgumentCaptor<List<UserAdd>> usersCaptor = ArgumentCaptor.forClass(List.class); + Mockito.verify(manager).addUsers(eq("metalake1"), usersCaptor.capture()); + Assertions.assertEquals("user1", usersCaptor.getValue().get(0).name()); + Assertions.assertEquals("ext-user1", usersCaptor.getValue().get(0).externalId()); + Assertions.assertEquals(false, usersCaptor.getValue().get(0).enabled()); + } + + @Test + public void testBulkRemoveUsersBestEffort() { + when(manager.removeUsers(any(), any(), any())) + .thenReturn( + Arrays.asList( + BulkItemResult.success(0, "user1"), + BulkItemResult.failure( + 1, "ghost", new NoSuchUserException("User does not exist: ghost")))); + + BulkRemoveRequest request = new BulkRemoveRequest(new String[] {"user1", "ghost"}); + Response response = + target("/bulk/metalakes/metalake1/users/remove") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + BulkRemoveResponse bulkResponse = response.readEntity(BulkRemoveResponse.class); + Assertions.assertArrayEquals(new String[] {"user1"}, bulkResponse.getNames()); + Assertions.assertEquals(1, bulkResponse.getErrors().length); + Assertions.assertEquals("ghost", bulkResponse.getErrors()[0].getName()); + Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE, bulkResponse.getErrors()[0].getCode()); + } + + @Test + public void testBulkAddGroupsWithNullRequest() { + Response resp = + target("/bulk/metalakes/metalake1/groups/add") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + + @Test + public void testBulkAddGroupsBestEffort() { + Group group1 = buildGroup("group1"); + when(manager.addGroups(any(), any())) + .thenReturn( + Arrays.asList( + BulkItemResult.success(0, "group1", group1), + BulkItemResult.failure( + 1, "group2", new GroupAlreadyExistsException("Group already exists: group2")))); + + BulkGroupAddRequest request = + new BulkGroupAddRequest( + new GroupAddRequest[] { + new GroupAddRequest("group1", "ext-group1"), new GroupAddRequest("group2") + }); + Response response = + target("/bulk/metalakes/metalake1/groups/add") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + BulkGroupResponse bulkResponse = response.readEntity(BulkGroupResponse.class); + Assertions.assertEquals(1, bulkResponse.getGroups().length); + Assertions.assertEquals("group1", bulkResponse.getGroups()[0].name()); + Assertions.assertEquals(1, bulkResponse.getErrors().length); + Assertions.assertEquals(1, bulkResponse.getErrors()[0].getIndex()); + Assertions.assertEquals("group2", bulkResponse.getErrors()[0].getName()); + Assertions.assertEquals( + ErrorConstants.ALREADY_EXISTS_CODE, bulkResponse.getErrors()[0].getCode()); + Assertions.assertEquals(2, bulkResponse.getSummary().getTotal()); + Assertions.assertEquals(1, bulkResponse.getSummary().getSucceeded()); + Assertions.assertEquals(1, bulkResponse.getSummary().getFailed()); + + ArgumentCaptor<List<GroupAdd>> groupsCaptor = ArgumentCaptor.forClass(List.class); + Mockito.verify(manager).addGroups(eq("metalake1"), groupsCaptor.capture()); + Assertions.assertEquals("group1", groupsCaptor.getValue().get(0).name()); + Assertions.assertEquals("ext-group1", groupsCaptor.getValue().get(0).externalId()); + } + + @Test + public void testBulkRemoveGroupsBestEffort() { + when(manager.removeGroups(any(), any(), any())) + .thenReturn( + Arrays.asList( + BulkItemResult.success(0, "group1"), + BulkItemResult.failure( + 1, "ghost", new NoSuchGroupException("Group does not exist: ghost")))); + + BulkRemoveRequest request = new BulkRemoveRequest(new String[] {"group1", "ghost"}); + Response response = + target("/bulk/metalakes/metalake1/groups/remove") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals(Response.Status.OK.getStatusCode(), response.getStatus()); + BulkRemoveResponse bulkResponse = response.readEntity(BulkRemoveResponse.class); + Assertions.assertArrayEquals(new String[] {"group1"}, bulkResponse.getNames()); + Assertions.assertEquals(1, bulkResponse.getErrors().length); + Assertions.assertEquals("ghost", bulkResponse.getErrors()[0].getName()); + Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE, bulkResponse.getErrors()[0].getCode()); + } + + @Test + public void testBulkRejectsEmptyAndExceededRequest() { + Response emptyResponse = + target("/bulk/metalakes/metalake1/users/add") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post( + Entity.entity( + new BulkUserAddRequest(new UserAddRequest[] {}), + MediaType.APPLICATION_JSON_TYPE)); + Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), emptyResponse.getStatus()); + + BulkRemoveRequest exceededRequest = + new BulkRemoveRequest(new String[] {"user1", "user2", "user3"}); + Response exceededResponse = + target("/bulk/metalakes/metalake1/users/remove") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(exceededRequest, MediaType.APPLICATION_JSON_TYPE)); + Assertions.assertEquals( + Response.Status.BAD_REQUEST.getStatusCode(), exceededResponse.getStatus()); + ErrorResponse errorResponse = exceededResponse.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, errorResponse.getCode()); + + Response emptyGroupResponse = + target("/bulk/metalakes/metalake1/groups/add") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post( + Entity.entity( + new BulkGroupAddRequest(new GroupAddRequest[] {}), + MediaType.APPLICATION_JSON_TYPE)); + Assertions.assertEquals( + Response.Status.BAD_REQUEST.getStatusCode(), emptyGroupResponse.getStatus()); + } + + private User buildUser(String user) { + return UserEntity.builder() + .withId(1L) + .withName(user) + .withRoleNames(Collections.emptyList()) + .withAuditInfo( + AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build()) + .build(); + } + + private Group buildGroup(String group) { + return GroupEntity.builder() + .withId(1L) + .withName(group) + .withRoleNames(Collections.emptyList()) + .withAuditInfo( + AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build()) + .build(); + } +} diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java index c158dacdbf..dfe09762e6 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java @@ -218,6 +218,17 @@ public class TestCatalogOperations extends BaseOperationsTest { Assertions.assertEquals(NoSuchMetalakeException.class.getSimpleName(), errorResponse.getType()); } + @Test + public void testCreateCatalogWithNullRequest() { + Response resp = + target("/metalakes/metalake1/catalogs") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testCreateCatalog() { CatalogCreateRequest req = diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java index 4c67ba3132..343a778a77 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java @@ -243,6 +243,17 @@ public class TestFilesetOperations extends BaseOperationsTest { Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResp2.getType()); } + @Test + public void testCreateFilesetWithNullRequest() { + Response resp = + target(filesetPath(metalake, catalog, schema)) + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testCreateFileset() { Fileset fileset = diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFunctionOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFunctionOperations.java index 5aad775be8..820874e935 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFunctionOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFunctionOperations.java @@ -266,6 +266,17 @@ public class TestFunctionOperations extends BaseOperationsTest { Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResp1.getType()); } + @Test + public void testRegisterFunctionWithNullRequest() { + Response resp = + target(functionPath()) + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testRegisterScalarFunction() { NameIdentifier funcId = NameIdentifierUtil.ofFunction(metalake, catalog, schema, "func1"); diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestGroupOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestGroupOperations.java index 56f8089206..712ada34d2 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestGroupOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestGroupOperations.java @@ -122,6 +122,17 @@ public class TestGroupOperations extends BaseOperationsTest { return resourceConfig; } + @Test + public void testAddGroupWithNullRequest() { + Response resp = + target("/metalakes/metalake1/groups") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testAddGroup() throws IOException { GroupAddRequest req = new GroupAddRequest("group1"); @@ -214,6 +225,52 @@ public class TestGroupOperations extends BaseOperationsTest { } @Test +<<<<<<< HEAD +======= + public void testAddGroupWithNullRequestBodyDoesNotExposeNpe() { + Response resp = + target("/metalakes/metalake1/groups") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity("null", MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + + @Test + public void testAddGroupWithExternalId() throws IOException { + GroupAddRequest req = new GroupAddRequest("group1", "ext-group-1"); + Group group = + GroupEntity.builder() + .withId(1L) + .withName("group1") + .withExternalId("ext-group-1") + .withRoleNames(Collections.emptyList()) + .withAuditInfo( + AuditInfo.builder().withCreator("creator").withCreateTime(Instant.now()).build()) + .build(); + + when(manager.addGroup(any(), any(), any())).thenReturn(group); + + BaseMetalake metalake = mock(BaseMetalake.class); + PropertiesMetadata propertiesMetadata = mock(PropertiesMetadata.class); + when(propertiesMetadata.getOrDefault(any(), any())).thenReturn(true); + when(metalake.propertiesMetadata()).thenReturn(propertiesMetadata); + when(entityStore.get(any(), any(), any())).thenReturn(metalake); + + Response resp = + target("/metalakes/metalake1/groups") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(req, MediaType.APPLICATION_JSON_TYPE)); + + Assertions.assertEquals(Status.OK.getStatusCode(), resp.getStatus()); + GroupResponse groupResponse = resp.readEntity(GroupResponse.class); + Assertions.assertEquals("ext-group-1", groupResponse.getGroup().externalId()); + } + + @Test +>>>>>>> 29a566bc6 ([#12788] fix(server): reject null request body before validate() in create/register/add operations (#12791)) public void testGetGroup() throws IOException { Group group = buildGroup("group1"); diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java index 52b072b962..70d7e7b404 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java @@ -236,6 +236,22 @@ public class TestJobOperations extends JerseyTest { Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResp3.getType()); } + @Test + public void testRegisterJobTemplateWithNullRequest() { + Response resp = + target(jobTemplatePath()) + .request(APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(new byte[0], APPLICATION_JSON_TYPE)); + + Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), resp.getStatus()); + ErrorResponse errorResponse = resp.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, errorResponse.getCode()); + Assertions.assertEquals( + IllegalArgumentException.class.getSimpleName(), errorResponse.getType()); + Assertions.assertTrue(errorResponse.getMessage().contains("Request body cannot be null")); + } + @Test public void testRegisterJobTemplate() { JobTemplateEntity template = diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestModelOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestModelOperations.java index 5f26d8e9d1..e69ec2a0b2 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestModelOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestModelOperations.java @@ -310,6 +310,17 @@ public class TestModelOperations extends BaseOperationsTest { assertNullRequestBodyRejected(alterVersionByAliasResponse); } + @Test + public void testRegisterModelWithNullRequest() { + Response resp = + target(modelPath()) + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testRegisterModel() { NameIdentifier modelId = NameIdentifierUtil.ofModel(metalake, catalog, schema, "model1"); diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java index 382e1f9d8b..3095512cac 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPolicyOperations.java @@ -265,6 +265,17 @@ public class TestPolicyOperations extends BaseOperationsTest { Assertions.assertEquals(0, policyListResp2.getPolicies().length); } + @Test + public void testCreatePolicyWithNullRequest() { + Response resp = + target(policyPath(metalake)) + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testCreatePolicy() { ImmutableMap<String, Object> contentFields = ImmutableMap.of("target_file_size_bytes", 1000); diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java index e22a10c616..b0111d1276 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java @@ -148,6 +148,17 @@ public class TestRoleOperations extends BaseOperationsTest { return resourceConfig; } + @Test + public void testCreateRoleWithNullRequest() { + Response resp = + target("/metalakes/metalake1/roles") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testCreateRole() throws IllegalAccessException, NoSuchFieldException, IOException { SecurableObject securableObject = diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestSchemaOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestSchemaOperations.java index 489b0070f6..94469ba1c1 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestSchemaOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestSchemaOperations.java @@ -218,6 +218,17 @@ public class TestSchemaOperations extends BaseOperationsTest { verify(dispatcher, never()).listSchemas(any()); } + @Test + public void testCreateSchemaWithNullRequest() { + Response resp = + target("/metalakes/" + metalake + "/catalogs/" + catalog + "/schemas") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testCreateSchema() { SchemaCreateRequest req = diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java index 9c41650d48..4d11ccb61e 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTableOperations.java @@ -225,6 +225,17 @@ public class TestTableOperations extends BaseOperationsTest { }; } + @Test + public void testCreateTableWithNullRequest() { + Response resp = + target(tablePath(metalake, catalog, schema)) + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testCreateTable() { Column[] columns = diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTagOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTagOperations.java index 4dac1582a5..e5152b8046 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTagOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTagOperations.java @@ -271,6 +271,17 @@ public class TestTagOperations extends BaseOperationsTest { Assertions.assertEquals(0, tagListResp2.getTags().length); } + @Test + public void testCreateTagWithNullRequest() { + Response resp = + target(tagPath(metalake)) + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testCreateTag() { TagEntity tag1 = diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTopicOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTopicOperations.java index 67c9cdbbb4..d26e642e12 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestTopicOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestTopicOperations.java @@ -226,6 +226,17 @@ public class TestTopicOperations extends BaseOperationsTest { Assertions.assertEquals(RuntimeException.class.getSimpleName(), errorResp2.getType()); } + @Test + public void testCreateTopicWithNullRequest() { + Response resp = + target(topicPath(metalake, catalog, schema)) + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testCreateTopic() { Topic topic = mockTopic("topic1", "comment", ImmutableMap.of("key1", "value1")); diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestUserOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestUserOperations.java index bb82a5dcdc..a5940ee74c 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestUserOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestUserOperations.java @@ -118,6 +118,17 @@ public class TestUserOperations extends BaseOperationsTest { return resourceConfig; } + @Test + public void testAddUserWithNullRequest() { + Response resp = + target("/metalakes/metalake1/users") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .post(Entity.entity(new byte[0], MediaType.APPLICATION_JSON_TYPE)); + + assertNullRequestBodyRejected(resp); + } + @Test public void testAddUser() throws IOException { UserAddRequest req = new UserAddRequest("user1");
