feat: WebSocket 로깅 인터셉터 작성 #635
Open
+61
−1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
whqtker
requested review from
Gyuhyeok99,
Hexeong,
JAEHEE25,
lsy1307,
sukangpunch and
wibaek
as code owners
Walkthrough이 변경사항은 WebSocket 통신 로깅을 위한 새로운 인터셉터 기능을 추가합니다. 다음과 같이 구성됩니다:
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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
🤖 Fix all issues with AI agents
In
`@src/main/java/com/example/solidconnection/chat/config/WebSocketLoggingInterceptor.java`:
- Around line 45-52: In extractUserId, avoid potential NPE/ClassCastException by
fetching tokenAuth.getPrincipal() into a local variable, check that it is
non-null and an instance of SiteUserDetails before casting, and only then call
getSiteUser().getId(); if the principal is null or not a SiteUserDetails, return
null. Update the method (referencing extractUserId,
TokenAuthentication.getPrincipal, and SiteUserDetails) to perform these safe
null/instance checks and short-circuit to null for invalid cases.
Comment on lines
+45
to
+52
| private Long extractUserId(StompHeaderAccessor accessor) { | ||
| Principal user = accessor.getUser(); | ||
| if (user instanceof TokenAuthentication tokenAuth) { | ||
| SiteUserDetails details = (SiteUserDetails) tokenAuth.getPrincipal(); | ||
| return details.getSiteUser().getId(); | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
extractUserId 메서드에서 잠재적인 NPE 및 ClassCastException 위험이 있습니다.
TokenAuthentication.getPrincipal()은Object타입을 반환하며, 인증되지 않은 상태에서는null을 반환할 수 있습니다 (관련 코드TokenAuthentication생성자 참조).getPrincipal()이null이거나SiteUserDetails타입이 아닌 경우, 직접 캐스팅 시 예외가 발생합니다.
🐛 안전한 타입 체크를 추가한 수정 제안
private Long extractUserId(StompHeaderAccessor accessor) {
Principal user = accessor.getUser();
- if (user instanceof TokenAuthentication tokenAuth) {
- SiteUserDetails details = (SiteUserDetails) tokenAuth.getPrincipal();
- return details.getSiteUser().getId();
+ if (user instanceof TokenAuthentication tokenAuth
+ && tokenAuth.getPrincipal() instanceof SiteUserDetails details) {
+ return details.getSiteUser().getId();
}
return null;
}🤖 Prompt for AI Agents
In
`@src/main/java/com/example/solidconnection/chat/config/WebSocketLoggingInterceptor.java`
around lines 45 - 52, In extractUserId, avoid potential NPE/ClassCastException
by fetching tokenAuth.getPrincipal() into a local variable, check that it is
non-null and an instance of SiteUserDetails before casting, and only then call
getSiteUser().getId(); if the principal is null or not a SiteUserDetails, return
null. Update the method (referencing extractUserId,
TokenAuthentication.getPrincipal, and SiteUserDetails) to perform these safe
null/instance checks and short-circuit to null for invalid cases.
sukangpunch
reviewed
Feb 2, 2026
Contributor
sukangpunch
left a comment
sukangpunch
left a comment
There was a problem hiding this comment.
고생하셨습니다!! 궁금한 점 리뷰 달았습니다
| public void configureClientInboundChannel(ChannelRegistration registration) { | ||
| InboundProperties inboundProperties = stompProperties.threadPool().inbound(); | ||
| registration.interceptors(stompHandler).taskExecutor().corePoolSize(inboundProperties.corePoolSize()).maxPoolSize(inboundProperties.maxPoolSize()).queueCapacity(inboundProperties.queueCapacity()); | ||
| webSocketLoggingInterceptor.ifPresent(registration::interceptors); |
Contributor
There was a problem hiding this comment.
혹시 인터셉터 등록 순서 문제는 발생하지 않나요?
StompHandler 보다 이전에 로깅 인터셉터가 등록 되었을 때, User Principal 을 잘 가져올 수 있는지 궁금합니다!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
관련 이슈
작업 내용
채팅 관련 QA를 진행하는데, STOMP 프레임이 로깅되지 않아 동작을 제대로 확인하기 어려웠습니다.
이에 STOMP 프레임을 로깅하는 인터셉터를 작성했습니다.
dev프로파일인 경우에만 동작하도록 구현했습니다.prod프로파일에서도 로깅하면 사용자 채팅 등이 로깅될 수 있기에, 보안 상의 문제가 있을 거라도 판단했습니다.선택적으로 인터셉터를 스프링 컨테이너에 등록하기에,
Optional로 주입했고ifPresent로 처리하게끔 구현했습니다.특이 사항
리뷰 요구사항 (선택)