Open
Conversation
- MessageType enum에 TYPING, PRESENCE 추가
- /app/chat/room/{routieSpaceId}/typing 발행 핸들러 추가
- PresenceRegistry와 STOMP 세션 이벤트 리스너로 접속 현황 브로드캐스트
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
github-actions
Bot
requested review from
DongchannN,
cookie-meringue and
threepebbles
Contributor
Test Results 41 files 41 suites 12s ⏱️ Results for commit 2ff4be6. ♻️ This comment has been updated with latest results. |
구독 시점에 이벤트 리스너가 /topic/chat/room/{id}로 PRESENCE를 함께
브로드캐스트하므로, 테스트가 type=CHAT 메시지를 만날 때까지 큐를 소비하도록 변경
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Contributor
Author
|
@CodeRabbit 리뷰해줘 |
|
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughWebSocket을 통한 실시간 채팅 입력 중(Typing) 및 접속 중 확인(Presence) 기능을 구현하였습니다. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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 docstrings
🧪 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: 4
🧹 Nitpick comments (5)
backend/spring-routie/src/main/java/routie/business/websocket/ui/dto/request/TypingRequest.java (1)
3-6: 방 ID의 이중 소스는 정합성 리스크가 있습니다.
routieSpaceId가 경로 변수와 payload에 동시에 존재합니다. 둘 중 하나를 단일 소스로 두고, 유지가 필요하면 컨트롤러에서 두 값의 일치 여부를 검증하는 편이 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/spring-routie/src/main/java/routie/business/websocket/ui/dto/request/TypingRequest.java` around lines 3 - 6, Typo/consistency risk: TypingRequest contains routieSpaceId in the payload while the same ID is also provided as a path variable; pick one source of truth and validate if both are kept. Remove routieSpaceId from the TypingRequest record to make the path variable the single source of truth, or alternatively keep routieSpaceId in TypingRequest and remove it from the controller path, and if you must support both keep both but add explicit equality validation in the controller (e.g., in the controller method handling the typing event, compare path variable and TypingRequest.routieSpaceId and return 400 on mismatch). Ensure any references to TypingRequest.routieSpaceId are updated accordingly.backend/spring-routie/src/main/java/routie/business/websocket/ui/v1/ChatControllerV1.java (1)
60-63: 경로 방 ID와 payload 방 ID의 불일치를 처리해 주세요.
TypingRequest.routieSpaceId를 유지한다면 Line 60의 경로 변수와 일치 검증이 필요합니다. 지금처럼 무시하면 클라이언트 디버깅과 감사 로그에서 혼선이 생깁니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/spring-routie/src/main/java/routie/business/websocket/ui/v1/ChatControllerV1.java` around lines 60 - 63, In ChatControllerV1, validate that the `@DestinationVariable` routieSpaceId matches TypingRequest.routieSpaceId (request.getRoutieSpaceId()) before proceeding; if they differ, reject the request (e.g., throw a 400 Bad Request / ResponseStatusException or a domain-specific ValidationException), log the mismatch with both IDs and the Participant for auditing, and avoid silently ignoring the payload ID so client debugging and audit trails remain consistent.backend/spring-routie/src/test/java/routie/business/websocket/ui/v1/ChatControllerV1Test.java (1)
148-161: CHAT 필터링은 적절하지만 신규 타입 전용 검증은 별도로 추가하는 것을 권장합니다.현재 루프는
PRESENCE/TYPING을 건너뛰기만 하므로, 해당 타입의 payload 스키마/필드 회귀는 잡지 못합니다.typing()및 presence 브로드캐스트에 대한 통합 테스트를 분리해 추가해두면 안전합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/spring-routie/src/test/java/routie/business/websocket/ui/v1/ChatControllerV1Test.java` around lines 148 - 161, The current test in ChatControllerV1Test only filters messages by MessageType.CHAT in the blockingQueue poll loop and so will miss regressions in the payload/schema for TYPING and PRESENCE messages; add separate focused integration tests (e.g., typingBroadcastTest and presenceBroadcastTest) that invoke the controller's typing() and presence broadcast flows, read from the same blockingQueue (similar to the existing poll loop) and assert the specific payload fields and schema for MessageType.TYPING and MessageType.PRESENCE respectively so regressions in those types are caught.backend/spring-routie/src/main/java/routie/business/websocket/ui/listener/PresenceEventListener.java (1)
36-37: 운영 관점: in-memory presence는 수평 확장 시 정합성이 깨집니다.
PresenceRegistry가 프로세스 메모리 기반이면 멀티 인스턴스에서 방별 active user 목록이 노드별로 분리됩니다. 배포가 다중 인스턴스라면 Redis(또는 브로커 기반 공유 상태) + pub/sub 동기화로 전환을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/spring-routie/src/main/java/routie/business/websocket/ui/listener/PresenceEventListener.java` around lines 36 - 37, PresenceEventListener currently depends on an in-memory PresenceRegistry which will break correctness in multi-instance deployments; replace the in-memory registry with a distributed implementation (e.g., Redis-backed PresenceRegistry) and wire that into PresenceEventListener and any places that mutate presence, and add pub/sub synchronization so adds/removes are propagated across nodes; specifically, implement a RedisPresenceRegistry (or use a shared broker/state store), register it as the PresenceRegistry bean used by PresenceEventListener, ensure presence mutations publish/subscribe updates to keep per-room active user lists consistent, and add necessary Redis config/bean definitions and message listeners so SimpMessagingTemplate notifications operate correctly across all instances.backend/spring-routie/src/main/java/routie/business/websocket/application/PresenceRegistry.java (1)
79-80: 문자열 결합 키 대신 타입 키 사용을 권장합니다.Line 80의
sessionId + "|" + subscriptionId는 구분자 충돌 가능성을 남깁니다. 키를record로 분리하면 충돌 위험과 파싱 의존성을 함께 줄일 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/spring-routie/src/main/java/routie/business/websocket/application/PresenceRegistry.java` around lines 79 - 80, 현재 compositeKey(sessionId, subscriptionId)에서 문자열 결합을 사용해 충돌 가능성이 있으니 문자열 키 대신 Java record 타입을 도입하세요: 새 record (예: SubscriberKey or PresenceKey)를 선언해 sessionId, subscriptionId 필드를 보관하고 compositeKey 메서드를 해당 record를 반환하도록 변경하며, 내부 Map/Set(예: where compositeKey를 키로 쓰는 컬렉션) 선언도 String에서 이 record 타입을 키로 변경하세요; record는 자동으로 equals/hashCode를 제공하므로 별도 파싱/구분자 처리가 불필요해집니다.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@backend/spring-routie/src/main/java/routie/business/websocket/application/PresenceRegistry.java`:
- Around line 20-33: addSubscription/removeSubscription currently treat each
session|subscriptionId independently and remove the user from roomToUsers on
every unsubscribe, causing premature presence removal when the same participant
has multiple subscriptions; update the logic to maintain a per-participant
subscription count per room: keep using subscriptionKeyBySessionSub and
SubscriptionKey to map session-sub → (routieSpaceId, userId) but add a new
structure or change roomToUsers to store (ActiveUser, AtomicInteger count) or
maintain a separate Map<routieSpaceId, Map<userId, AtomicInteger>>; in
addSubscription (method addSubscription and compositeKey use) increment the
count and only insert ActiveUser when the count transitions 0→1, and in
removeSubscription decrement the count and only remove the ActiveUser from
roomToUsers when the count reaches 0.
- Around line 30-32: The add/remove sequence in PresenceRegistry is non-atomic:
using roomToUsers.computeIfAbsent(...).put(...) and later checking isEmpty()
then roomToUsers.remove(...) can lose a recently added user if another thread
removes the map between computeIfAbsent and put; change the logic to use atomic
map operations instead — use roomToUsers.compute(routieSpaceId, ...) to
atomically create-or-update the inner map and add the User in the same lambda
for the addUser path, and use roomToUsers.computeIfPresent(routieSpaceId, ...)
to remove the User and return null when the inner map becomes empty for the
removeUser path so the outer map entry is removed atomically; update the methods
that reference computeIfAbsent/put and isEmpty/remove accordingly.
In
`@backend/spring-routie/src/main/java/routie/business/websocket/ui/listener/PresenceEventListener.java`:
- Around line 40-64: onSubscribe currently registers any subscriber to presence
based solely on destination; add an authorization check to ensure the subscriber
is a room participant before calling presenceRegistry.addSubscription and
broadcastPresence. Use extractRoutieSpaceId(accessor.getDestination()) to get
the room id, retrieve the participant via getParticipant(accessor), then verify
room membership (e.g., call an existing service/method that validates
participant.getId() against the routieSpaceId or implement a new
isParticipantInRoom(routieSpaceId, participantId)). If the check fails, log a
warning and return without adding the subscription or broadcasting; only proceed
to presenceRegistry.addSubscription(...) and broadcastPresence(routieSpaceId)
when the membership check passes.
In
`@backend/spring-routie/src/main/java/routie/business/websocket/ui/v1/ChatControllerV1.java`:
- Around line 57-77: The typing(...) handler in ChatControllerV1 currently only
authenticates the user but does not verify that the participant is a member of
the routieSpaceId room; add a membership check (e.g., call a service method like
spaceMembershipService.isMember(routieSpaceId, participant.getId()) or
routieSpaceService.hasParticipant(...)) at the top of typing(...) and if the
check fails throw an AccessDeniedException or return an appropriate error
response (do not proceed to log or send the TypingResponse). Ensure you
reference the typing(...) method and Participant/TypingRequest types and use the
existing logger to record invalid access attempts before rejecting them.
---
Nitpick comments:
In
`@backend/spring-routie/src/main/java/routie/business/websocket/application/PresenceRegistry.java`:
- Around line 79-80: 현재 compositeKey(sessionId, subscriptionId)에서 문자열 결합을 사용해 충돌
가능성이 있으니 문자열 키 대신 Java record 타입을 도입하세요: 새 record (예: SubscriberKey or
PresenceKey)를 선언해 sessionId, subscriptionId 필드를 보관하고 compositeKey 메서드를 해당
record를 반환하도록 변경하며, 내부 Map/Set(예: where compositeKey를 키로 쓰는 컬렉션) 선언도 String에서 이
record 타입을 키로 변경하세요; record는 자동으로 equals/hashCode를 제공하므로 별도 파싱/구분자 처리가 불필요해집니다.
In
`@backend/spring-routie/src/main/java/routie/business/websocket/ui/dto/request/TypingRequest.java`:
- Around line 3-6: Typo/consistency risk: TypingRequest contains routieSpaceId
in the payload while the same ID is also provided as a path variable; pick one
source of truth and validate if both are kept. Remove routieSpaceId from the
TypingRequest record to make the path variable the single source of truth, or
alternatively keep routieSpaceId in TypingRequest and remove it from the
controller path, and if you must support both keep both but add explicit
equality validation in the controller (e.g., in the controller method handling
the typing event, compare path variable and TypingRequest.routieSpaceId and
return 400 on mismatch). Ensure any references to TypingRequest.routieSpaceId
are updated accordingly.
In
`@backend/spring-routie/src/main/java/routie/business/websocket/ui/listener/PresenceEventListener.java`:
- Around line 36-37: PresenceEventListener currently depends on an in-memory
PresenceRegistry which will break correctness in multi-instance deployments;
replace the in-memory registry with a distributed implementation (e.g.,
Redis-backed PresenceRegistry) and wire that into PresenceEventListener and any
places that mutate presence, and add pub/sub synchronization so adds/removes are
propagated across nodes; specifically, implement a RedisPresenceRegistry (or use
a shared broker/state store), register it as the PresenceRegistry bean used by
PresenceEventListener, ensure presence mutations publish/subscribe updates to
keep per-room active user lists consistent, and add necessary Redis config/bean
definitions and message listeners so SimpMessagingTemplate notifications operate
correctly across all instances.
In
`@backend/spring-routie/src/main/java/routie/business/websocket/ui/v1/ChatControllerV1.java`:
- Around line 60-63: In ChatControllerV1, validate that the `@DestinationVariable`
routieSpaceId matches TypingRequest.routieSpaceId (request.getRoutieSpaceId())
before proceeding; if they differ, reject the request (e.g., throw a 400 Bad
Request / ResponseStatusException or a domain-specific ValidationException), log
the mismatch with both IDs and the Participant for auditing, and avoid silently
ignoring the payload ID so client debugging and audit trails remain consistent.
In
`@backend/spring-routie/src/test/java/routie/business/websocket/ui/v1/ChatControllerV1Test.java`:
- Around line 148-161: The current test in ChatControllerV1Test only filters
messages by MessageType.CHAT in the blockingQueue poll loop and so will miss
regressions in the payload/schema for TYPING and PRESENCE messages; add separate
focused integration tests (e.g., typingBroadcastTest and presenceBroadcastTest)
that invoke the controller's typing() and presence broadcast flows, read from
the same blockingQueue (similar to the existing poll loop) and assert the
specific payload fields and schema for MessageType.TYPING and
MessageType.PRESENCE respectively so regressions in those types are caught.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3badbfb0-7cfd-4c7c-9a89-cc3b6e59b8de
📒 Files selected for processing (9)
backend/spring-routie/src/main/java/routie/business/websocket/application/PresenceRegistry.javabackend/spring-routie/src/main/java/routie/business/websocket/domain/MessageType.javabackend/spring-routie/src/main/java/routie/business/websocket/ui/dto/request/TypingRequest.javabackend/spring-routie/src/main/java/routie/business/websocket/ui/dto/response/ActiveUser.javabackend/spring-routie/src/main/java/routie/business/websocket/ui/dto/response/PresenceResponse.javabackend/spring-routie/src/main/java/routie/business/websocket/ui/dto/response/TypingResponse.javabackend/spring-routie/src/main/java/routie/business/websocket/ui/listener/PresenceEventListener.javabackend/spring-routie/src/main/java/routie/business/websocket/ui/v1/ChatControllerV1.javabackend/spring-routie/src/test/java/routie/business/websocket/ui/v1/ChatControllerV1Test.java
Comment on lines
+20
to
+33
| public void addSubscription( | ||
| final String sessionId, | ||
| final String subscriptionId, | ||
| final Long routieSpaceId, | ||
| final ActiveUser user | ||
| ) { | ||
| subscriptionKeyBySessionSub.put( | ||
| compositeKey(sessionId, subscriptionId), | ||
| new SubscriptionKey(routieSpaceId, user.userId()) | ||
| ); | ||
| roomToUsers | ||
| .computeIfAbsent(routieSpaceId, id -> new ConcurrentHashMap<>()) | ||
| .put(user.userId(), user); | ||
| } |
There was a problem hiding this comment.
같은 사용자의 다중 구독을 단일 구독처럼 처리해 Presence가 조기 이탈됩니다.
Line 40, Line 55에서 구독 1건 해제마다 사용자를 바로 방에서 제거하고 있습니다.
현재 구조는 sessionId|subscriptionId 단위로 구독을 저장하므로, 동일 participantId가 같은 방에 2개 이상 구독한 경우(재구독/멀티탭) 아직 활성 구독이 남아 있어도 목록에서 사라집니다.
🔧 제안 수정안 (participant별 구독 수 참조 카운트)
public class PresenceRegistry {
private final Map<Long, Map<Long, ActiveUser>> roomToUsers = new ConcurrentHashMap<>();
private final Map<String, SubscriptionKey> subscriptionKeyBySessionSub = new ConcurrentHashMap<>();
+ private final Map<SubscriptionKey, Integer> subscriptionCountByUserInRoom = new ConcurrentHashMap<>();
public void addSubscription(
final String sessionId,
final String subscriptionId,
final Long routieSpaceId,
final ActiveUser user
) {
- subscriptionKeyBySessionSub.put(
- compositeKey(sessionId, subscriptionId),
- new SubscriptionKey(routieSpaceId, user.userId())
- );
+ final SubscriptionKey roomUserKey = new SubscriptionKey(routieSpaceId, user.userId());
+ subscriptionKeyBySessionSub.put(compositeKey(sessionId, subscriptionId), roomUserKey);
+ subscriptionCountByUserInRoom.merge(roomUserKey, 1, Integer::sum);
roomToUsers
.computeIfAbsent(routieSpaceId, id -> new ConcurrentHashMap<>())
.put(user.userId(), user);
}
public Optional<SubscriptionKey> removeSubscription(final String sessionId, final String subscriptionId) {
final SubscriptionKey key = subscriptionKeyBySessionSub.remove(compositeKey(sessionId, subscriptionId));
if (key == null) {
return Optional.empty();
}
- removeUserFromRoom(key.routieSpaceId(), key.participantId());
+ decrementAndRemoveIfLast(key);
return Optional.of(key);
}
public List<SubscriptionKey> removeAllForSession(final String sessionId) {
...
for (final SubscriptionKey key : removed) {
- removeUserFromRoom(key.routieSpaceId(), key.participantId());
+ decrementAndRemoveIfLast(key);
}
return removed;
}
+
+ private void decrementAndRemoveIfLast(final SubscriptionKey key) {
+ subscriptionCountByUserInRoom.compute(key, (k, count) -> {
+ if (count == null || count <= 1) {
+ removeUserFromRoom(k.routieSpaceId(), k.participantId());
+ return null;
+ }
+ return count - 1;
+ });
+ }
}Also applies to: 35-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/spring-routie/src/main/java/routie/business/websocket/application/PresenceRegistry.java`
around lines 20 - 33, addSubscription/removeSubscription currently treat each
session|subscriptionId independently and remove the user from roomToUsers on
every unsubscribe, causing premature presence removal when the same participant
has multiple subscriptions; update the logic to maintain a per-participant
subscription count per room: keep using subscriptionKeyBySessionSub and
SubscriptionKey to map session-sub → (routieSpaceId, userId) but add a new
structure or change roomToUsers to store (ActiveUser, AtomicInteger count) or
maintain a separate Map<routieSpaceId, Map<userId, AtomicInteger>>; in
addSubscription (method addSubscription and compositeKey use) increment the
count and only insert ActiveUser when the count transitions 0→1, and in
removeSubscription decrement the count and only remove the ActiveUser from
roomToUsers when the count reaches 0.
Comment on lines
+30
to
+32
| roomToUsers | ||
| .computeIfAbsent(routieSpaceId, id -> new ConcurrentHashMap<>()) | ||
| .put(user.userId(), user); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
TARGET="backend/spring-routie/src/main/java/routie/business/websocket/application/PresenceRegistry.java"
echo "[check-1] add 경로: computeIfAbsent + put 분리 여부"
rg -n -C2 'computeIfAbsent\(routieSpaceId' "$TARGET"
rg -n -C2 '\.put\(user\.userId\(\), user\)' "$TARGET"
echo "[check-2] remove 경로: isEmpty 후 remove 분리 여부"
rg -n -C2 'users\.isEmpty\(\)' "$TARGET"
rg -n -C2 'roomToUsers\.remove\(routieSpaceId\)' "$TARGET"Repository: woowacourse-teams/2025-routie
Length of output: 800
🏁 Script executed:
cat -n backend/spring-routie/src/main/java/routie/business/websocket/application/PresenceRegistry.javaRepository: woowacourse-teams/2025-routie
Length of output: 3639
동시 add/remove 작업 간 활성 사용자 유실 가능성 존재
computeIfAbsent 후 put (30-32줄)과 isEmpty() 체크 후 remove (74-76줄)이 각각 비원자 작업으로 분리되어 있습니다. 다음 시나리오에서 방금 추가된 사용자가 접근 불가능한 상태가 됩니다:
- 스레드 A:
computeIfAbsent로 방 맵을 획득 - 스레드 B: 다른 사용자의 제거로
isEmpty()확인 후roomToUsers.remove()실행 - 스레드 A: 돌아와서
put()실행 → 맵이 더 이상roomToUsers에 없음
제안 수정안
public void addSubscription(
final String sessionId,
final String subscriptionId,
final Long routieSpaceId,
final ActiveUser user
) {
subscriptionKeyBySessionSub.put(
compositeKey(sessionId, subscriptionId),
new SubscriptionKey(routieSpaceId, user.userId())
);
- roomToUsers
- .computeIfAbsent(routieSpaceId, id -> new ConcurrentHashMap<>())
- .put(user.userId(), user);
+ roomToUsers.compute(routieSpaceId, (id, users) -> {
+ if (users == null) {
+ users = new ConcurrentHashMap<>();
+ }
+ users.put(user.userId(), user);
+ return users;
+ });
}
private void removeUserFromRoom(final Long routieSpaceId, final Long participantId) {
- final Map<Long, ActiveUser> users = roomToUsers.get(routieSpaceId);
- if (users == null) {
- return;
- }
- users.remove(participantId);
- if (users.isEmpty()) {
- roomToUsers.remove(routieSpaceId);
- }
+ roomToUsers.computeIfPresent(routieSpaceId, (id, users) -> {
+ users.remove(participantId);
+ return users.isEmpty() ? null : users;
+ });
}compute()와 computeIfPresent()를 사용하여 외부 맵의 읽기-수정-쓰기 작업을 원자 단위로 통일해야 합니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| roomToUsers | |
| .computeIfAbsent(routieSpaceId, id -> new ConcurrentHashMap<>()) | |
| .put(user.userId(), user); | |
| public void addSubscription( | |
| final String sessionId, | |
| final String subscriptionId, | |
| final Long routieSpaceId, | |
| final ActiveUser user | |
| ) { | |
| subscriptionKeyBySessionSub.put( | |
| compositeKey(sessionId, subscriptionId), | |
| new SubscriptionKey(routieSpaceId, user.userId()) | |
| ); | |
| roomToUsers.compute(routieSpaceId, (id, users) -> { | |
| if (users == null) { | |
| users = new ConcurrentHashMap<>(); | |
| } | |
| users.put(user.userId(), user); | |
| return users; | |
| }); | |
| } | |
| private void removeUserFromRoom(final Long routieSpaceId, final Long participantId) { | |
| roomToUsers.computeIfPresent(routieSpaceId, (id, users) -> { | |
| users.remove(participantId); | |
| return users.isEmpty() ? null : users; | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/spring-routie/src/main/java/routie/business/websocket/application/PresenceRegistry.java`
around lines 30 - 32, The add/remove sequence in PresenceRegistry is non-atomic:
using roomToUsers.computeIfAbsent(...).put(...) and later checking isEmpty()
then roomToUsers.remove(...) can lose a recently added user if another thread
removes the map between computeIfAbsent and put; change the logic to use atomic
map operations instead — use roomToUsers.compute(routieSpaceId, ...) to
atomically create-or-update the inner map and add the User in the same lambda
for the addUser path, and use roomToUsers.computeIfPresent(routieSpaceId, ...)
to remove the User and return null when the inner map becomes empty for the
removeUser path so the outer map entry is removed atomically; update the methods
that reference computeIfAbsent/put and isEmpty/remove accordingly.
Comment on lines
+40
to
+64
| public void onSubscribe(final SessionSubscribeEvent event) { | ||
| final StompHeaderAccessor accessor = StompHeaderAccessor.wrap(event.getMessage()); | ||
| final Long routieSpaceId = extractRoutieSpaceId(accessor.getDestination()); | ||
| if (routieSpaceId == null) { | ||
| return; | ||
| } | ||
|
|
||
| final Participant participant = getParticipant(accessor); | ||
| if (participant == null) { | ||
| log.warn("구독 이벤트에 participant 정보가 없습니다. sessionId: {}", accessor.getSessionId()); | ||
| return; | ||
| } | ||
|
|
||
| final ActiveUser user = new ActiveUser( | ||
| participant.getId(), | ||
| participant.getNickname(), | ||
| participant.getRole().name() | ||
| ); | ||
| presenceRegistry.addSubscription( | ||
| accessor.getSessionId(), | ||
| accessor.getSubscriptionId(), | ||
| routieSpaceId, | ||
| user | ||
| ); | ||
| broadcastPresence(routieSpaceId); |
There was a problem hiding this comment.
구독 이벤트에 방 참여 권한 검증이 없어 presence 노출 위험이 있습니다.
Line 40-64는 /topic/chat/room/{id} 구독만으로 사용자를 active 목록에 등록하고 브로드캐스트합니다. 방 참여자 검증이 없으면 비참여 사용자가 presence를 조회하고 목록에 포함될 수 있습니다.
🔧 권장 수정 예시
public void onSubscribe(final SessionSubscribeEvent event) {
final StompHeaderAccessor accessor = StompHeaderAccessor.wrap(event.getMessage());
final Long routieSpaceId = extractRoutieSpaceId(accessor.getDestination());
if (routieSpaceId == null) {
return;
}
final Participant participant = getParticipant(accessor);
if (participant == null) {
log.warn("구독 이벤트에 participant 정보가 없습니다. sessionId: {}", accessor.getSessionId());
return;
}
+ if (!presenceAuthorizationService.canSubscribeRoom(participant.getId(), routieSpaceId)) {
+ log.warn("권한 없는 room 구독 시도. participantId: {}, roomId: {}", participant.getId(), routieSpaceId);
+ return;
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/spring-routie/src/main/java/routie/business/websocket/ui/listener/PresenceEventListener.java`
around lines 40 - 64, onSubscribe currently registers any subscriber to presence
based solely on destination; add an authorization check to ensure the subscriber
is a room participant before calling presenceRegistry.addSubscription and
broadcastPresence. Use extractRoutieSpaceId(accessor.getDestination()) to get
the room id, retrieve the participant via getParticipant(accessor), then verify
room membership (e.g., call an existing service/method that validates
participant.getId() against the routieSpaceId or implement a new
isParticipantInRoom(routieSpaceId, participantId)). If the check fails, log a
warning and return without adding the subscription or broadcasting; only proceed
to presenceRegistry.addSubscription(...) and broadcastPresence(routieSpaceId)
when the membership check passes.
Comment on lines
+57
to
+77
| @MessageMapping("/chat/room/{routieSpaceId}/typing") | ||
| @SendTo("/topic/chat/room/{routieSpaceId}") | ||
| public TypingResponse typing( | ||
| @DestinationVariable("routieSpaceId") final Long routieSpaceId, | ||
| @AuthenticatedParticipant final Participant participant, | ||
| @Payload final TypingRequest request | ||
| ) { | ||
| log.info( | ||
| "타이핑 이벤트 Space ID: {}, Sender: {}, isTyping: {}", | ||
| routieSpaceId, participant.getId(), request.isTyping() | ||
| ); | ||
|
|
||
| return new TypingResponse( | ||
| MessageType.TYPING, | ||
| participant.getId(), | ||
| participant.getNickname(), | ||
| participant.getRole().name(), | ||
| request.isTyping(), | ||
| Instant.now().toString() | ||
| ); | ||
| } |
There was a problem hiding this comment.
타이핑 이벤트 송신에 방 참여 권한 검증이 없습니다.
Line 59-76 흐름에서는 인증만 확인하고, 해당 사용자가 routieSpaceId 방의 참여자인지 확인하지 않습니다. 이 경우 인증된 임의 사용자가 타 방으로 TYPING 이벤트를 송신할 수 있습니다.
🔧 권장 수정 예시
public TypingResponse typing(
`@DestinationVariable`("routieSpaceId") final Long routieSpaceId,
`@AuthenticatedParticipant` final Participant participant,
`@Payload` final TypingRequest request
) {
+ chatService.validateParticipantInRoutieSpace(routieSpaceId, participant.getId());
+
log.info(
"타이핑 이벤트 Space ID: {}, Sender: {}, isTyping: {}",
routieSpaceId, participant.getId(), request.isTyping()
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/spring-routie/src/main/java/routie/business/websocket/ui/v1/ChatControllerV1.java`
around lines 57 - 77, The typing(...) handler in ChatControllerV1 currently only
authenticates the user but does not verify that the participant is a member of
the routieSpaceId room; add a membership check (e.g., call a service method like
spaceMembershipService.isMember(routieSpaceId, participant.getId()) or
routieSpaceService.hasParticipant(...)) at the top of typing(...) and if the
check fails throw an AccessDeniedException or return an appropriate error
response (do not proceed to log or send the TypingResponse). Ensure you
reference the typing(...) method and Participant/TypingRequest types and use the
existing logger to record invalid access attempts before rejecting them.
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
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.
As-Is
To-Be
WebSocket Messages 명세서의 세 가지 API를 구현한다.
/app/chat/room/{routieSpaceId}/typing(PUB)/topic/chat/room/{routieSpaceId}(type: TYPING)/topic/chat/room/{routieSpaceId}(type: PRESENCE)변경 사항
MessageTypeenum에TYPING,PRESENCE추가TypingRequest/TypingResponse/PresenceResponse/ActiveUserDTO 추가ChatControllerV1에/typing핸들러 추가 (DB 저장 없이 브로드캐스트만)PresenceRegistry추가: 방별 접속자와sessionId|subscriptionId→ (방, 참여자) 역참조 맵을ConcurrentHashMap으로 관리 (in-memory)PresenceEventListener추가:SessionSubscribeEvent/SessionUnsubscribeEvent/SessionDisconnectEvent를 받아/topic/chat/room/{id}패턴에서만 presence 업데이트 후PRESENCE브로드캐스트Check List
Test Screenshot
로컬 STOMP 클라이언트 두 개로 다음 플로우 검증 완료:
PRESENCE(activeUsers 2명) 수신TYPING수신PRESENCE(activeUsers 1명) 수신(Optional) Additional Description
Closes #1125
Summary by CodeRabbit
릴리스 노트