[FEAT] 웹 소켓 기반 시그널링 서버 초안을 구축한다. #22
Conversation
Walkthrough이 변경 사항은 백엔드에 WebSocket 기반 실시간 시그널링 기능을 추가하고, JWT 인증을 WebSocket 핸드셰이크에 적용합니다. 새로운 핸들러, 메시지 모델, 인터셉터, 보안 및 WebSocket 설정 클래스가 도입되었으며, 기존 인증 필터와 JWT 유틸리티에 WebSocket 지원을 위한 예외 처리가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant WebSocketConfig
participant JwtHandshakeInterceptor
participant JwtTokenUtil
participant SignalingHandler
Client->>WebSocketConfig: /ws/signaling으로 WebSocket 연결 요청
WebSocketConfig->>JwtHandshakeInterceptor: 핸드셰이크 인터셉트
JwtHandshakeInterceptor->>JwtTokenUtil: JWT 토큰 추출 및 검증
JwtTokenUtil-->>JwtHandshakeInterceptor: 토큰 유효 여부 반환
JwtHandshakeInterceptor-->>WebSocketConfig: 허용/거부 결정
WebSocketConfig->>SignalingHandler: 연결 승인 시 세션 등록
Client->>SignalingHandler: 시그널링 메시지 전송(join/offer/answer 등)
SignalingHandler->>SignalingHandler: 메시지 파싱 및 처리, 브로드캐스트/전달
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (9)
backend/src/main/resources/http/auth.http (1)
11-11: 샘플 로그인 이메일 업데이트 확인
로그인 예시의 이메일이[email protected]으로 적절히 변경되었습니다.샘플 데이터가 여러 곳에 분산될 경우,
{{TEST_USER_EMAIL}}등의 환경 변수 플레이스홀더로 관리하여 일관성과 유지 보수성을 강화하는 것을 고려해 보세요.backend/src/main/resources/http/member.http (1)
6-7: 회원가입 샘플 데이터 일관성 확인
회원가입 예시의name과auth.http)와 일치하도록 변경되었습니다.동일한 테스트 유저 정보를 여러 파일에서 사용하면 유지 보수가 어려워질 수 있으니, 환경 변수 또는 전역 플레이스홀더(
{{TEST_USER_NAME}},{{TEST_USER_EMAIL}})로 관리하는 방안을 검토해 보세요.backend/src/main/java/com/focussu/backend/auth/filter/JwtAuthenticationFilter.java (1)
32-40: WebSocket 핸드셰이크 요청 제외 로직이 정확하게 구현되었습니다.WebSocket 핸드셰이크 요청을 정확히 식별하여 JWT 필터에서 제외하는 로직이 잘 구현되었습니다. HTTP 메서드, Upgrade 헤더, URI를 모두 확인하여 안전합니다.
주석을 더 명확하게 개선할 수 있습니다:
- // 1) WebSocket 핸드쉐이크는 GET + Upgrade:websocket - - // 2) 이 외에도 필요하다면 특정 경로를 추가로 제외할 수 있습니다. + // WebSocket 핸드셰이크 요청(GET + Upgrade:websocket + /ws/signaling)은 + // JWT 필터링에서 제외하여 별도의 핸드셰이크 인터셉터에서 처리backend/src/main/java/com/focussu/backend/signalling/WebSocketConfig.java (1)
28-31: 핸들러를 싱글톤으로 관리하는 것을 고려해보세요.현재 구현에서는 문제없지만, 향후 상태를 관리해야 하는 경우 싱글톤 패턴이 더 적절할 수 있습니다.
+ private final SignalingHandler signalingHandler = new SignalingHandler(); + @Bean public WebSocketHandler signalingHandler() { - return new SignalingHandler(); + return signalingHandler; }backend/src/main/java/com/focussu/backend/signalling/JwtHandshakeInterceptor.java (2)
50-56: 토큰 검증과 사용자 정보 추출을 최적화하세요.현재
validateToken과getUsernameFromToken을 각각 호출하여 토큰을 두 번 파싱합니다. 성능 개선을 위해 한 번에 처리하는 것을 고려해보세요.// 4) 검증 - if (token == null || !jwtTokenUtil.validateToken(token)) { + if (token == null) { response.setStatusCode(HttpStatus.UNAUTHORIZED); return false; } - attrs.put("userId", jwtTokenUtil.getUsernameFromToken(token)); + try { + String userId = jwtTokenUtil.getUsernameFromToken(token); + if (userId == null || !jwtTokenUtil.validateToken(token)) { + response.setStatusCode(HttpStatus.UNAUTHORIZED); + return false; + } + attrs.put("userId", userId); + } catch (Exception e) { + log.warn("[HandshakeInterceptor] 토큰 검증 실패: {}", e.getMessage()); + response.setStatusCode(HttpStatus.UNAUTHORIZED); + return false; + }
60-68: 일관된 로깅 방식을 사용하세요.
System.out.println대신 SLF4J logger를 사용하여 일관된 로깅 레벨과 포맷을 유지하세요.@Override public void afterHandshake(ServerHttpRequest request, ServerHttpResponse response, WebSocketHandler wsHandler, Exception exception) { - // 아무 처리 필요 없으면 빈 바디로 둡니다. - // // 또는 디버깅용: - System.out.println("WebSocket Handshake 완료: " + request.getRemoteAddress()); + log.debug("[HandshakeInterceptor] WebSocket Handshake 완료: {}", request.getRemoteAddress()); + if (exception != null) { + log.error("[HandshakeInterceptor] Handshake 중 오류 발생: {}", exception.getMessage()); + } }backend/src/main/java/com/focussu/backend/signalling/SignalingHandler.java (3)
19-26: 클래스 설계가 잘 되어있지만 ObjectMapper 설정을 고려해보세요.동시성을 고려한 ConcurrentHashMap 사용이 적절합니다. 다만 ObjectMapper의 보안 설정을 추가하는 것을 권장합니다.
ObjectMapper의 보안을 강화하려면 다음과 같이 설정하는 것을 고려해보세요:
- private final ObjectMapper mapper = new ObjectMapper(); + private final ObjectMapper mapper = new ObjectMapper() + .configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false) + .configure(JsonParser.Feature.STRICT_DUPLICATE_DETECTION, true);
116-121: 방 멤버십 검증 로직이 개선 가능합니다.현재 조건문이 복잡하고 가독성이 떨어집니다.
다음과 같이 가독성을 개선하세요:
WebSocketSession ws = sessions.get(to); - if (ws != null && ws.isOpen() && rooms.getOrDefault(roomId, Collections.emptySet()).contains(to)) { - ws.sendMessage(new TextMessage(mapper.writeValueAsString(msg))); + Set<String> roomMembers = rooms.getOrDefault(roomId, Collections.emptySet()); + + if (ws == null || !ws.isOpen()) { + log.warn("[Signaling] Target user {} session not available", to); + return; + } + + if (!roomMembers.contains(to)) { + log.warn("[Signaling] Target user {} not in room {}", to, roomId); + return; + } + + try { + ws.sendMessage(new TextMessage(mapper.writeValueAsString(msgCopy))); log.info("[Signaling] SEND to {} in room {}: {}", to, roomId, msg.getType()); - } else { - log.warn("[Signaling] Cannot send to {} (not in room or closed)", to); - } + } catch (IOException e) { + log.error("[Signaling] Failed to send message to {}: {}", to, e.getMessage()); + }
19-123: 전반적인 아키텍처 개선 제안사항들입니다.코드의 기본 구조는 좋지만 몇 가지 개선할 부분이 있습니다.
다음 사항들을 고려해보세요:
- 메모리 관리: 빈 방(room) 정리 로직 추가
- 성능 최적화: 대용량 방에 대한 브로드캐스트 최적화
- 보안: 방 접근 권한 검증
- 모니터링: 메트릭 수집 (연결 수, 메시지 수 등)
- 설정: 최대 방 크기, 사용자당 최대 방 수 등의 제한 설정
// 예시: 빈 방 정리 로직 private void cleanupEmptyRooms() { rooms.entrySet().removeIf(entry -> entry.getValue().isEmpty()); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
backend/build.gradle(1 hunks)backend/src/main/java/com/focussu/backend/auth/filter/JwtAuthenticationFilter.java(1 hunks)backend/src/main/java/com/focussu/backend/auth/util/JwtTokenUtil.java(1 hunks)backend/src/main/java/com/focussu/backend/config/SecurityConfig.java(1 hunks)backend/src/main/java/com/focussu/backend/signalling/JwtHandshakeInterceptor.java(1 hunks)backend/src/main/java/com/focussu/backend/signalling/SignalingHandler.java(1 hunks)backend/src/main/java/com/focussu/backend/signalling/SignalingMessage.java(1 hunks)backend/src/main/java/com/focussu/backend/signalling/WebSecurityIgnoreConfig.java(1 hunks)backend/src/main/java/com/focussu/backend/signalling/WebSocketConfig.java(1 hunks)backend/src/main/resources/http/auth.http(1 hunks)backend/src/main/resources/http/member.http(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
backend/src/main/java/com/focussu/backend/signalling/WebSecurityIgnoreConfig.java (1)
backend/src/main/java/com/focussu/backend/signalling/WebSocketConfig.java (1)
Configuration(11-32)
backend/src/main/java/com/focussu/backend/signalling/WebSocketConfig.java (1)
backend/src/main/java/com/focussu/backend/signalling/WebSecurityIgnoreConfig.java (1)
Configuration(7-14)
🔇 Additional comments (5)
backend/build.gradle (1)
50-52:Details
❓ Verification inconclusive
중복 의존성 확인이 필요합니다.
Jackson Databind 의존성을 명시적으로 추가했지만, Spring Boot Web 스타터에 이미 포함되어 있을 수 있습니다. 불필요한 중복 의존성을 방지하기 위해 확인이 필요합니다.
다음 스크립트를 실행하여 의존성 중복을 확인해주세요:
🏁 Script executed:
#!/bin/bash # 설명: Spring Boot에 포함된 Jackson 의존성 확인 # Jackson이 이미 포함되어 있는지 확인 # Spring Boot 스타터들이 가져오는 의존성 확인 echo "Checking if Jackson is already included in Spring Boot starters..." # gradle dependencies로 의존성 트리 확인 (실제 프로젝트에서 실행 필요) echo "Run 'gradle dependencies' to check dependency tree" echo "Look for 'com.fasterxml.jackson.core:jackson-databind' in the output"Length of output: 412
의존성 중복 여부 검토 필요
build.gradle에 명시한com.fasterxml.jackson.core:jackson-databind이 Spring Boot 스타터 웹(WebSocket 포함)을 통해 이미 포함될 수 있으니 중복 여부를 확인해 주세요.
- 위치:
backend/build.gradleLines 50-52- 확인 방법:
./gradlew dependencies --configuration compileClasspath \ | grep com.fasterxml.jackson.core:jackson-databind중복으로 확인되면 불필요한 의존성을 제거하거나 버전을 통일해 주세요.
backend/src/main/java/com/focussu/backend/auth/util/JwtTokenUtil.java (1)
86-95: WebSocket 인증을 위한 간소화된 토큰 검증 메서드가 잘 구현되었습니다.UserDetails 없이 토큰만으로 유효성을 검증하는 메서드가 WebSocket 핸드셰이크 시나리오에 적합하게 구현되었습니다. 기존 메서드들을 재사용하여 일관성을 유지하고 있으며, 예외 처리도 적절합니다.
backend/src/main/java/com/focussu/backend/config/SecurityConfig.java (1)
106-106:Details
✅ Verification successful
WebSocket 엔드포인트 보안 설정을 확인해주세요.
/ws/signaling경로를permitAll()로 설정했는데, 이는 HTTP 핸드셰이크 단계에서는 인증을 요구하지 않겠다는 의미입니다. 실제 WebSocket 인증은 핸드셰이크 인터셉터에서 처리되는 것으로 보이지만, 보안상 이 접근법이 적절한지 확인이 필요합니다.다음 사항들을 확인해주세요:
- JwtHandshakeInterceptor가 제대로 구현되어 있는지
- 인증되지 않은 사용자의 WebSocket 연결 시도가 차단되는지
- 취약점이 없는지 보안 테스트가 필요합니다
🏁 Script executed:
#!/bin/bash # WebSocket 핸드셰이크 인터셉터 구현 확인 rg -A 10 "JwtHandshakeInterceptor" --type javaLength of output: 2616
🏁 Script executed:
#!/bin/bash # JWT 핸드셰이크 인터셉터 전체 구현 확인 cat backend/src/main/java/com/focussu/backend/signalling/JwtHandshakeInterceptor.javaLength of output: 2597
WebSocket 핸드셰이크 인증 처리 확인 완료
JwtHandshakeInterceptor의 beforeHandshake() 메서드에서 Authorization 헤더 또는 쿼리 파라미터로 전달된 토큰을 검증하고, 유효하지 않을 경우 HTTP 401 응답과 함께 false를 반환하므로
/ws/signaling을permitAll()로 설정한 뒤에도 인증되지 않은 연결은 모두 차단됩니다.
현재 설정은 적절하니 추가 조치가 필요 없습니다.backend/src/main/java/com/focussu/backend/signalling/WebSecurityIgnoreConfig.java (1)
7-14: WebSocket 엔드포인트를 위한 적절한 보안 설정입니다.WebSocket 핸드셰이크 요청이 Spring Security 필터 체인을 우회하도록 하는 설정이 올바르게 구현되었습니다.
JwtHandshakeInterceptor에서 실제 JWT 인증을 처리하므로 이 구성이 적절합니다.backend/src/main/java/com/focussu/backend/signalling/SignalingMessage.java (1)
8-21: WebRTC 시그널링을 위한 잘 설계된 메시지 모델입니다.Lombok 어노테이션을 적절히 활용하여 간결한 코드를 작성했고, JsonNode를 사용한 유연한 페이로드 처리와 부분 생성자 제공이 좋습니다. WebRTC 시그널링 요구사항에 적합한 구조입니다.
| registry | ||
| .addHandler(signalingHandler(), "/ws/signaling") | ||
| .setAllowedOrigins("*") | ||
| .addInterceptors(new JwtHandshakeInterceptor(jwtTokenUtil)); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
CORS 설정을 프로덕션 환경에 맞게 제한하세요.
setAllowedOrigins("*")는 개발 환경에서는 편리하지만 프로덕션에서는 보안 위험이 있습니다. 실제 프론트엔드 도메인으로 제한하는 것을 권장합니다.
- .setAllowedOrigins("*")
+ .setAllowedOrigins("https://yourdomain.com", "https://localhost:3000")📝 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.
| registry | |
| .addHandler(signalingHandler(), "/ws/signaling") | |
| .setAllowedOrigins("*") | |
| .addInterceptors(new JwtHandshakeInterceptor(jwtTokenUtil)); | |
| } | |
| registry | |
| .addHandler(signalingHandler(), "/ws/signaling") | |
| .setAllowedOrigins("https://yourdomain.com", "https://localhost:3000") | |
| .addInterceptors(new JwtHandshakeInterceptor(jwtTokenUtil)); | |
| } |
🤖 Prompt for AI Agents
In backend/src/main/java/com/focussu/backend/signalling/WebSocketConfig.java
around lines 22 to 26, the CORS setting uses setAllowedOrigins("*"), which is
insecure for production. Replace the wildcard "*" with the specific frontend
domain(s) allowed to connect, ensuring only trusted origins can access the
WebSocket endpoint.
| // 3) 로깅 | ||
| log.info("[HandshakeInterceptor] 요청 URI: {}", request.getURI()); | ||
| log.info("[HandshakeInterceptor] 토큰: {}", token); | ||
|
|
There was a problem hiding this comment.
보안상 민감한 토큰 정보를 로그에 기록하지 마세요.
JWT 토큰을 로그에 기록하는 것은 보안 위험을 초래할 수 있습니다. 토큰 존재 여부만 로깅하거나 마스킹 처리를 권장합니다.
// 3) 로깅
log.info("[HandshakeInterceptor] 요청 URI: {}", request.getURI());
- log.info("[HandshakeInterceptor] 토큰: {}", token);
+ log.info("[HandshakeInterceptor] 토큰 존재 여부: {}", token != null);📝 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.
| // 3) 로깅 | |
| log.info("[HandshakeInterceptor] 요청 URI: {}", request.getURI()); | |
| log.info("[HandshakeInterceptor] 토큰: {}", token); | |
| // 3) 로깅 | |
| log.info("[HandshakeInterceptor] 요청 URI: {}", request.getURI()); | |
| log.info("[HandshakeInterceptor] 토큰 존재 여부: {}", token != null); |
🤖 Prompt for AI Agents
In
backend/src/main/java/com/focussu/backend/signalling/JwtHandshakeInterceptor.java
around lines 45 to 48, the JWT token is being logged in plain text, which poses
a security risk. Modify the logging to avoid printing the full token; instead,
log only whether the token exists or mask part of the token to prevent exposing
sensitive information.
| String userId = (String) session.getAttributes().get("userId"); | ||
| sessions.remove(userId); | ||
| rooms.values().forEach(m -> m.remove(userId)); | ||
| log.info("[Signaling] DISCONNECTED: {}", userId); |
There was a problem hiding this comment.
연결 종료 시 userId null 체크가 필요합니다.
연결 종료 시에도 userId가 null일 가능성을 고려해야 합니다.
다음과 같이 null 체크를 추가하세요:
String userId = (String) session.getAttributes().get("userId");
+ if (userId == null) {
+ log.warn("[Signaling] Cannot cleanup: userId not found in session attributes");
+ return;
+ }
sessions.remove(userId);📝 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.
| String userId = (String) session.getAttributes().get("userId"); | |
| sessions.remove(userId); | |
| rooms.values().forEach(m -> m.remove(userId)); | |
| log.info("[Signaling] DISCONNECTED: {}", userId); | |
| String userId = (String) session.getAttributes().get("userId"); | |
| if (userId == null) { | |
| log.warn("[Signaling] Cannot cleanup: userId not found in session attributes"); | |
| return; | |
| } | |
| sessions.remove(userId); | |
| rooms.values().forEach(m -> m.remove(userId)); | |
| log.info("[Signaling] DISCONNECTED: {}", userId); |
🤖 Prompt for AI Agents
In backend/src/main/java/com/focussu/backend/signalling/SignalingHandler.java
around lines 80 to 83, add a null check for userId before using it. The current
code assumes userId is always non-null, but it can be null on connection
termination. Fix this by checking if userId is not null before removing it from
sessions and rooms, and before logging the disconnection event.
| String userId = (String) session.getAttributes().get("userId"); | ||
| sessions.put(userId, session); |
There was a problem hiding this comment.
userId null 체크 추가가 필요합니다.
세션 속성에서 userId를 가져올 때 null일 가능성을 고려해야 합니다.
다음과 같이 null 체크를 추가하세요:
- String userId = (String) session.getAttributes().get("userId");
- sessions.put(userId, session);
+ String userId = (String) session.getAttributes().get("userId");
+ if (userId == null) {
+ log.warn("[Signaling] Connection rejected: userId not found in session attributes");
+ session.close();
+ return;
+ }
+ sessions.put(userId, session);📝 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.
| String userId = (String) session.getAttributes().get("userId"); | |
| sessions.put(userId, session); | |
| String userId = (String) session.getAttributes().get("userId"); | |
| if (userId == null) { | |
| log.warn("[Signaling] Connection rejected: userId not found in session attributes"); | |
| session.close(); | |
| return; | |
| } | |
| sessions.put(userId, session); |
🤖 Prompt for AI Agents
In backend/src/main/java/com/focussu/backend/signalling/SignalingHandler.java
around lines 29 to 30, add a null check for userId after retrieving it from
session attributes to prevent potential NullPointerException. If userId is null,
handle it appropriately, such as by skipping adding to sessions or logging a
warning, before calling sessions.put(userId, session).
| protected void handleTextMessage(WebSocketSession session, TextMessage msg) throws IOException { | ||
| SignalingMessage in = mapper.readValue(msg.getPayload(), SignalingMessage.class); | ||
| String from = (String) session.getAttributes().get("userId"); | ||
| String roomId = in.getRoomId(); | ||
| log.info("[Signaling] [{}] {} ▶ {}", roomId, from, in.getType()); |
There was a problem hiding this comment.
JSON 파싱 예외 처리 및 입력 검증이 필요합니다.
메시지 파싱 시 예외 처리와 기본적인 검증이 부족합니다.
다음과 같이 예외 처리와 검증을 추가하세요:
- SignalingMessage in = mapper.readValue(msg.getPayload(), SignalingMessage.class);
- String from = (String) session.getAttributes().get("userId");
- String roomId = in.getRoomId();
+ SignalingMessage in;
+ try {
+ in = mapper.readValue(msg.getPayload(), SignalingMessage.class);
+ } catch (IOException e) {
+ log.warn("[Signaling] Invalid message format from {}: {}", session.getId(), e.getMessage());
+ return;
+ }
+
+ String from = (String) session.getAttributes().get("userId");
+ if (from == null) {
+ log.warn("[Signaling] No userId in session attributes");
+ return;
+ }
+
+ String roomId = in.getRoomId();
+ if (roomId == null || roomId.trim().isEmpty()) {
+ log.warn("[Signaling] Invalid roomId from {}", from);
+ return;
+ }📝 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.
| protected void handleTextMessage(WebSocketSession session, TextMessage msg) throws IOException { | |
| SignalingMessage in = mapper.readValue(msg.getPayload(), SignalingMessage.class); | |
| String from = (String) session.getAttributes().get("userId"); | |
| String roomId = in.getRoomId(); | |
| log.info("[Signaling] [{}] {} ▶ {}", roomId, from, in.getType()); | |
| protected void handleTextMessage(WebSocketSession session, TextMessage msg) throws IOException { | |
| - SignalingMessage in = mapper.readValue(msg.getPayload(), SignalingMessage.class); | |
| - String from = (String) session.getAttributes().get("userId"); | |
| - String roomId = in.getRoomId(); | |
| + SignalingMessage in; | |
| + try { | |
| + in = mapper.readValue(msg.getPayload(), SignalingMessage.class); | |
| + } catch (IOException e) { | |
| + log.warn("[Signaling] Invalid message format from {}: {}", session.getId(), e.getMessage()); | |
| + return; | |
| + } | |
| + | |
| + String from = (String) session.getAttributes().get("userId"); | |
| + if (from == null) { | |
| + log.warn("[Signaling] No userId in session attributes"); | |
| + return; | |
| + } | |
| + | |
| + String roomId = in.getRoomId(); | |
| + if (roomId == null || roomId.trim().isEmpty()) { | |
| + log.warn("[Signaling] Invalid roomId from {}", from); | |
| + return; | |
| + } | |
| log.info("[Signaling] [{}] {} ▶ {}", roomId, from, in.getType()); |
🤖 Prompt for AI Agents
In backend/src/main/java/com/focussu/backend/signalling/SignalingHandler.java
around lines 35 to 39, the JSON parsing of the incoming message lacks exception
handling and input validation. Wrap the mapper.readValue call in a try-catch
block to handle potential parsing exceptions, and add checks to validate that
the parsed SignalingMessage and its critical fields like roomId and type are not
null or invalid before proceeding. Log or handle errors appropriately to prevent
the method from failing silently or crashing.
| if (msg.getPayload() == null) { | ||
| msg.setPayload(mapper.createObjectNode()); | ||
| } | ||
| ((ObjectNode) msg.getPayload()).put("from", from); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
sendTo 메서드에서도 동일한 메시지 변조 문제가 있습니다.
broadcast 메서드와 동일한 문제입니다.
동일하게 메시지를 복사한 후 수정하도록 변경하세요:
- if (msg.getPayload() == null) {
- msg.setPayload(mapper.createObjectNode());
- }
- ((ObjectNode) msg.getPayload()).put("from", from);
+ SignalingMessage msgCopy = new SignalingMessage(msg.getType(), msg.getRoomId(), msg.getTo(),
+ msg.getPayload() != null ? msg.getPayload().deepCopy() : mapper.createObjectNode());
+ ((ObjectNode) msgCopy.getPayload()).put("from", from);📝 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.
| if (msg.getPayload() == null) { | |
| msg.setPayload(mapper.createObjectNode()); | |
| } | |
| ((ObjectNode) msg.getPayload()).put("from", from); | |
| SignalingMessage msgCopy = new SignalingMessage( | |
| msg.getType(), | |
| msg.getRoomId(), | |
| msg.getTo(), | |
| msg.getPayload() != null | |
| ? msg.getPayload().deepCopy() | |
| : mapper.createObjectNode() | |
| ); | |
| ((ObjectNode) msgCopy.getPayload()).put("from", from); |
🤖 Prompt for AI Agents
In backend/src/main/java/com/focussu/backend/signalling/SignalingHandler.java
around lines 110 to 113, the sendTo method modifies the original message payload
directly, causing unintended side effects similar to the broadcast method. To
fix this, create a copy of the message payload before making any modifications,
then update the copy with the "from" field and use this modified copy instead of
altering the original message.
| if (msg.getPayload() == null) { | ||
| msg.setPayload(mapper.createObjectNode()); | ||
| } | ||
| ((ObjectNode) msg.getPayload()).put("from", sender); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
메시지 객체 변조 문제가 있습니다.
원본 메시지 객체의 payload를 직접 수정하고 있어 예상치 못한 부작용이 발생할 수 있습니다.
메시지를 복사한 후 수정하도록 변경하세요:
- if (msg.getPayload() == null) {
- msg.setPayload(mapper.createObjectNode());
- }
- ((ObjectNode) msg.getPayload()).put("from", sender);
+ SignalingMessage msgCopy = new SignalingMessage(msg.getType(), msg.getRoomId(), msg.getTo(),
+ msg.getPayload() != null ? msg.getPayload().deepCopy() : mapper.createObjectNode());
+ ((ObjectNode) msgCopy.getPayload()).put("from", sender);그리고 이후 코드에서 msg 대신 msgCopy를 사용하세요.
📝 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.
| if (msg.getPayload() == null) { | |
| msg.setPayload(mapper.createObjectNode()); | |
| } | |
| ((ObjectNode) msg.getPayload()).put("from", sender); | |
| SignalingMessage msgCopy = new SignalingMessage( | |
| msg.getType(), | |
| msg.getRoomId(), | |
| msg.getTo(), | |
| msg.getPayload() != null | |
| ? msg.getPayload().deepCopy() | |
| : mapper.createObjectNode() | |
| ); | |
| ((ObjectNode) msgCopy.getPayload()).put("from", sender); |
🤖 Prompt for AI Agents
In backend/src/main/java/com/focussu/backend/signalling/SignalingHandler.java
around lines 90 to 93, the code directly modifies the original message object's
payload, which can cause unintended side effects. To fix this, create a deep
copy of the message object before making any changes, then modify the payload on
this copied object. Replace all subsequent uses of the original message variable
with the new copied message variable to avoid mutating the original message.
| Set<String> members = rooms.getOrDefault(roomId, Collections.emptySet()); | ||
| members.remove(from); | ||
| // (2) peer-left 방송 | ||
| ObjectNode leavePayload = mapper.createObjectNode().put("from", from); | ||
| SignalingMessage leftMsg = new SignalingMessage("peer-left", roomId, null, leavePayload); | ||
| broadcast(roomId, leftMsg, from); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
동시성 문제 가능성이 있습니다.
방에서 사용자를 제거할 때 동시성 문제가 발생할 수 있습니다.
다음과 같이 안전하게 수정하세요:
- Set<String> members = rooms.getOrDefault(roomId, Collections.emptySet());
- members.remove(from);
+ rooms.computeIfPresent(roomId, (k, members) -> {
+ members.remove(from);
+ return members.isEmpty() ? null : members;
+ });📝 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.
| Set<String> members = rooms.getOrDefault(roomId, Collections.emptySet()); | |
| members.remove(from); | |
| // (2) peer-left 방송 | |
| ObjectNode leavePayload = mapper.createObjectNode().put("from", from); | |
| SignalingMessage leftMsg = new SignalingMessage("peer-left", roomId, null, leavePayload); | |
| broadcast(roomId, leftMsg, from); | |
| rooms.computeIfPresent(roomId, (k, members) -> { | |
| members.remove(from); | |
| return members.isEmpty() ? null : members; | |
| }); | |
| // (2) peer-left 방송 | |
| ObjectNode leavePayload = mapper.createObjectNode().put("from", from); | |
| SignalingMessage leftMsg = new SignalingMessage("peer-left", roomId, null, leavePayload); | |
| broadcast(roomId, leftMsg, from); |
🤖 Prompt for AI Agents
In backend/src/main/java/com/focussu/backend/signalling/SignalingHandler.java
around lines 53 to 58, the code removes a user from the room's member set
without synchronization, which can cause concurrency issues. To fix this, ensure
thread-safe access by synchronizing on the room's member set or using a
concurrent collection when modifying the members set. This will prevent
concurrent modification exceptions and maintain data consistency.
Resolved Issue #6
Summary by CodeRabbit
신규 기능
버그 수정
문서