Conversation
|
Warning Rate limit exceeded@kkhhmm3103 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 2 minutes and 33 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
Walkthrough멀티게임 WebSocket에 방 단위 세션·닉네임·슬롯 캐시를 추가하고 EMOJI_CHAT 프로토콜로 방내 이모지 브로드캐스트를 구현하며, 클라이언트에 4인 카드 레이아웃과 슬롯 기반 이모지 채팅 스크립트를 통합했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as 클라이언트 (브라우저)
participant WS as MultiWebSocket (서버)
participant Cache as Room Session Cache
Browser->>WS: WebSocket 연결 (onOpen, roomId)
activate WS
WS->>Cache: session → room/user/nick/slot 저장
deactivate WS
rect rgb(220,245,220)
Browser->>WS: "EMOJI_CHAT:<key>" (보냄)
activate WS
WS->>Cache: 같은 방의 세션 목록 조회
WS-->>Browser: EMOJI_CHAT JSON 페이로드 브로드캐스트 (각 세션)
deactivate WS
end
Browser->>Browser: window.onEmojiChat(payload) → showBubbleOnSlot(slot, emoji)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 6
🧹 Nitpick comments (1)
src/main/java/game/multi/ws/MultiWebSocket.java (1)
251-273: 예외 무시 대신 로깅 권장
catch (Exception ignore)패턴이 디버깅을 어렵게 만들 수 있습니다. 최소한 경고 수준의 로깅을 추가하면 문제 추적에 도움이 됩니다.🔎 개선 제안
- } catch (Exception ignore) {} + } catch (Exception e) { + System.err.println("slot 캐싱 실패: " + e.getMessage()); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/game/multi/ws/MultiWebSocket.javasrc/main/webapp/WEB-INF/views/chat/gameEmoji.jspsrc/main/webapp/WEB-INF/views/game/multi.jspsrc/main/webapp/static/chat/emojiChat.jssrc/main/webapp/static/chat/emojiChatMulti.js
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/webapp/static/chat/emojiChatMulti.js (2)
src/main/webapp/static/chat/room.js (3)
bubble(52-52)onWsMessage(18-28)sendWs(7-7)src/main/webapp/static/room/room.js (1)
ws(15-15)
src/main/webapp/static/chat/emojiChat.js (1)
src/main/webapp/static/room/room.js (1)
ws(15-15)
src/main/java/game/multi/ws/MultiWebSocket.java (2)
src/main/webapp/static/room/room.js (1)
roomId(3-3)src/main/webapp/static/record/mypage.js (1)
nickname(246-246)
🔇 Additional comments (9)
src/main/webapp/WEB-INF/views/chat/gameEmoji.jsp (1)
1-33: LGTM!주석 변경만 있으며, 개인전(1:1) 이모지 채팅 UI 구조가 적절히 유지되고 있습니다.
src/main/webapp/static/chat/emojiChat.js (2)
4-10: LGTM!
p1/p2기반 DOM 참조와 null-safe 검사가 적절하게 구현되었습니다. 개인전 모드에서 상대(p1)와 나(p2) 구분이 명확합니다.
86-106: LGTM!
sendEmoji함수가 WebSocket 연결 상태를 적절히 검사하고, 텍스트 프로토콜(EMOJI_CHAT:key)로 서버에 전송하는 로직이 올바릅니다.src/main/java/game/multi/ws/MultiWebSocket.java (3)
37-44: 정적 캐시 메모리 누수 주의
ConcurrentHashMap사용은 적절하지만, 애플리케이션 전체 수명 동안 유지되는 정적 캐시입니다.onClose에서 정리 로직이 있지만, 예외 상황(서버 비정상 종료, 네트워크 끊김 등)에서 세션이 정리되지 않을 수 있습니다.향후 세션 누수 방지를 위해 주기적인 캐시 정리 메커니즘(scheduled cleanup) 도입을 고려해 주세요.
64-68: LGTM!방 멤버 검증 후 비멤버인 경우 세션을 닫고 즉시 반환하는 로직이 적절합니다. 무단 접근 방지에 효과적입니다.
177-201: LGTM!세션 종료 시 모든 캐시(room, user, nick, slot)를 적절히 정리하고, 빈 방은
roomSessions에서 제거하여 메모리 누수를 방지합니다.src/main/webapp/static/chat/emojiChatMulti.js (1)
14-29: LGTM!
showBubbleOnSlot함수가 null 체크와 타임아웃 정리를 적절히 수행합니다. 기존 타임아웃을 취소한 후 새 타임아웃을 설정하는 패턴이 올바릅니다.src/main/webapp/WEB-INF/views/game/multi.jsp (2)
35-57: LGTM!4명 플레이어 카드 레이아웃이
data-slot속성과 함께 적절히 구성되었습니다. 슬롯 기반 이모지 표시 로직과 잘 연동됩니다.
204-210: LGTM!
EMOJI_CHAT메시지 타입 처리가window.onEmojiChat함수 존재 여부를 확인한 후 호출하여 안전합니다.
| if (msg != null && msg.startsWith("EMOJI_CHAT:")) { | ||
| String emojiKey = msg.substring("EMOJI_CHAT:".length()).trim(); | ||
| if (emojiKey.isEmpty()) | ||
| return; | ||
|
|
||
| String roomId = sessionRoomMap.get(session.getId()); | ||
| if (roomId == null) | ||
| return; | ||
|
|
||
| String userId = sessionUserMap.get(session.getId()); | ||
| String nickname = sessionNickMap.get(session.getId()); | ||
| if (nickname == null || nickname.isBlank()) | ||
| nickname = (userId != null ? userId : "unknown"); | ||
|
|
||
| int slot = -1; | ||
| Integer cachedSlot = sessionSlotMap.get(session.getId()); | ||
| if (cachedSlot != null) | ||
| slot = cachedSlot; | ||
|
|
||
| JsonObject root = new JsonObject(); | ||
| root.addProperty("type", "EMOJI_CHAT"); | ||
|
|
||
| JsonObject payload = new JsonObject(); | ||
| if (userId != null) | ||
| payload.addProperty("from", userId); | ||
| payload.addProperty("fromNick", nickname); | ||
| payload.addProperty("emoji", emojiKey); | ||
| if (slot >= 0) | ||
| payload.addProperty("slot", slot); | ||
|
|
||
| root.add("payload", payload); | ||
|
|
||
| String json = gson.toJson(root); | ||
|
|
||
| // room 내 세션에게만 전송 | ||
| Map<String, Session> targets = roomSessions.get(roomId); | ||
| if (targets != null) { | ||
| for (Session s : targets.values()) { | ||
| if (s != null && s.isOpen()) { | ||
| try { | ||
| synchronized (s) { | ||
| s.getBasicRemote().sendText(json); | ||
| } | ||
| } catch (Exception ignore) {} | ||
| } | ||
| } | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
입력 검증 부재 및 예외 처리 개선 필요
emojiKey에 대한 검증이 없습니다. 현재는 클라이언트가 전달한 값을 그대로 JSON에 포함하여 브로드캐스트합니다. 악의적인 입력(예: 매우 긴 문자열, 특수 문자)에 대한 방어가 필요합니다.
또한 Line 163에서 예외를 무시하면 디버깅이 어려워집니다.
🔎 개선 제안
if (msg != null && msg.startsWith("EMOJI_CHAT:")) {
String emojiKey = msg.substring("EMOJI_CHAT:".length()).trim();
- if (emojiKey.isEmpty())
+ // 허용된 이모지 키만 처리
+ if (emojiKey.isEmpty() || !isValidEmojiKey(emojiKey))
return;// 헬퍼 메서드 추가
private boolean isValidEmojiKey(String key) {
return key != null && key.matches("^(smile|angry|clap)$");
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/game/multi/ws/MultiWebSocket.java around lines 120-168,
validate and sanitize the emojiKey before using it and stop swallowing
exceptions: add a helper (e.g., isValidEmojiKey) that enforces non-null, max
length (e.g., 64 chars) and an allowlist or safe pattern (e.g., alphanumerics,
hyphens/underscores or explicit emoji keys), call it after extracting emojiKey
and return if it fails (optionally send an error to the sender); when
broadcasting, catch exceptions but log them with context (session id, roomId,
target session id) instead of ignoring so failures are debuggable. Ensure the
payload only contains the validated/sanitized emojiKey and keep existing
slot/nickname handling.
| /* 전역 로그인 정보 주입 */ | ||
| window.loginUserId = "<%= (session.getAttribute("loginUserId") != null ? session.getAttribute("loginUserId") : "") %>"; | ||
| window.loginNickname = "<%= (session.getAttribute("loginNickname") != null ? session.getAttribute("loginNickname") : "") %>"; |
There was a problem hiding this comment.
XSS 취약점 주의
세션 속성값이 JavaScript 문자열로 직접 삽입됩니다. loginNickname에 </script> 또는 " 같은 문자가 포함되면 XSS 공격에 취약할 수 있습니다.
🔎 안전한 방법 제안
JSTL fn:escapeXml 또는 JSON 인코딩을 사용하세요:
+<%@ taglib prefix="fn" uri="http://java.sun.com/jsp/jstl/functions" %>
...
- window.loginUserId = "<%= (session.getAttribute("loginUserId") != null ? session.getAttribute("loginUserId") : "") %>";
- window.loginNickname = "<%= (session.getAttribute("loginNickname") != null ? session.getAttribute("loginNickname") : "") %>";
+ window.loginUserId = "${fn:escapeXml(sessionScope.loginUserId)}";
+ window.loginNickname = "${fn:escapeXml(sessionScope.loginNickname)}";Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/webapp/WEB-INF/views/game/multi.jsp around lines 80-82, session
values are directly injected into JavaScript which can allow XSS if
loginNickname contains quotes or </script>; instead escape or JSON-encode the
values before embedding. Replace the direct expressions with a safe encoder
(e.g., use JSTL fn:escapeXml or c:out for HTML-escaping and then JSON-encode, or
produce the values via a server-side JSON serializer /
StringEscapeUtils.escapeEcmaScript) and assign the encoded string to
window.loginNickname and window.loginUserId so that special characters are
properly escaped and cannot break out of the JS string literal.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/webapp/static/chat/emojiChatMulti.js
🔇 Additional comments (4)
src/main/webapp/static/chat/emojiChatMulti.js (4)
1-8: LGTM - 깔끔한 초기화 구조IIFE 패턴으로 스코프를 격리하고, 상태 표시 요소에 대한 방어적 null 체크가 잘 되어 있습니다.
10-12: LGTM - 슬롯 기반 카드 선택템플릿 리터럴을 사용한 속성 선택자가 명확하고 적절합니다.
14-29: LGTM - 견고한 말풍선 표시 로직카드와 말풍선 요소에 대한 방어적 체크가 잘 되어 있고, 타임아웃 정리 로직(
clearTimeout)도 적절히 구현되어 있습니다.
60-66: LGTM - 적절한 초기화 및 이벤트 바인딩전역 함수 노출과 이벤트 핸들러 바인딩이 깔끔하게 구현되어 있습니다.
querySelectorAll은 요소가 없어도 빈 NodeList를 반환하므로 안전합니다.
| function sendEmoji(key) { | ||
| const ws = window.singleWs; | ||
| if (!ws || ws.readyState !== WebSocket.OPEN) { | ||
| setStatus("EMOJI: WS 미연결"); | ||
| return; | ||
| } | ||
|
|
||
| const emoji = EMOJI_MAP[key] || key; | ||
|
|
||
| /* 내 슬롯 */ | ||
| showBubbleOnSlot(window.mySlot, emoji); | ||
|
|
||
| ws.send("EMOJI_CHAT:" + key); | ||
| } |
There was a problem hiding this comment.
window.mySlot 유효성 검증 추가 필요
Line 55에서 window.mySlot을 검증 없이 사용하고 있습니다. 만약 mySlot이 초기화되지 않았거나 undefined인 경우, 발신자의 슬롯에 이모지가 표시되지 않는 문제가 발생할 수 있습니다.
🔎 수정 제안
function sendEmoji(key) {
const ws = window.singleWs;
if (!ws || ws.readyState !== WebSocket.OPEN) {
setStatus("EMOJI: WS 미연결");
return;
}
+ if (window.mySlot === undefined || window.mySlot === null) {
+ setStatus("EMOJI: 슬롯 미설정");
+ return;
+ }
const emoji = EMOJI_MAP[key] || key;
/* 내 슬롯 */
showBubbleOnSlot(window.mySlot, emoji);
ws.send("EMOJI_CHAT:" + key);
}🤖 Prompt for AI Agents
In src/main/webapp/static/chat/emojiChatMulti.js around lines 45 to 58, the
function uses window.mySlot without validation which can cause silent failures
when mySlot is undefined; add a guard before calling showBubbleOnSlot — check
that window.mySlot is defined (and optionally of expected type/shape) and only
call showBubbleOnSlot when valid, otherwise log or setStatus indicating the
local slot is unavailable and continue sending the WebSocket message (or return
early if desired). Ensure the check avoids throwing and preserves existing WS
send behavior.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/main/webapp/WEB-INF/views/game/multi.jsp (2)
307-307: 스크립트 경로에 심각한 오류가 있습니다.현재 경로
/chat/emojiChat-multi.js는 세 가지 문제를 가지고 있습니다:
- 파일명 오류:
emojiChat-multi.js(존재하지 않음) →emojiChatMulti.js(실제 파일)- 경로 오류:
/chat/→/static/chat/(정확한 위치)- 컨텍스트 경로 누락: 애플리케이션이 루트가 아닌 경로에 배포될 경우 리소스를 찾을 수 없습니다.
- <script src="/chat/emojiChat-multi.js"></script> + <script src="${pageContext.request.contextPath}/static/chat/emojiChatMulti.js"></script>
gameEmoji.jsp와 같이 프로젝트 내 다른 JSP 파일들이 동일한 패턴을 사용하고 있습니다.
80-82: XSS 취약점이 여전히 존재합니다.세션 속성값이 JavaScript 문자열에 직접 삽입됩니다.
loginNickname또는loginUserId에</script>,",'같은 특수 문자가 포함되면 스크립트 컨텍스트를 벗어나 XSS 공격이 가능합니다.🔎 안전한 구현 방법
JSTL
fn:escapeXml을 사용하여 HTML/XML 이스케이프를 적용하세요:+<%@ taglib prefix="fn" uri="http://java.sun.com/jsp/jstl/functions" %> +<%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %> ... - window.loginUserId = "<%= (session.getAttribute("loginUserId") != null ? session.getAttribute("loginUserId") : "") %>"; - window.loginNickname = "<%= (session.getAttribute("loginNickname") != null ? session.getAttribute("loginNickname") : "") %>"; + window.loginUserId = "${fn:escapeXml(sessionScope.loginUserId != null ? sessionScope.loginUserId : '')}"; + window.loginNickname = "${fn:escapeXml(sessionScope.loginNickname != null ? sessionScope.loginNickname : '')}";또는 더 안전하게 JSON 인코딩을 사용하는 방법:
<%@ page import="com.fasterxml.jackson.databind.ObjectMapper" %> <% ObjectMapper mapper = new ObjectMapper(); String safeUserId = mapper.writeValueAsString(session.getAttribute("loginUserId") != null ? session.getAttribute("loginUserId") : ""); String safeNickname = mapper.writeValueAsString(session.getAttribute("loginNickname") != null ? session.getAttribute("loginNickname") : ""); %> <script> window.loginUserId = <%= safeUserId %>; window.loginNickname = <%= safeNickname %>; </script>
🧹 Nitpick comments (2)
src/main/webapp/WEB-INF/views/game/multi.jsp (2)
114-115: 혼란스러운 변수명을 개선하세요.단체전 페이지에서 WebSocket 인스턴스를
window.singleWs로 명명한 것은 혼란을 야기할 수 있습니다.emojiChatMulti.js가 이 변수를 참조하는 경우, 변수명을window.multiWs또는window.gameWs로 변경하는 것이 코드의 명확성을 높일 수 있습니다.🔎 제안하는 개선사항
- /* mojiChat.js가 window.singleWs를 사용 */ - window.singleWs = ws; + /* emojiChatMulti.js가 window.multiWs를 사용 */ + window.multiWs = ws;단,
emojiChatMulti.js에서도 동일하게window.multiWs를 참조하도록 수정해야 합니다.
231-250: 게임 종료 후 방 이동 로직이 구현되었습니다.게임 종료 후 서버에 상태를 업데이트하고 3초 후 방으로 이동하는 로직이 적절히 구현되었습니다. 에러 처리도 포함되어 있어 서버 호출 실패 시에도 사용자가 방으로 돌아갈 수 있습니다.
💡 선택적 개선사항
성공과 실패 케이스가 동일한 로직을 수행하므로 중복을 제거할 수 있습니다:
function goToRoomView() { try { ws.close(); } catch(e) {} 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 => { - setTimeout(() => { - location.href = contextPath + "/room?roomId=" + encodeURIComponent(roomId) + "&playType=1"; - }, 3000); - }) - .catch(err => { + .catch(err => console.warn("방 상태 업데이트 실패:", err)) + .finally(() => { 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 (1)
src/main/webapp/WEB-INF/views/game/multi.jsp
🔇 Additional comments (5)
src/main/webapp/WEB-INF/views/game/multi.jsp (5)
26-26: CSS 경로 수정 완료!이전 리뷰에서 지적된 컨텍스트 경로 누락 문제가 해결되었습니다.
${pageContext.request.contextPath}를 사용하여 경로를 올바르게 구성했습니다.
35-57: 플레이어 카드 구조가 올바르게 구현되었습니다.4개의 플레이어 카드가
data-slot속성과 위치 클래스를 사용하여 적절히 배치되었습니다. 각 카드는 프로필, 이름, 말풍선 영역을 포함하고 있어 이모티콘 채팅 표시에 필요한 구조를 갖추고 있습니다.
63-70: 이모티콘 버튼 UI가 적절히 구성되었습니다.이모티콘 버튼과 웹소켓 상태 표시 영역이 올바르게 추가되었습니다. 버튼의
data-emoji속성은 외부 스크립트에서 이벤트 핸들러를 연결하는 데 사용될 것으로 보입니다.
204-210: 이모지 채팅 통합 패턴이 올바릅니다.WebSocket 메시지 핸들러에서
window.onEmojiChat콜백을 확인하고 호출하는 방식은 외부 스크립트와의 느슨한 결합을 유지하면서도 효과적인 통합을 제공합니다.
252-269: 타이머 구현이 적절합니다.타이머 로직이 올바르게 구현되었으며,
clearInterval을 통한 적절한 정리와 5초 이하일 때 빨간색으로 표시하는 시각적 피드백이 포함되어 있습니다.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
3 participants
📌 작업 내용
🔗 관련 이슈
👀 리뷰 시 참고사항
💭 느낀 점
초기에 구조를 명확히 나누는 게 이후 기능 추가 시 충돌을 줄여준다는 점을 체감함
💻 스크린샷 (선택)
Summary by CodeRabbit
새로운 기능
UI 개선
수정
✏️ Tip: You can customize this high-level summary in your review settings.