Conversation
Walkthrough단체전 게임의 플레이어 카드에 닉네임과 슬롯 정보를 추가하기 위해 백엔드 WebSocket 핸들러에서 룸별 세션 캐싱 및 EMOJI_CHAT 프로토콜을 확장하고, 프론트엔드에서 새로운 MULTI_USER 메시지 처리와 반응형 UI 레이아웃으로 리팩토링하였습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트<br/>(브라우저)
participant WS as WebSocket<br/>MultiWebSocket
participant SessionCache as 세션<br/>캐시
Client->>WS: onOpen(session)
activate WS
WS->>SessionCache: roomId/userId 등록<br/>sessionNickMap, sessionSlotMap 초기화
SessionCache-->>WS: 캐시 준비 완료
WS-->>Client: 연결 수립
deactivate WS
Client->>WS: GAME_MULTI_START 메시지
activate WS
WS->>SessionCache: cacheSlotIfStart()<br/>슬롯 감지 및 저장
SessionCache-->>WS: 슬롯 반환
WS->>SessionCache: buildMultiUserJson() 생성
WS->>WS: 룸 전체에 MULTI_USER<br/>브로드캐스트
WS->>WS: sendAllKnownUsersTo()<br/>기존 플레이어 정보 전달
WS-->>Client: MULTI_USER 페이로드<br/>(slot, nickname)
deactivate WS
Client->>Client: onMultiUser(payload)<br/>슬롯 닉네임 카드 업데이트
Client->>WS: EMOJI_CHAT 메시지<br/>(이모지)
activate WS
WS->>WS: 룸별 스코프로<br/>브로드캐스트
WS-->>Client: EMOJI_CHAT 응답<br/>(from, fromNick, slot)
deactivate WS
Client->>Client: onEmojiChat(payload)<br/>플레이어 카드에 이모지 표시
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (9)
src/main/java/game/multi/ws/MultiWebSocket.java (5)
35-42: 캐시 구조 설계 적절함
ConcurrentHashMap을 사용한 room 단위 세션/유저/닉/슬롯 캐싱 구조가 적절합니다. 다만,sessionSlotMap의 slot 값이 0~3 범위임을 명시하는 주석이 있으니, 범위 유효성 검증을cacheSlotIfStart에서 추가하는 것을 권장합니다.
62-65: 세션 종료 시 CloseReason 추가 권장멤버십 검증 실패 시
session.close()만 호출하면 클라이언트가 종료 사유를 알 수 없습니다. 디버깅과 사용자 경험을 위해CloseReason을 포함하는 것이 좋습니다.🔎 제안된 수정
if (!multiPlayerDao.isMember(roomId, userId)) { - session.close(); + session.close(new javax.websocket.CloseReason( + javax.websocket.CloseReason.CloseCodes.VIOLATED_POLICY, + "Not a member of this room")); return; }
154-158: 예외 무시(silent catch) 개선 필요빈 catch 블록은 전송 실패 원인을 숨깁니다. 최소한 debug 레벨 로깅을 추가하여 문제 진단이 가능하도록 하세요.
🔎 제안된 수정
try { synchronized (s) { s.getBasicRemote().sendText(json); } -} catch (Exception ignore) {} +} catch (Exception e) { + // 연결 끊김 등 일시적 오류는 무시하되 로깅 + System.err.println("EMOJI broadcast 실패: " + e.getMessage()); +}
263-267: 슬롯 범위 유효성 검증 추가 권장주석에서 slot이 0~3 범위라고 명시했으나, 실제로 범위를 검증하지 않습니다. 잘못된 slot 값이 캐싱되면 이후 로직에서 예상치 못한 동작이 발생할 수 있습니다.
🔎 제안된 수정
if (obj.has("slot")) { int slot = obj.get("slot").getAsInt(); + if (slot < 0 || slot > 3) { + return null; // 유효하지 않은 슬롯 + } sessionSlotMap.put(target.getId(), slot); return slot; }
346-351: 반복되는 빈 catch 블록 패턴파일 전체에서
catch (Exception ignore) {}가 반복됩니다. 공통 유틸리티 메서드로 추출하거나, 최소한 일관된 로깅을 추가하는 것을 권장합니다.🔎 공통 전송 메서드 추출 예시
private void safeSend(Session s, String json) { if (s == null || !s.isOpen()) return; try { synchronized (s) { s.getBasicRemote().sendText(json); } } catch (Exception e) { // 연결 종료 시 발생할 수 있는 예외는 무시 } }src/main/webapp/WEB-INF/views/game/multi.jsp (4)
85-97: 슬롯 번호와 요소 ID 매핑 확인 필요플레이어 카드의
id와data-slot이 직관적이지 않습니다:
#p1→ slot 0,#p4→ slot 2 (왼쪽 열)#p3→ slot 1,#p2→ slot 3 (오른쪽 열)의도된 UI 배치라면 주석으로 명시하는 것이 유지보수에 도움됩니다.
254-266: MULTI_USER 처리 로직 중복
MULTI_USER메시지 처리가 여기와emojiChatMulti.js의window.onMultiUser에 모두 구현되어 있습니다.emojiChatMulti.js에서window.onMultiUser를 정의하지만 여기서는 직접 DOM을 업데이트합니다. 일관성을 위해 한 곳에서만 처리하거나,window.onMultiUser를 호출하세요.🔎 제안된 수정
/* 닉네임/슬롯 수신 */ if (data.type === "MULTI_USER") { - const p = data.payload || {}; - const slot = p.slot; - const nick = p.nickname; - - const card = document.querySelector(`.player-card[data-slot='${slot}']`); - if (card) { - const nameEl = card.querySelector(".name"); - if (nameEl && nick) nameEl.textContent = nick; - } + if (typeof window.onMultiUser === "function") { + window.onMultiUser(data.payload || {}); + } return; }
308-318: 타이머 메모리 누수 방지 권장
startTimer의setInterval이 게임 종료(MULTI_WIN, GAME_OVER) 시에만 정리됩니다. 페이지 이탈 시 타이머가 계속 실행될 수 있으므로beforeunload이벤트에서도 정리하는 것이 좋습니다.🔎 제안된 수정
window.addEventListener("beforeunload", () => { clearInterval(timer); });
290-306: fetch 응답 미사용 및 사용자 피드백 부재
fetch응답(data)을 사용하지 않고, 3초 대기 후 리다이렉트합니다. 사용자에게 "잠시 후 방으로 이동합니다" 같은 피드백을 제공하는 것이 좋습니다.🔎 제안된 수정
function goToRoomView() { try { ws.close(); } catch (e) {} + statusDiv.innerText = "잠시 후 방으로 이동합니다..."; fetch(contextPath + "/room/playersToRoom", { method: "POST", headers: { "Content-Type": "application/x-www-form-urlencoded; charset=UTF-8" }, body: "roomId=" + encodeURIComponent(roomId) }) - .then(res => res.json()) - .then(data => { + .then(() => { setTimeout(() => { location.href = contextPath + "/room?roomId=" + encodeURIComponent(roomId) + "&playType=1"; }, 3000); })
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/game/multi/ws/MultiWebSocket.javasrc/main/webapp/WEB-INF/views/game/multi.jspsrc/main/webapp/static/chat/emojiChatMulti.js
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/webapp/static/chat/emojiChatMulti.js (1)
src/main/webapp/static/room/room.js (1)
ws(15-15)
🔇 Additional comments (7)
src/main/java/game/multi/ws/MultiWebSocket.java (2)
295-325: LGTM - 늦게 입장한 유저 동기화 처리슬롯이 확정된 기존 유저들의 닉네임을 새로 입장한 유저에게 전송하는 로직이 잘 구현되어 있습니다. 4명 제한의 단체전이므로 반복 전송 방식도 적절합니다.
206-215: 중복 호출 우려는 근거 없음 - 코드가 의도대로 동작 중
handleOpen에서 4명 입장 시 정확히 4개의SendJob객체만 생성되며, 각 job은 서로 다른 session을 target으로 합니다.dispatch메서드는 jobs 리스트를 순회하면서 각 job을 한 번씩만 처리하므로, 동일 세션에 대한 중복 호출은 발생하지 않습니다.sessionSlotMap이ConcurrentHashMap이므로 slot 캐싱과 이후 읽기도 스레드 안전하게 처리됩니다.src/main/webapp/static/chat/emojiChatMulti.js (4)
12-14: XSS 취약점 가능성 검토
slot값이 숫자로 검증되고 있어 현재는 안전하지만, 향후 유지보수 시 주의가 필요합니다.data-slot속성 선택자에 직접 문자열을 삽입하는 패턴입니다.
38-48: LGTM - 이모지 수신 핸들러
slot이 숫자 타입인지 확인하는 검증이 적절히 추가되었습니다.
54-69: LGTM - MULTI_USER 핸들러슬롯 타입 검증 및 카드 존재 확인 후 닉네임을 업데이트하는 로직이 안전하게 구현되었습니다.
93-100: LGTM - 이벤트 바인딩 개선인라인
onclick대신addEventListener를 사용한 이벤트 바인딩이 적절합니다.src/main/webapp/WEB-INF/views/game/multi.jsp (1)
10-71: LGTM - CSS 레이아웃반응형 flex 레이아웃과 플레이어 카드 스타일링이 잘 구성되었습니다.
| /* 내 슬롯 카드에 즉시 표시 */ | ||
| if (typeof window.mySlot === "number") { | ||
| showBubbleOnSlot(window.mySlot, emoji); | ||
| } | ||
|
|
||
| /* 서버는 문자열 프로토콜을 기대함 */ | ||
| ws.send("EMOJI_CHAT:" + key); |
There was a problem hiding this comment.
발신자에게 이모지 중복 표시 가능성
Line 82-84에서 발신자 슬롯에 즉시 이모지를 표시하고, 서버에서 브로드캐스트된 메시지도 다시 수신합니다. 서버가 발신자 포함 전체에 브로드캐스트하면 동일 이모지가 두 번 표시될 수 있습니다.
🔎 해결 방안
서버에서 발신자를 제외하고 브로드캐스트하거나, 클라이언트에서 자신의 슬롯 메시지를 필터링하세요:
window.onEmojiChat = (payload) => {
if (!payload) return;
const slot = payload.slot;
+ // 자신이 보낸 이모지는 이미 표시됨
+ if (typeof window.mySlot === "number" && slot === window.mySlot) return;
+
const key = payload.emoji;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
src/main/webapp/static/chat/emojiChatMulti.js around lines 81-87: the code
immediately shows the emoji on the sender's slot and also relies on a server
broadcast, causing duplicate display for the sender; to fix either (A) stop the
immediate local display and rely on the server broadcast, or (B) keep the
immediate display but add sender metadata to the outgoing message (e.g., include
slot/id) and update the incoming message handler to ignore messages where the
sender id/slot equals window.mySlot so the sender does not render the
broadcasted emoji again.
| function log(msg) { | ||
| const logDiv = document.getElementById("log"); | ||
| logDiv.innerHTML += msg + "<br>"; | ||
| logDiv.scrollTop = logDiv.scrollHeight; | ||
| } |
There was a problem hiding this comment.
XSS 취약점: innerHTML 사용
log 함수에서 서버 메시지를 innerHTML로 삽입합니다. 서버 메시지가 신뢰할 수 없는 입력을 포함하면 XSS 공격에 취약합니다. textContent를 사용하거나 HTML 이스케이프 처리가 필요합니다.
🔎 제안된 수정
function log(msg) {
const logDiv = document.getElementById("log");
- logDiv.innerHTML += msg + "<br>";
+ const line = document.createElement("div");
+ line.textContent = msg;
+ logDiv.appendChild(line);
logDiv.scrollTop = logDiv.scrollHeight;
}📝 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.
| function log(msg) { | |
| const logDiv = document.getElementById("log"); | |
| logDiv.innerHTML += msg + "<br>"; | |
| logDiv.scrollTop = logDiv.scrollHeight; | |
| } | |
| function log(msg) { | |
| const logDiv = document.getElementById("log"); | |
| const line = document.createElement("div"); | |
| line.textContent = msg; | |
| logDiv.appendChild(line); | |
| logDiv.scrollTop = logDiv.scrollHeight; | |
| } |
🤖 Prompt for AI Agents
In src/main/webapp/WEB-INF/views/game/multi.jsp around lines 355 to 359, the log
function inserts server messages using innerHTML which allows XSS; change it to
use safe DOM methods (e.g., create a text node or set element.textContent for
the message, then append a <br> element created via document.createElement)
instead of concatenating into innerHTML, and maintain the existing scrollTop =
scrollHeight behavior so the log still autoscrolls.
3 participants
📌 작업 내용
🔗 관련 이슈
👀 리뷰 시 참고사항
💭 느낀 점
💻 스크린샷 (선택)
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선사항
✏️ Tip: You can customize this high-level summary in your review settings.