From 04e13fbb1cf1aa07c7d9ed3d32e55951bbf67cd7 Mon Sep 17 00:00:00 2001 From: Shivamrut Date: Mon, 2 Mar 2026 11:56:59 +0530 Subject: [PATCH] fix(rest): return 400 for missing required request parameters instead of 500 MissingServletRequestParameterException extends ServletRequestBindingException which extends Exception (not RuntimeException), causing it to bypass the RuntimeException->400 handler and fall into the catch-all Exception->500 handler. Similarly, MissingServletRequestPartException (thrown for missing multipart file parameters) follows the same inheritance chain and was also incorrectly returning 500. Added dedicated handlers for both exceptions that correctly return 400 Bad Request. Fixes #3776 Signed-off-by: Shivamrut --- .../rest/resourceserver/core/RestExceptionHandler.java | 7 +++++++ .../resourceserver/integration/AttachmentTest.java | 2 +- .../resourceserver/integration/ImportExportTest.java | 10 +++++----- .../rest/resourceserver/integration/VendorTest.java | 2 +- .../resourceserver/restdocs/AttachmentSpecTest.java | 8 ++++++++ 5 files changed, 22 insertions(+), 7 deletions(-) diff --git a/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/core/RestExceptionHandler.java b/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/core/RestExceptionHandler.java index c63a6fcfb6..fc3258c90a 100644 --- a/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/core/RestExceptionHandler.java +++ b/rest/resource-server/src/main/java/org/eclipse/sw360/rest/resourceserver/core/RestExceptionHandler.java @@ -28,7 +28,9 @@ import org.springframework.security.core.AuthenticationException; import org.springframework.web.HttpMediaTypeNotSupportedException; import org.springframework.web.HttpRequestMethodNotSupportedException; +import org.springframework.web.bind.MissingServletRequestParameterException; import org.springframework.web.bind.annotation.ControllerAdvice; +import org.springframework.web.multipart.support.MissingServletRequestPartException; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.servlet.resource.NoResourceFoundException; @@ -58,6 +60,11 @@ public ResponseEntity handleMessageNotReadableException(RuntimeExc return new ResponseEntity<>(new ErrorMessage(e, HttpStatus.BAD_REQUEST), HttpStatus.BAD_REQUEST); } + @ExceptionHandler({MissingServletRequestParameterException.class, MissingServletRequestPartException.class}) + public ResponseEntity handleMissingServletRequestParameter(Exception e) { + return new ResponseEntity<>(new ErrorMessage(e, HttpStatus.BAD_REQUEST), HttpStatus.BAD_REQUEST); + } + @ExceptionHandler(RuntimeException.class) public ResponseEntity handleRuntimeException(RuntimeException e) { return new ResponseEntity<>(new ErrorMessage(e, HttpStatus.BAD_REQUEST), HttpStatus.BAD_REQUEST); diff --git a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/AttachmentTest.java b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/AttachmentTest.java index ecedd59450..39e2879ab5 100644 --- a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/AttachmentTest.java +++ b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/AttachmentTest.java @@ -169,6 +169,6 @@ public void should_fail_create_attachment_without_files() throws IOException { requestEntity, String.class); - assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, response.getStatusCode()); + assertEquals(HttpStatus.BAD_REQUEST, response.getStatusCode()); } } diff --git a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/ImportExportTest.java b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/ImportExportTest.java index daf1004e0b..0e8cfcc74f 100644 --- a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/ImportExportTest.java +++ b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/ImportExportTest.java @@ -456,7 +456,7 @@ public void should_fail_upload_component_without_file() throws IOException { requestEntity, String.class); - assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, response.getStatusCode()); + assertEquals(HttpStatus.BAD_REQUEST, response.getStatusCode()); } @Test @@ -475,7 +475,7 @@ public void should_fail_upload_release_without_file() throws IOException { requestEntity, String.class); - assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, response.getStatusCode()); + assertEquals(HttpStatus.BAD_REQUEST, response.getStatusCode()); } @Test @@ -494,7 +494,7 @@ public void should_fail_upload_attachment_without_file() throws IOException { requestEntity, String.class); - assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, response.getStatusCode()); + assertEquals(HttpStatus.BAD_REQUEST, response.getStatusCode()); } @Test @@ -651,8 +651,8 @@ public void should_verify_error_response_structure() throws IOException { String.class); // Verify error response - assertEquals("HTTP status should be INTERNAL_SERVER_ERROR", - HttpStatus.INTERNAL_SERVER_ERROR, response.getStatusCode()); + assertEquals("HTTP status should be BAD_REQUEST", + HttpStatus.BAD_REQUEST, response.getStatusCode()); // Verify error response body String responseBody = response.getBody(); diff --git a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/VendorTest.java b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/VendorTest.java index b9b4cf282e..48b2f4b376 100644 --- a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/VendorTest.java +++ b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/integration/VendorTest.java @@ -492,7 +492,7 @@ public void should_fail_merge_vendors_with_missing_parameters() throws IOExcepti requestEntity, String.class); - assertEquals(HttpStatus.INTERNAL_SERVER_ERROR, response.getStatusCode()); + assertEquals(HttpStatus.BAD_REQUEST, response.getStatusCode()); } // ========== EXCEPTION COVERAGE TESTS ========== diff --git a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/AttachmentSpecTest.java b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/AttachmentSpecTest.java index 044bfc1555..cc0f6cd827 100644 --- a/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/AttachmentSpecTest.java +++ b/rest/resource-server/src/test/java/org/eclipse/sw360/rest/resourceserver/restdocs/AttachmentSpecTest.java @@ -184,4 +184,12 @@ public void should_document_create_attachment() throws Exception { this.mockMvc.perform(builder).andExpect(status().isOk()).andDo(this.documentationHandler.document()); } + @Test + public void should_return_400_when_sha1_parameter_is_missing() throws Exception { + mockMvc.perform(get("/api/attachments") + .header("Authorization", TestHelper.generateAuthHeader(testUserId, testUserPassword)) + .accept(MediaTypes.HAL_JSON)) + .andExpect(status().isBadRequest()); + } + }