Skip to content

Fix #305: Sanitize error details and add validation tests for owner pets#306

Open
San-43 wants to merge 6 commits intospring-petclinic:masterfrom
San-43:fix-305-validation-error-details
Open

Fix #305: Sanitize error details and add validation tests for owner pets#306
San-43 wants to merge 6 commits intospring-petclinic:masterfrom
San-43:fix-305-validation-error-details

Conversation

@San-43
Copy link
Copy Markdown
Contributor

@San-43 San-43 commented Mar 16, 2026

This PR fixes #305.

I updated the global exception handling, so API errors are more consistent and don’t expose internal technical details.

What changed

  • The ExceptionControllerAdvice has been updated. It now provides generic, safe messages when unexpected errors occur, when requests are invalid, or when data integrity issues are detected.
  • ProblemDetail now always includes schemaValidationErrors (empty by default).
  • For validation failures (MethodArgumentNotValidException), schemaValidationErrors is populated from BindingResult.
  • Added logging so backend troubleshooting is still possible without leaking details to clients.

Why this change

OpenAPI currently defines schemaValidationErrors in ProblemDetail, so this PR aligns runtime behavior with that contract while keeping messages client-safe and non-technical.

Tests

  • Updated OwnerRestControllerTests to match the new error format.
  • Added/updated tests to verify:
    • generic error details are returned
    • technical/sensitive exception details are not exposed
    • schemaValidationErrors is present for validation errors

Feedback is welcome if you think something should be adjusted or improved.

@San-43
Copy link
Copy Markdown
Contributor Author

San-43 commented Mar 24, 2026

Hey @vfedoriv srry to bother you, any thoughts on this?

@vfedoriv
Copy link
Copy Markdown
Collaborator

@San-43 looks fine, except one detail - as I see, schemaValidationErrors contains array of ValidationMessage:

$ref: '#/components/schemas/ValidationMessage'

not a List<Map<String, String>>

@San-43 San-43 force-pushed the fix-305-validation-error-details branch from 94f4a67 to 306a2e3 Compare March 25, 2026 22:20
@San-43
Copy link
Copy Markdown
Contributor Author

San-43 commented Mar 25, 2026

Hello @vfedoriv, I think this is correct now. I have one question though: since BindingErrorsResponse is no longer used, should we mark it as @Deprecated or remove it?

@San-43 San-43 force-pushed the fix-305-validation-error-details branch from 306a2e3 to 407c039 Compare March 26, 2026 07:37
@San-43 San-43 force-pushed the fix-305-validation-error-details branch from 26e0c99 to 9d3c02b Compare March 26, 2026 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validation error response exposes internal implementation details in POST /api/owners/{ownerId}/pets

2 participants