From da1ab25d69284c5ca00e3e1f700f90125fa3f528 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Fri, 6 Nov 2026 00:32:56 +0800 Subject: [PATCH] fix(notification): return client errors for invalid delivery pagination Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .trellis/spec/backend/error-handling.md | 2 +- docs/notification-email.md | 2 ++ ...ificationEmailDeliveryAdminController.java | 2 -- .../NotificationEmailDeliveryController.java | 2 -- .../exception/GlobalExceptionHandler.java | 8 +++++ ...NotificationEmailDeliverySecurityTest.java | 30 +++++++++++++++++++ .../exception/GlobalExceptionHandlerTest.java | 13 ++++++++ 7 files changed, 54 insertions(+), 5 deletions(-) diff --git a/.trellis/spec/backend/error-handling.md b/.trellis/spec/backend/error-handling.md index 10e6fe55d..a6bd66e52 100644 --- a/.trellis/spec/backend/error-handling.md +++ b/.trellis/spec/backend/error-handling.md @@ -25,7 +25,7 @@ |---|---|---| | `ObjectOptimisticLockingFailureException` | HTTP 409、响应 `code=409`、本地化冲突提示 | `persistenceConflictsReturn409WithoutLeakingDatabaseDetails` | | `StorageAccessException` | HTTP 503；日志带 requestId 和认证状态，不输出稳定 userId | `handleStorageAccess_shouldLogAuthenticationWithoutStableUserId` | -| MVC 缺参、不可读请求体、类型不匹配 | HTTP 400、ApiResponse `code=400`；不记作未处理内部故障 | `handleMvcBadRequest_shouldReturn400WithoutUnhandledErrorLog` | +| MVC 缺参、不可读请求体、类型不匹配、原生方法入参约束 | HTTP 400、ApiResponse `code=400`；不记作未处理内部故障 | `handleMvcBadRequest_shouldReturn400WithoutUnhandledErrorLog`、`handleMethodValidationDistinguishesInputFromReturnValueErrors` | | 不支持的 HTTP 方法 | HTTP 405、ApiResponse `code=405`，保留 Allow 响应头 | `handleMethodNotAllowed_shouldReturn405AndAllowHeader` | 反例：用 `exception.getMessage().contains(...)` 为新业务规则分配状态码；只断言状态而遗漏协议头； diff --git a/docs/notification-email.md b/docs/notification-email.md index 16700be2a..2ff76cfa1 100644 --- a/docs/notification-email.md +++ b/docs/notification-email.md @@ -25,6 +25,8 @@ SMTP 连接、TLS 和企业邮箱接入方式见 [SMTP 配置指南](19-smtp-pas 每封摘要最多展开 50 条，超出的通知通过邮件中的 **批次详情** 入口分页查看；不会为分页再拆发多封。详情路径为 `/dashboard/notifications/email/{id}`，要求登录为对应收件账号，并逐项检查当前权限。退出空间、失去权限或资源删除后，相关项不再展示。保留期结束、批次不存在或不属于当前账号时，页面会提示不可用。此明细不依赖站内通知是否开启、是否已读或已删除。明细采用游标逐页读取原始记录，再过滤当前权限；本页可见项为空但仍有下一游标时，可以继续查看后页。“原始记录数”不是当前可查看条数，“不可访问”数量仅对应本页。已合并的旧批次会提供目标摘要入口。 +详情接口的 `size` 范围为 1–100，管理员投递列表的 `page` 范围为 0–1000000；非法分页参数或非数字游标返回 HTTP 400，不调用查询服务，也不在错误响应中回显原始游标。服务端返回值校验失败仍按 HTTP 500 处理。 + ## 管理员如何开启 1. 在部署侧配置 SMTP、独立通知发件人、有限网络超时和真实公开站点根地址。不要把凭证提交进 Git，也不要在后台填 SMTP 密钥。 diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/NotificationEmailDeliveryAdminController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/NotificationEmailDeliveryAdminController.java index bcc72a40b..03eaefab9 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/NotificationEmailDeliveryAdminController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/admin/NotificationEmailDeliveryAdminController.java @@ -7,10 +7,8 @@ import com.iflytek.skillhub.service.notificationemail.NotificationEmailDeliveryQ import jakarta.validation.constraints.*; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.core.annotation.AuthenticationPrincipal; -import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.*; @RestController -@Validated @PreAuthorize("hasRole('SUPER_ADMIN')") @RequestMapping({"/api/v1/admin/notification-email-deliveries", "/api/web/admin/notification-email-deliveries"}) public class NotificationEmailDeliveryAdminController extends BaseApiController { diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NotificationEmailDeliveryController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NotificationEmailDeliveryController.java index 6caccbd86..9c854ca52 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NotificationEmailDeliveryController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/NotificationEmailDeliveryController.java @@ -4,10 +4,8 @@ import com.iflytek.skillhub.controller.BaseApiController; import com.iflytek.skillhub.dto.*; import com.iflytek.skillhub.service.notificationemail.NotificationEmailDeliveryQueryService; import jakarta.validation.constraints.*; -import org.springframework.validation.annotation.Validated; import org.springframework.web.bind.annotation.*; @RestController -@Validated @RequestMapping({"/api/v1/notification-email-deliveries", "/api/web/notification-email-deliveries"}) public class NotificationEmailDeliveryController extends BaseApiController { private final NotificationEmailDeliveryQueryService service; diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java index e12c1cabc..4bc64a2a6 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/exception/GlobalExceptionHandler.java @@ -29,6 +29,7 @@ import org.springframework.web.bind.MethodArgumentNotValidException; import org.springframework.web.bind.MissingRequestHeaderException; import org.springframework.web.bind.MissingServletRequestParameterException; import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.method.annotation.HandlerMethodValidationException; import org.springframework.web.bind.annotation.RestControllerAdvice; import org.springframework.web.context.request.async.AsyncRequestTimeoutException; import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; @@ -119,6 +120,13 @@ public class GlobalExceptionHandler { apiResponseFactory.error(400, "error.badRequest")); } + @ExceptionHandler(HandlerMethodValidationException.class) + public ResponseEntity> handleMethodValidation( + HandlerMethodValidationException ex, HttpServletRequest request) { + // Invalid controller results are server faults, not client input errors. + return ex.isForReturnValue() ? handleGlobalException(ex, request) : handleMvcBadRequest(ex, request); + } + @ExceptionHandler(HttpRequestMethodNotSupportedException.class) public ResponseEntity> handleMethodNotAllowed( HttpRequestMethodNotSupportedException ex, diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NotificationEmailDeliverySecurityTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NotificationEmailDeliverySecurityTest.java index 3b7d5fb3f..0d704e30e 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NotificationEmailDeliverySecurityTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/NotificationEmailDeliverySecurityTest.java @@ -66,6 +66,36 @@ class NotificationEmailDeliverySecurityTest { .contentType(MediaType.APPLICATION_JSON).content("{\"confirmDuplicateRisk\":true}")).andExpect(status().isUnauthorized()); verify(service,never()).retry(eq(2L),any(),anyBoolean()); } + @Test void invalidDeliveryPaginationReturnsClientErrorsWithoutCallingTheService() throws Exception { + for (int size : new int[]{0, 101}) { + mvc.perform(get("/api/web/notification-email-deliveries/1").param("size", String.valueOf(size)) + .with(authentication(auth("ordinary", "USER")))) + .andExpect(status().isBadRequest()).andExpect(jsonPath("$.code").value(400)); + mvc.perform(get("/api/web/admin/notification-email-deliveries").param("size", String.valueOf(size)) + .with(authentication(auth("admin", "SUPER_ADMIN")))) + .andExpect(status().isBadRequest()).andExpect(jsonPath("$.code").value(400)); + } + for (int page : new int[]{-1, 1000001}) { + mvc.perform(get("/api/web/admin/notification-email-deliveries").param("page", String.valueOf(page)) + .with(authentication(auth("admin", "SUPER_ADMIN")))) + .andExpect(status().isBadRequest()); + } + mvc.perform(get("/api/web/notification-email-deliveries/1").param("cursor", "private-invalid-cursor") + .with(authentication(auth("ordinary", "USER")))) + .andExpect(status().isBadRequest()) + .andExpect(content().string(org.hamcrest.Matchers.not(org.hamcrest.Matchers.containsString("private-invalid-cursor")))); + verifyNoInteractions(service); + } + + @Test void validPageSizeBoundariesReachTheService() throws Exception { + for (int size : new int[]{1, 100}) { + mvc.perform(get("/api/web/notification-email-deliveries/1").param("size", String.valueOf(size)) + .with(authentication(auth("ordinary", "USER")))) + .andExpect(status().isOk()); + verify(service).detail(1L, "ordinary", null, size); + } + } + private UsernamePasswordAuthenticationToken auth(String id,String role) { return new UsernamePasswordAuthenticationToken(new PlatformPrincipal(id,id,null,null,null,Set.of(role)),null, List.of(new SimpleGrantedAuthority("ROLE_"+role))); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java index 8816832fc..4d52cdbc2 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/exception/GlobalExceptionHandlerTest.java @@ -232,6 +232,19 @@ class GlobalExceptionHandlerTest { .isSameAs(ex); } + @Test + void handleMethodValidationDistinguishesInputFromReturnValueErrors() { + var invalidInput = org.mockito.Mockito.mock( + org.springframework.web.method.annotation.HandlerMethodValidationException.class); + when(invalidInput.isForReturnValue()).thenReturn(false); + assertThat(handler.handleMethodValidation(invalidInput, request).getBody().code()).isEqualTo(400); + + var invalidResult = org.mockito.Mockito.mock( + org.springframework.web.method.annotation.HandlerMethodValidationException.class); + when(invalidResult.isForReturnValue()).thenReturn(true); + assertThat(handler.handleMethodValidation(invalidResult, request).getBody().code()).isEqualTo(500); + } + @Test void handleMvcBadRequest_shouldReturn400WithoutUnhandledErrorLog() { attachAppender();