fix(websocket): add heartbeat scheduling and transport limits#313
Conversation
📝 WalkthroughWalkthroughWebSocket configuration now includes heartbeat messaging and transport size/time constraints via a dedicated task scheduler bean. The authentication interceptor adds multi-command STOMP handling with specific logic for CONNECT authentication, SUBSCRIBE destination validation, and unconditional SEND rejection. Changes
Sequence DiagramsequenceDiagram
participant Client as WebSocket Client
participant Interceptor as WebSocketAuthInterceptor
participant AuthService as Authentication Service
participant Broker as Message Broker
Client->>Interceptor: CONNECT (Bearer token)
activate Interceptor
Interceptor->>AuthService: Validate token & extract claims
AuthService-->>Interceptor: UsernamePasswordAuthenticationToken
Interceptor->>Broker: Allow connection
deactivate Interceptor
Client->>Interceptor: SUBSCRIBE /topic/books
activate Interceptor
Interceptor->>Interceptor: Check destination path<br/>(no .., allowed prefix?)
Interceptor->>Broker: Route subscription
deactivate Interceptor
Client->>Interceptor: SEND /app/message
activate Interceptor
Interceptor->>Interceptor: Reject SEND operation
Interceptor-->>Client: MessagingException
deactivate Interceptor
Client->>Interceptor: DISCONNECT
activate Interceptor
Interceptor->>Interceptor: Log disconnection
deactivate Interceptor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
booklore-api/src/main/java/org/booklore/config/security/interceptor/WebSocketAuthInterceptor.java (1)
87-90: Confirm/appdestination prefix is intentionally unused.With all SEND frames rejected, the
/appapplication destination prefix configured inWebSocketConfig.java(line 37) becomes unreachable—clients cannot invoke@MessageMappinghandlers. Per the PR description this is intentional ("server-push only"), but consider removing the/appprefix configuration to avoid confusion.If
/appis truly unused, clean up the broker config:// In WebSocketConfig.configureMessageBroker() // registry.setApplicationDestinationPrefixes("/app"); // Remove if not needed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@booklore-api/src/main/java/org/booklore/config/security/interceptor/WebSocketAuthInterceptor.java` around lines 87 - 90, The interceptor method rejectSend currently rejects all SEND frames, making the configured application destination prefix (setApplicationDestinationPrefixes("/app") in WebSocketConfig.configureMessageBroker) unreachable; if server-push only behavior is intended, remove the unused application destination prefix from WebSocketConfig.configureMessageBroker (i.e. delete or comment out the setApplicationDestinationPrefixes call) and add a short comment in WebSocketConfig noting that SEND frames are blocked by WebSocketAuthInterceptor.rejectSend and that /app is intentionally disabled to avoid confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@booklore-api/src/main/java/org/booklore/config/WebSocketConfig.java`:
- Around line 41-48: There are two unqualified TaskScheduler beans
(taskScheduler() in TaskExecutorConfig and wsHeartbeatScheduler() in
WebSocketConfig) causing NoUniqueBeanDefinitionException when TaskService
autowires TaskScheduler; fix by either removing the `@Bean` from
wsHeartbeatScheduler() so it becomes a private/internal scheduler used only
inside WebSocketConfig, or explicitly qualify one of them—e.g., add
`@Qualifier`("wsHeartbeatScheduler") to the wsHeartbeatScheduler() bean and update
any injection points that should use it, or mark taskScheduler() as `@Primary`;
alternatively inject the scheduler into WebSocketConfig via constructor and stop
exposing wsHeartbeatScheduler() as a bean.
---
Nitpick comments:
In
`@booklore-api/src/main/java/org/booklore/config/security/interceptor/WebSocketAuthInterceptor.java`:
- Around line 87-90: The interceptor method rejectSend currently rejects all
SEND frames, making the configured application destination prefix
(setApplicationDestinationPrefixes("/app") in
WebSocketConfig.configureMessageBroker) unreachable; if server-push only
behavior is intended, remove the unused application destination prefix from
WebSocketConfig.configureMessageBroker (i.e. delete or comment out the
setApplicationDestinationPrefixes call) and add a short comment in
WebSocketConfig noting that SEND frames are blocked by
WebSocketAuthInterceptor.rejectSend and that /app is intentionally disabled to
avoid confusion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0c0e3c58-0176-4365-b975-91e614879490
📒 Files selected for processing (2)
booklore-api/src/main/java/org/booklore/config/WebSocketConfig.javabooklore-api/src/main/java/org/booklore/config/security/interceptor/WebSocketAuthInterceptor.java
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Packaging Smoke Test
🧰 Additional context used
📓 Path-based instructions (1)
booklore-api/src/**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
booklore-api/src/**/*.java: Use 4-space indentation in Java files and match surrounding Java style
Prefer constructor injection via Lombok patterns already used in the codebase. Do not introduce@Autowiredfield injection
Use MapStruct for entity/DTO mapping
Files:
booklore-api/src/main/java/org/booklore/config/WebSocketConfig.javabooklore-api/src/main/java/org/booklore/config/security/interceptor/WebSocketAuthInterceptor.java
🔇 Additional comments (5)
booklore-api/src/main/java/org/booklore/config/WebSocketConfig.java (1)
69-75: Transport limits configuration looks good.The limits are well-chosen for mitigating resource exhaustion:
- 15s send time limit prevents slow clients from holding resources
- 512KB send buffer and 128KB message size are reasonable caps
- 30s time-to-first-message helps detect abandoned connections
booklore-api/src/main/java/org/booklore/config/security/interceptor/WebSocketAuthInterceptor.java (4)
36-48: Switch-based command handling is clean and readable.Good use of the modern switch expression syntax for routing STOMP commands. The explicit handling of CONNECT, SUBSCRIBE, SEND, and DISCONNECT with a default pass-through is well-structured.
51-70: CONNECT authentication looks solid.The method correctly validates the Authorization header format, extracts the Bearer token, and delegates to
authenticateToken. UsingMessagingExceptionensures proper STOMP ERROR frame propagation to the client.
72-85: Subscription validation is secure.The whitelist approach with
ALLOWED_DEST_PREFIXESis a good security practice. The..check provides basic path traversal protection. The combination ofrequireAuthenticatedand destination validation ensures only authenticated users can subscribe to permitted destinations.
98-113: Token authentication improvements are good.Using
isBlank()for validation,List.of()for empty authorities (instead of null), and debug-level logging for failures are all solid improvements.
1 participant
Description
Linked Issue: Fixes #
Changes
Summary by CodeRabbit