Add comprehensive input validation and error handling across all API controllers#528
Add comprehensive input validation and error handling across all API controllers#528devin-ai-integration[bot] wants to merge 11 commits intomasterfrom
Conversation
- NewArticleParam: @SiZe on title(255), description(255), body(65535) - UpdateArticleParam: @SiZe on title(255), description(255), body(65535) - RegisterParam: @SiZe on email(1-255), username(1-20), password(8-72) - UpdateUserParam: @SiZe on email(255), username(20), password(72), bio(65535), image(512) - LoginParam: @SiZe on email(255), password(72) - NewCommentParam: @SiZe on body(65535) Co-Authored-By: Toby Drinkall <toby.drinkall@cognition.ai>
Replace unsafe authorization.split(" ")[1] calls with a safe
extractToken() helper that validates the header starts with "Token "
before extracting the token value. Throws InvalidAuthenticationException
for malformed Authorization headers instead of ArrayIndexOutOfBoundsException.
Co-Authored-By: Toby Drinkall <toby.drinkall@cognition.ai>
- Add @validated class-level annotation to all 7 API controllers - Add @notblank to path variables in ArticleApi, ArticleFavoriteApi, CommentsApi, ProfileApi - Add @Min/@max to offset/limit query params in ArticlesApi - Run spotlessJavaApply for Google Java Format compliance Co-Authored-By: Toby Drinkall <toby.drinkall@cognition.ai>
- Add ResourceNotFoundException handler (404 with structured error body) - Add NoAuthorizationException handler (403 with structured error body) - Override handleHttpMessageNotReadable for malformed JSON (422) - Add generic Exception fallback handler (500) - Remove @ResponseStatus annotations from ResourceNotFoundException and NoAuthorizationException - Add HttpMessageNotReadableException import Co-Authored-By: Toby Drinkall <toby.drinkall@cognition.ai>
When @validated is on the controller class, constraint violation property paths have 2 segments (e.g., article.slug) instead of 3. The previous code used Arrays.copyOfRange(splits, 2, length) which returned empty string for 2-segment paths. Now returns the last segment for paths with 1 or 2 segments. Co-Authored-By: Toby Drinkall <toby.drinkall@cognition.ai>
…ollers' into devin/1775708081-validation-tests
…er' into devin/1775708081-validation-tests
…r' into devin/1775708081-validation-tests
- ArticlesApiTest: negative offset, excessive limit, zero limit return 422 - CommentsApiTest: overly long comment body (>65535 chars) returns 422 - UsersApiTest: short password (<8 chars) and malformed JSON return 422 - CurrentUserApiTest: malformed Authorization header returns 422 - Apply Spotless formatting to DefaultJwtServiceTest - Add gradle.properties with JDK 17 JVM args for Spotless compatibility Co-Authored-By: Toby Drinkall <toby.drinkall@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
… error message Co-Authored-By: Toby Drinkall <toby.drinkall@cognition.ai>
Co-Authored-By: Toby Drinkall <toby.drinkall@cognition.ai>
End-to-End Test ResultsRan Spring Boot app locally ( Session: https://app.devin.ai/sessions/96594bc364384ae292afa201e2f9d351 Validation & Error Handling Tests (12/12 passed)
Key EvidencePagination validation ( Password size constraint ( Auth header scheme validation ( 404 structured response: Note on Auth Header TestsTests 6 & 7 return 401 from Spring Security's |
Summary
Comprehensive input validation and structured error handling added across all REST API controllers. This PR integrates work from 5 parallel child sessions:
@Sizeconstraints on all DTO/Param classes (NewArticleParam,UpdateArticleParam,RegisterParam,UpdateUserParam,LoginParam,NewCommentParam) to enforce field length limits@Validated+ param validation on all 7 controllers —@NotBlankon path variables,@Min/@Maxon pagination query params (offset >= 0,1 <= limit <= 100)CustomizeExceptionHandlerwith explicit handlers forResourceNotFoundException(404),NoAuthorizationException(403), malformed JSON (422), and a genericExceptionfallback (500 with logging) — all returning structured{"errors": {...}}bodies@ResponseStatusfromResourceNotFoundExceptionandNoAuthorizationException(status now controlled by the handler)authorization.split(" ")[1]inCurrentUserApi— replaced withextractToken()that validates the"Token "prefix and throwsInvalidAuthenticationException("Authorization header must use Token scheme")on malformed headersgradle.propertiesadded with JDK 17--add-exportsargs for Spotless compatibilityUpdates since last revision
Addressed Devin Review feedback:
logger.error("Unhandled exception", ex)to the catch-allExceptionhandler so unhandled errors are logged with stack traces before returning 500String messageconstructor toInvalidAuthenticationExceptionand updatedextractTokento throw with"Authorization header must use Token scheme"instead of the misleading"invalid email or password"Review & Testing Checklist for Human
Exceptioncatch-all handler (CustomizeExceptionHandler.java:151): This catches all unhandled exceptions, which may intercept Spring's built-in handling forHttpRequestMethodNotSupportedException(405),MissingServletRequestParameterException(400), etc. Verify these still return correct status codes, or consider narrowing the catch-all.getParamhelper change (CustomizeExceptionHandler.java:165): Logic changed fromsplits.length == 1tosplits.length <= 2. Verify existingConstraintViolationExceptionerror responses still format field names correctly — this affects how@Validatedparameter violations display in theerrorsbody.should_show_error_message_for_blank_usernameasserts onerrors.username[0], but@NotBlankand@Size(min=1)now both fire on empty input — error ordering is not guaranteed. This test passed locally but may be flaky.@NotBlankon path variables (e.g.,@NotBlank @PathVariable("slug") String slug): For controllers with path-based routing like/articles/{slug}, a blank slug means a different URL entirely. Verify this validation actually triggers in practice vs. Spring returning 404 before it's reached.Suggested Test Plan
./gradlew clean test— all 75 tests should pass./gradlew bootRun) and manually test:GET /articles?offset=-1→ 422GET /articles?limit=0→ 422POST /userswith{"user":{"email":"a@b.com","username":"x","password":"short"}}→ 422POST /userswith malformed JSON body → 422GET /userwithAuthorization: BadHeader→ 422 with"Authorization header must use Token scheme"GET /articles/nonexistent-slug→ 404 with{"errors":{"resource":["not found"]}}DELETE /articles/{slug}without auth → 403 with structured bodyPOST /invalid-endpoint→ verify it still returns 405 (not 500 from the catch-all)Notes
DefaultJwtServiceTest.javadiff is a Spotless auto-format change only (line wrapping), no logic change.Link to Devin session: https://app.devin.ai/sessions/96594bc364384ae292afa201e2f9d351
Requested by: @tobydrinkall