Skip to content

Commit 45f53e6

Browse files
vfedorivvfedoriv
authored
Merge pull request #306 from San-43/fix-305-validation-error-details
Fix #305: Sanitize error details and add validation tests for owner pets
2 parents c04a4a5 + 9d3c02b commit 45f53e6

File tree

2 files changed

+165
-25
lines changed

2 files changed

+165
-25
lines changed

‎src/main/java/org/springframework/samples/petclinic/rest/advice/ExceptionControllerAdvice.java‎

Lines changed: 56 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -16,21 +16,26 @@
1616

1717
package org.springframework.samples.petclinic.rest.advice;
1818

19+
import java.net.URI;
20+
import java.time.Instant;
21+
import java.util.List;
22+
import java.util.Objects;
23+
1924
import jakarta.servlet.http.HttpServletRequest;
25+
import org.slf4j.Logger;
26+
import org.slf4j.LoggerFactory;
2027
import org.springframework.dao.DataIntegrityViolationException;
2128
import org.springframework.http.HttpStatus;
2229
import org.springframework.http.ProblemDetail;
2330
import org.springframework.http.ResponseEntity;
2431
import org.springframework.samples.petclinic.rest.controller.BindingErrorsResponse;
32+
import org.springframework.samples.petclinic.rest.dto.ValidationMessageDto;
2533
import org.springframework.validation.BindingResult;
2634
import org.springframework.web.bind.MethodArgumentNotValidException;
2735
import org.springframework.web.bind.annotation.ControllerAdvice;
2836
import org.springframework.web.bind.annotation.ExceptionHandler;
2937
import org.springframework.web.bind.annotation.ResponseBody;
3038

31-
import java.net.URI;
32-
import java.time.Instant;
33-
3439
/**
3540
* Global Exception handler for REST controllers.
3641
* <p>
@@ -42,21 +47,27 @@
4247
@ControllerAdvice
4348
public class ExceptionControllerAdvice {
4449

50+
private static final Logger logger = LoggerFactory.getLogger(ExceptionControllerAdvice.class);
51+
private static final String ERROR_UNEXPECTED = "An unexpected error occurred while processing your request";
52+
private static final String ERROR_DATA_INTEGRITY = "The requested resource could not be processed due to a data constraint violation";
53+
private static final String ERROR_INVALID_REQUEST = "The request contains invalid or missing parameters";
54+
4555
/**
4656
* Private method for constructing the {@link ProblemDetail} object passing the name and details of the exception
4757
* class.
4858
*
49-
* @param ex Object referring to the thrown exception.
59+
* @param e Object referring to the thrown exception.
5060
* @param status HTTP response status.
5161
* @param url URL request.
5262
*/
53-
private ProblemDetail detailBuild(Exception ex, HttpStatus status, StringBuffer url) {
54-
ProblemDetail detail = ProblemDetail.forStatus(status);
55-
detail.setType(URI.create(url.toString()));
56-
detail.setTitle(ex.getClass().getSimpleName());
57-
detail.setDetail(ex.getLocalizedMessage());
58-
detail.setProperty("timestamp", Instant.now());
59-
return detail;
63+
private ProblemDetail detailBuild(Exception e, HttpStatus status, StringBuffer url, String detail) {
64+
ProblemDetail problemDetail = ProblemDetail.forStatus(status);
65+
problemDetail.setType(URI.create(url.toString()));
66+
problemDetail.setTitle(e.getClass().getSimpleName());
67+
problemDetail.setDetail(detail);
68+
problemDetail.setProperty("timestamp", Instant.now());
69+
problemDetail.setProperty("schemaValidationErrors", List.<ValidationMessageDto>of());
70+
return problemDetail;
6071
}
6172

6273
/**
@@ -69,50 +80,71 @@ private ProblemDetail detailBuild(Exception ex, HttpStatus status, StringBuffer
6980
@ExceptionHandler(Exception.class)
7081
@ResponseBody
7182
public ResponseEntity<ProblemDetail> handleGeneralException(Exception e, HttpServletRequest request) {
83+
logger.error("Unexpected error at {} {}", request.getMethod(), request.getRequestURI(), e);
7284
HttpStatus status = HttpStatus.INTERNAL_SERVER_ERROR;
73-
ProblemDetail detail = this.detailBuild(e, status, request.getRequestURL());
85+
ProblemDetail detail = this.detailBuild(e, status, request.getRequestURL(), ERROR_UNEXPECTED);
7486
return ResponseEntity.status(status).body(detail);
7587
}
7688

7789
/**
7890
* Handles {@link DataIntegrityViolationException} which typically indicates database constraint violations. This
7991
* method returns a 404 Not Found status if an entity does not exist.
8092
*
81-
* @param ex The {@link DataIntegrityViolationException} to be handled
93+
* @param e The {@link DataIntegrityViolationException} to be handled
8294
* @param request {@link HttpServletRequest} object referring to the current request.
8395
* @return A {@link ResponseEntity} containing the error information and a 404 Not Found status
8496
*/
8597
@ExceptionHandler(DataIntegrityViolationException.class)
8698
@ResponseBody
87-
public ResponseEntity<ProblemDetail> handleDataIntegrityViolationException(DataIntegrityViolationException ex, HttpServletRequest request) {
99+
public ResponseEntity<ProblemDetail> handleDataIntegrityViolationException(DataIntegrityViolationException e, HttpServletRequest request) {
100+
logger.warn("Data integrity violation at {} {}: {}",
101+
request.getMethod(),
102+
request.getRequestURI(),
103+
e.getMessage());
104+
logger.debug("Data integrity violation stacktrace", e);
88105
HttpStatus status = HttpStatus.NOT_FOUND;
89-
ProblemDetail detail = ProblemDetail.forStatus(status);
90-
detail.setType(URI.create(request.getRequestURL().toString()));
91-
detail.setTitle(ex.getClass().getSimpleName());
92-
detail.setDetail("Request could not be processed");
93-
detail.setProperty("timestamp", Instant.now());
106+
ProblemDetail detail = this.detailBuild(e, status, request.getRequestURL(), ERROR_DATA_INTEGRITY);
94107
return ResponseEntity.status(status).body(detail);
95108
}
96109

97110
/**
98111
* Handles exception thrown by Bean Validation on controller methods parameters
99112
*
100-
* @param ex The {@link MethodArgumentNotValidException} to be handled
113+
* @param e The {@link MethodArgumentNotValidException} to be handled
101114
* @param request {@link HttpServletRequest} object referring to the current request.
102115
* @return A {@link ResponseEntity} containing the error information and a 400 Bad Request status.
103116
*/
104117
@ExceptionHandler(MethodArgumentNotValidException.class)
105118
@ResponseBody
106-
public ResponseEntity<ProblemDetail> handleMethodArgumentNotValidException(MethodArgumentNotValidException ex, HttpServletRequest request) {
119+
public ResponseEntity<ProblemDetail> handleMethodArgumentNotValidException(MethodArgumentNotValidException e, HttpServletRequest request) {
107120
HttpStatus status = HttpStatus.BAD_REQUEST;
108121
BindingErrorsResponse errors = new BindingErrorsResponse();
109-
BindingResult bindingResult = ex.getBindingResult();
122+
BindingResult bindingResult = e.getBindingResult();
123+
ProblemDetail detail = this.detailBuild(e, status, request.getRequestURL(), ERROR_INVALID_REQUEST);
110124
if (bindingResult.hasErrors()) {
111125
errors.addAllErrors(bindingResult);
112-
ProblemDetail detail = this.detailBuild(ex, status, request.getRequestURL());
126+
List<ValidationMessageDto> schemaValidationErrors = bindingResult.getFieldErrors().stream()
127+
.map(fieldError -> {
128+
String rejectedValue = Objects.toString(fieldError.getRejectedValue(), "null");
129+
String defaultMessage = Objects.toString(fieldError.getDefaultMessage(), "Validation failed");
130+
String message = "Field '%s' %s (rejected value: %s)".formatted(
131+
fieldError.getField(),
132+
defaultMessage,
133+
rejectedValue);
134+
return new ValidationMessageDto(message)
135+
.putAdditionalProperty("field", fieldError.getField())
136+
.putAdditionalProperty("rejectedValue", rejectedValue)
137+
.putAdditionalProperty("defaultMessage", defaultMessage);
138+
})
139+
.toList();
140+
logger.debug("Validation error at {} {}: {}",
141+
request.getMethod(),
142+
request.getRequestURI(),
143+
bindingResult.getFieldErrors());
144+
detail.setProperty("schemaValidationErrors", schemaValidationErrors);
113145
return ResponseEntity.status(status).body(detail);
114146
}
115-
return ResponseEntity.status(status).build();
147+
return ResponseEntity.status(status).body(detail);
116148
}
117149

118150
}

‎src/test/java/org/springframework/samples/petclinic/rest/controller/OwnerRestControllerTests.java‎

Lines changed: 109 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ void testCreatePetShouldNotExposeTechnicalDetails() throws Exception {
389389
.content(newPetAsJSON).accept(MediaType.APPLICATION_JSON_VALUE).contentType(MediaType.APPLICATION_JSON_VALUE))
390390
.andDo(MockMvcResultHandlers.print())
391391
.andExpect(status().isNotFound())
392-
.andExpect(jsonPath("$.detail").value("Request could not be processed"));
392+
.andExpect(jsonPath("$.detail").value("The requested resource could not be processed due to a data constraint violation"));
393393
}
394394

395395
@Test
@@ -407,6 +407,114 @@ void testCreatePetWithUnknownOwnerShouldReturnNotFound() throws Exception {
407407
.andExpect(status().isNotFound());
408408
}
409409

410+
@Test
411+
@WithMockUser(roles = "OWNER_ADMIN")
412+
void testCreatePetWithNullTypeShouldReturnBadRequestWithGenericDetail() throws Exception {
413+
PetDto newPet = pets.get(0);
414+
newPet.setId(null);
415+
newPet.setType(null);
416+
ObjectMapper mapper = JsonMapper.builder()
417+
.defaultDateFormat(new SimpleDateFormat("dd/MM/yyyy"))
418+
.build();
419+
String newPetAsJSON = mapper.writeValueAsString(newPet);
420+
this.mockMvc.perform(post("/api/owners/1/pets")
421+
.content(newPetAsJSON).accept(MediaType.APPLICATION_JSON_VALUE).contentType(MediaType.APPLICATION_JSON_VALUE))
422+
.andDo(MockMvcResultHandlers.print())
423+
.andExpect(status().isBadRequest())
424+
.andExpect(jsonPath("$.detail").value("The request contains invalid or missing parameters"))
425+
.andExpect(jsonPath("$.title").value("MethodArgumentNotValidException"))
426+
.andExpect(jsonPath("$.schemaValidationErrors").isArray())
427+
.andExpect(jsonPath("$.schemaValidationErrors[0].message").exists())
428+
.andExpect(jsonPath("$.schemaValidationErrors[0].field").exists())
429+
.andExpect(jsonPath("$.schemaValidationErrors[0].rejectedValue").exists())
430+
.andExpect(jsonPath("$.schemaValidationErrors[0].defaultMessage").exists());
431+
}
432+
433+
@Test
434+
@WithMockUser(roles = "OWNER_ADMIN")
435+
void testCreatePetWithEmptyTypeNameShouldReturnBadRequestWithGenericDetail() throws Exception {
436+
PetDto newPet = pets.get(0);
437+
newPet.setId(null);
438+
newPet.getType().setName("");
439+
ObjectMapper mapper = JsonMapper.builder()
440+
.defaultDateFormat(new SimpleDateFormat("dd/MM/yyyy"))
441+
.build();
442+
String newPetAsJSON = mapper.writeValueAsString(newPet);
443+
this.mockMvc.perform(post("/api/owners/1/pets")
444+
.content(newPetAsJSON).accept(MediaType.APPLICATION_JSON_VALUE).contentType(MediaType.APPLICATION_JSON_VALUE))
445+
.andDo(MockMvcResultHandlers.print())
446+
.andExpect(status().isBadRequest())
447+
.andExpect(jsonPath("$.detail").value("The request contains invalid or missing parameters"))
448+
.andExpect(jsonPath("$.title").value("MethodArgumentNotValidException"))
449+
.andExpect(jsonPath("$.schemaValidationErrors").isArray())
450+
.andExpect(jsonPath("$.schemaValidationErrors[0].message").exists())
451+
.andExpect(jsonPath("$.schemaValidationErrors[0].field").exists())
452+
.andExpect(jsonPath("$.schemaValidationErrors[0].rejectedValue").exists())
453+
.andExpect(jsonPath("$.schemaValidationErrors[0].defaultMessage").exists());
454+
}
455+
456+
@Test
457+
@WithMockUser(roles = "OWNER_ADMIN")
458+
void testCreatePetWithNullTypeIdShouldReturnBadRequestWithGenericDetail() throws Exception {
459+
PetDto newPet = pets.get(0);
460+
newPet.setId(null);
461+
newPet.getType().setId(null);
462+
ObjectMapper mapper = JsonMapper.builder()
463+
.defaultDateFormat(new SimpleDateFormat("dd/MM/yyyy"))
464+
.build();
465+
String newPetAsJSON = mapper.writeValueAsString(newPet);
466+
this.mockMvc.perform(post("/api/owners/1/pets")
467+
.content(newPetAsJSON).accept(MediaType.APPLICATION_JSON_VALUE).contentType(MediaType.APPLICATION_JSON_VALUE))
468+
.andDo(MockMvcResultHandlers.print())
469+
.andExpect(status().isBadRequest())
470+
.andExpect(jsonPath("$.detail").value("The request contains invalid or missing parameters"))
471+
.andExpect(jsonPath("$.title").value("MethodArgumentNotValidException"))
472+
.andExpect(jsonPath("$.schemaValidationErrors").isArray())
473+
.andExpect(jsonPath("$.schemaValidationErrors[0].message").exists())
474+
.andExpect(jsonPath("$.schemaValidationErrors[0].field").exists())
475+
.andExpect(jsonPath("$.schemaValidationErrors[0].rejectedValue").exists())
476+
.andExpect(jsonPath("$.schemaValidationErrors[0].defaultMessage").exists());
477+
}
478+
479+
@Test
480+
@WithMockUser(roles = "OWNER_ADMIN")
481+
void testCreatePetWithUnexpectedErrorShouldReturnInternalServerErrorWithGenericDetail() throws Exception {
482+
PetDto newPet = pets.get(0);
483+
newPet.setId(null);
484+
ObjectMapper mapper = JsonMapper.builder()
485+
.defaultDateFormat(new SimpleDateFormat("dd/MM/yyyy"))
486+
.build();
487+
String newPetAsJSON = mapper.writeValueAsString(newPet);
488+
given(this.clinicService.findOwnerById(1)).willReturn(ownerMapper.toOwner(owners.get(0)));
489+
doThrow(new IllegalStateException("JDBC timeout while executing insert into pets"))
490+
.when(this.clinicService).savePet(any());
491+
this.mockMvc.perform(post("/api/owners/1/pets")
492+
.content(newPetAsJSON).accept(MediaType.APPLICATION_JSON_VALUE).contentType(MediaType.APPLICATION_JSON_VALUE))
493+
.andDo(MockMvcResultHandlers.print())
494+
.andExpect(status().isInternalServerError())
495+
.andExpect(jsonPath("$.detail").value("An unexpected error occurred while processing your request"))
496+
.andExpect(jsonPath("$.title").value("IllegalStateException"))
497+
.andExpect(jsonPath("$.timestamp").exists());
498+
}
499+
500+
@Test
501+
@WithMockUser(roles = "OWNER_ADMIN")
502+
void testCreateOwnerWithUnexpectedErrorShouldReturnInternalServerErrorWithGenericDetail() throws Exception {
503+
OwnerDto newOwnerDto = owners.get(0);
504+
newOwnerDto.setId(null);
505+
ObjectMapper mapper = new ObjectMapper();
506+
String newOwnerAsJSON = mapper.writeValueAsString(newOwnerDto);
507+
doThrow(new RuntimeException("Low-level persistence exception details"))
508+
.when(this.clinicService).saveOwner(any());
509+
this.mockMvc.perform(post("/api/owners")
510+
.content(newOwnerAsJSON).accept(MediaType.APPLICATION_JSON_VALUE).contentType(MediaType.APPLICATION_JSON_VALUE))
511+
.andDo(MockMvcResultHandlers.print())
512+
.andExpect(status().isInternalServerError())
513+
.andExpect(jsonPath("$.detail").value("An unexpected error occurred while processing your request"))
514+
.andExpect(jsonPath("$.title").value("RuntimeException"))
515+
.andExpect(jsonPath("$.timestamp").exists());
516+
}
517+
410518
@Test
411519
@WithMockUser(roles = "OWNER_ADMIN")
412520
void testCreateVisitSuccess() throws Exception {

0 commit comments

Comments
 (0)
⚡