Skip to content

XSS 방지#55

Open
pdh0128 wants to merge 1 commit intodev-0.2from
feature-0.2.2/xss-prevention/PW-364
Open

XSS 방지#55
pdh0128 wants to merge 1 commit intodev-0.2from
feature-0.2.2/xss-prevention/PW-364

Conversation

@pdh0128
Copy link
Member

@pdh0128 pdh0128 commented Nov 20, 2025

XSS 방지

Summary by CodeRabbit

릴리스 노트

  • 기타 개선
    • 애플리케이션 내부 기반 구조 개선

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 20, 2025

🐰 코드 리뷰 요약

Walkthrough

두 개의 새로운 자바 클래스가 추가되었습니다. JacksonConfig는 설정 패키지에, 클래스 a는 역직렬화 패키지에 추가되었으나, 후자는 파일명과 클래스명이 일치하지 않습니다.

Changes

Cohort / File(s) 변경 사항
설정 클래스
src/main/java/com/example/auth/global/config/JacksonConfig.java
새로운 공개 클래스 JacksonConfig 추가 (빈 본문)
역직렬화 클래스
src/main/java/com/example/auth/global/deserializer/XssStringJsonDeserializer.java
새로운 공개 클래스 a 추가 (파일명과 클래스명 불일치 - 명명 규칙 위반)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 분

  • 주의 사항:
    • XssStringJsonDeserializer.java 파일의 공개 클래스명이 a로 되어 있으나, 파일명과 일치하지 않습니다. 자바 명명 규칙을 준수하여 클래스명을 XssStringJsonDeserializer로 변경해야 합니다.
    • 두 클래스 모두 구현 내용이 비어있으므로, 의도된 기능이 누락되었는지 확인이 필요합니다.

Poem

🐰 새로운 클래스들이 도착했네요,
하나는 설정, 하나는 직렬화...
하지만 이름이 좀 이상한데요?
파일명과 클래스명을 맞춰주세요!
완성된 코드를 기대하며 기다립니다. 🌟

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive PR 제목은 'XSS 방지'이며, 추가된 파일들(JacksonConfig.java, XssStringJsonDeserializer.java)의 목적과 관련이 있으나, 실제 구현 내용은 비어있는 클래스들이므로 제목과 실제 변경 사항 간의 불일치가 존재합니다. PR 제목은 의도를 나타내지만, 실제 구현된 코드가 없어 구체적인 변경 사항을 명확히 설명하는 더 상세한 제목이나 설명이 필요합니다.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature-0.2.2/xss-prevention/PW-364

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 88d70eb and 14301d4.

📒 Files selected for processing (2)
  • src/main/java/com/example/auth/global/config/JacksonConfig.java (1 hunks)
  • src/main/java/com/example/auth/global/deserializer/XssStringJsonDeserializer.java (1 hunks)

Comment on lines +3 to +4
public class JacksonConfig {
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

빈 클래스로는 XSS 방지 기능이 구현되지 않습니다.

이 클래스는 현재 비어있어 XSS 방지 기능을 제공하지 않습니다. PR 목적이 "XSS 방지"인 것을 고려하면, 다음 사항들이 필요합니다:

  • @Configuration 어노테이션 추가
  • ObjectMapper 빈 정의 및 커스텀 XSS deserializer 등록
  • XssStringJsonDeserializer를 Jackson 모듈에 등록

다음과 같은 구현을 제안합니다:

 package com.example.auth.global.config;
 
+import com.example.auth.global.deserializer.XssStringJsonDeserializer;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.module.SimpleModule;
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+@Configuration
 public class JacksonConfig {
+    @Bean
+    public ObjectMapper objectMapper() {
+        ObjectMapper mapper = new ObjectMapper();
+        SimpleModule module = new SimpleModule();
+        module.addDeserializer(String.class, new XssStringJsonDeserializer());
+        mapper.registerModule(module);
+        return mapper;
+    }
 }

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +3 to +4
public class a {
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

컴파일 오류: 파일명과 클래스명이 일치하지 않습니다.

Java에서 public 클래스의 이름은 반드시 파일명과 일치해야 합니다. 현재 파일명은 XssStringJsonDeserializer.java인데 클래스명이 a로 되어 있어 컴파일되지 않습니다.

또한, XSS 방지를 위한 실제 구현 로직이 없습니다. 이 클래스는:

  • JsonDeserializer<String>을 상속해야 합니다
  • XSS 공격을 방지하기 위한 문자열 sanitization 로직을 구현해야 합니다

다음과 같이 수정하세요:

 package com.example.auth.global.deserializer;
 
-public class a {
+import com.fasterxml.jackson.core.JsonParser;
+import com.fasterxml.jackson.databind.DeserializationContext;
+import com.fasterxml.jackson.databind.JsonDeserializer;
+import org.apache.commons.text.StringEscapeUtils;
+import java.io.IOException;
+
+public class XssStringJsonDeserializer extends JsonDeserializer<String> {
+    @Override
+    public String deserialize(JsonParser p, DeserializationContext ctxt) throws IOException {
+        String value = p.getValueAsString();
+        if (value == null) {
+            return null;
+        }
+        // XSS 방지를 위해 HTML 특수문자를 이스케이프 처리
+        return StringEscapeUtils.escapeHtml4(value);
+    }
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In
src/main/java/com/example/auth/global/deserializer/XssStringJsonDeserializer.java
around lines 3 to 4, the file contains a public class named `a` which causes a
compile error because the public class name must match the file name; replace it
with a public class named `XssStringJsonDeserializer` that extends
`com.fasterxml.jackson.databind.JsonDeserializer<String>`, implement the
deserialize(JsonParser, DeserializationContext) method to read the incoming
string, handle nulls, and apply XSS sanitization (e.g., strip/escape HTML tags
or use a trusted sanitizer library) before returning the cleaned string; also
add necessary imports and consider registering/annotating the deserializer where
needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant