Skip to content

3주차 미션 / 서버 4조 이정언#14

Open
Jung-eon1018 wants to merge 14 commits intoKonkuk-KUIT:mainfrom
Jung-eon1018:Jung-eon1018
Open

3주차 미션 / 서버 4조 이정언#14
Jung-eon1018 wants to merge 14 commits intoKonkuk-KUIT:mainfrom
Jung-eon1018:Jung-eon1018

Conversation

@Jung-eon1018
Copy link

@Jung-eon1018 Jung-eon1018 commented Oct 3, 2025

Summary by CodeRabbit

  • New Features
    • HTTP 요청 파싱/응답 생성(상태코드, 헤더, 쿠키, 리다이렉트) 추가
    • 라우터로 경로별 컨트롤러 매핑
    • 정적 리소스 서빙 및 / → /index.html 포워드
    • 회원가입·로그인·사용자 목록 라우트 추가, 로그인 시 쿠키 설정
  • Refactor
    • 서버 요청 처리 흐름을 라우팅 기반으로 정리
    • 회원가입 폼 전송 방식을 POST로 변경
    • index.html에 Bootstrap JS와 로컬 스크립트 로드
  • Documentation
    • 프로젝트 README 추가
  • Tests
    • 요청/응답 동작 단위 테스트 및 샘플 요청 리소스 추가
  • Chores
    • Java 21 도구체인 설정 및 테스트 의존성 업데이트
    • IDE VCS/Gradle 설정 추가

@coderabbitai
Copy link

coderabbitai bot commented Oct 3, 2025

Walkthrough

HTTP 서버 구성 요소가 대거 추가·개편되어 라우팅 기반 처리 흐름을 도입했다. 요청 파싱(HttpRequest), 응답 조립(HttpResponse), 라우터/컨트롤러 계층, 관련 enum과 테스트 리소스가 추가됐다. 여러 패키지 명이 main.java.*로 변경됐고, 빌드/IDE 설정과 웹앱 정적 자원 일부가 업데이트됐다.

Changes

Cohort / File(s) Summary of Changes
IDE 설정
.idea/gradle.xml, .idea/vcs.xml
GradleMigrationSettings 추가 및 Git VCS 매핑 추가.
문서
README.md
프로젝트 소개/기능/스택 문서 추가.
빌드 설정
build.gradle
Java toolchain 21 지정, JUnit 의존성 명시(5.10.2), AssertJ 추가.
패키지 리네임: 모델/저장소
src/main/java/.../model/User.java, src/main/java/.../db/Repository.java, src/main/java/.../db/MemoryUserRepository.java
패키지 및 관련 import를 main.java.*로 변경. 로직 변경 없음.
패키지 리네임: 유틸/서버
src/main/java/.../http/util/HttpRequestUtils.java, src/main/java/.../http/util/IOUtils.java, src/main/java/.../webserver/WebServer.java
패키지를 main.java.*로 변경. 동작 동일.
요청/응답 핵심
src/main/java/http/HttpRequest.java, src/main/java/http/HttpResponse.java, src/main/java/http/enums/*
HTTP 요청 파서, 응답 작성기, HttpMethod/HttpStatus/HttpHeader/QueryKey enum 신설.
라우팅/컨트롤러
src/main/java/http/controller/Controller.java, .../Route.java, .../Router.java, .../Routes.java, .../StaticResourceController.java, .../UserListController.java, .../UserLoginController.java, .../UserSignupController.java
컨트롤러 인터페이스, 라우트/라우터, 기본 라우트 구성 및 정적/사용자 관련 컨트롤러 추가.
요청 처리 리팩터링
src/main/java/webserver/RequestHandler.java
라우터 기반 흐름으로 전환: HttpRequest 파싱 → Router 해석 → Controller 실행 → HttpResponse 전송. 기존 직접 응답 작성 삭제.
테스트 코드
src/test/java/http/HttpRequestTest.java, src/test/java/http/HttpResponseTest.java, src/test/java/http/TestUtil.java
요청/응답 파싱·전송 동작 검증용 단위 테스트 및 테스트 유틸 추가.
테스트 리소스
src/test/resources/http/*
예제 요청 텍스트 3종 추가(GET 쿼리, POST 폼, 쿠키).
웹앱 리소스
webapp/index.html, webapp/index_test.html, webapp/user/form.html
index에 Bootstrap JS/CDN 및 scripts.js 로드 추가, 테스트용 HTML 추가, 회원가입 폼 메서드 GET→POST 변경.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant WS as WebServer
  participant RH as RequestHandler
  participant Req as HttpRequest
  participant R as Router
  participant Ctrl as Controller
  participant Res as HttpResponse

  C->>WS: TCP connect
  WS->>RH: accept(socket)
  RH->>Req: from(BufferedReader)
  Req-->>RH: parsed method/path/headers/body
  RH->>R: resolve(method, path)
  alt Route found
    R-->>RH: Controller
    RH->>Ctrl: execute(request, response)
    Ctrl->>Res: forward/redirect/set headers
  else No match
    R-->>RH: fallback Controller
    RH->>Ctrl: execute(request, response)
  end
  Res-->>C: HTTP/1.1 status + headers + body
  C-->>WS: close
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • chore: readme추가 #10: 동일한 IDE 설정 파일과 README 추가가 중복되어 구성 파일 변경과 문서화 측면에서 연관됨.

Poem

귀 쫑긋, 라우터 깡총, 길을 정리해
Request는 점프, Response는 쾌—
쿠키 한 조각, 리다이렉트 두 번
정적도 척척, 가입도 금세!
burrow 속 서버는 오늘도 OK 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 1.49% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive 제목 “3주차 미션 / 서버 4조 이정언”은 과제 식별에는 도움이 되지만 PR에 포함된 구체적인 변경 사항이나 주요 기능을 설명하지 않아 PR 내용을 한눈에 파악하기 어렵습니다. PR에 포함된 핵심 변경 사항(예: HTTP 요청/응답 처리 및 라우터 구현, 컨트롤러 추가 등)을 간결하고 명확하게 반영하는 제목으로 수정해 주세요.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

@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: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/java/http/util/HttpRequestUtils.java (1)

9-19: 쿼리 파라미터 파싱 시 배열 인덱스 검증이 필요합니다.

Line 15에서 queries[1]에 접근할 때, 쿼리 파라미터가 값이 없는 경우(예: key= 또는 key)를 처리하지 않아 ArrayIndexOutOfBoundsException이 발생할 수 있습니다. 현재는 catch (Exception e)로 모든 예외를 잡아 빈 맵을 반환하지만, 이는 너무 광범위하여 다른 버그를 숨길 수 있습니다.

다음과 같이 개선할 수 있습니다:

 public static Map<String, String> parseQueryParameter(String queryString) {
-    try {
-        String[] queryStrings = queryString.split("&");
-
-        return Arrays.stream(queryStrings)
-                .map(q -> q.split("="))
-                .collect(Collectors.toMap(queries -> queries[0], queries -> queries[1]));
-    } catch (Exception e) {
-        return new HashMap<>();
-    }
+    if (queryString == null || queryString.isEmpty()) {
+        return new HashMap<>();
+    }
+    
+    String[] queryStrings = queryString.split("&");
+    
+    return Arrays.stream(queryStrings)
+            .map(q -> q.split("=", 2))
+            .filter(queries -> queries.length == 2)
+            .collect(Collectors.toMap(
+                queries -> queries[0], 
+                queries -> queries[1],
+                (v1, v2) -> v1  // 중복 키 처리
+            ));
 }

주요 개선 사항:

  • split("=", 2)로 최대 2개로만 분할하여 값에 =가 포함된 경우 처리
  • filter로 유효한 key-value 쌍만 처리
  • 중복 키 처리를 위한 merge function 추가
  • 불필요한 광범위한 예외 처리 제거
♻️ Duplicate comments (1)
src/main/java/http/controller/UserLoginController.java (1)

20-20: 취약한 세션 관리

logined=true 쿠키는 클라이언트가 쉽게 조작할 수 있어 인증을 우회할 수 있습니다. UserListController에서도 동일한 문제가 발견되었습니다.

서버 측 세션 관리를 구현하세요:

  • 세션 ID 생성 (UUID 또는 SecureRandom)
  • 서버 측 세션 저장소에 userId 매핑
  • HttpOnly, Secure 플래그가 설정된 쿠키 사용
String sessionId = UUID.randomUUID().toString();
SessionManager.getInstance().createSession(sessionId, userId);
response.redirectWithCookie("/index.html", 
    "JSESSIONID=" + sessionId + "; Path=/; HttpOnly; Secure");
🧹 Nitpick comments (6)
src/main/java/http/enums/HttpStatus.java (1)

3-19: 깔끔한 구현입니다.

현재 구현된 세 가지 상태 코드(200, 302, 404)는 현재 기능 요구사항을 충족합니다. 향후 필요 시 추가 HTTP 상태 코드(예: 400 Bad Request, 500 Internal Server Error)를 확장할 수 있도록 설계되었습니다.

src/main/java/webserver/WebServer.java (1)

3-3: 동일 패키지 import는 불필요합니다.

main.java.webserver.RequestHandlerWebServer와 같은 패키지에 있으므로 명시적 import가 필요하지 않습니다. 제거해도 코드 동작에 영향을 주지 않습니다.

src/main/java/http/controller/StaticResourceController.java (1)

8-14: 정적 리소스 처리 로직이 정확합니다.

루트 경로("/")를 "/index.html"로 매핑하고 response.forward를 통해 리소스를 제공하는 구현이 올바릅니다. Router의 fallback 컨트롤러로서 역할을 잘 수행합니다.

선택사항: 디버깅 편의를 위해 리소스 경로를 로깅하는 것을 고려할 수 있습니다.

src/main/java/http/controller/UserListController.java (1)

14-14: 정적 리소스 경로 하드코딩

/user/list.html 경로가 하드코딩되어 있습니다. 경로 변경 시 여러 곳을 수정해야 하므로 상수로 추출하는 것을 고려하세요.

예시:

private static final String USER_LIST_PATH = "/user/list.html";
// ...
response.forward(USER_LIST_PATH);
src/main/java/http/enums/HttpHeader.java (1)

3-13: HTTP 헤더 상수 정의 잘 구현됨

HTTP 헤더를 enum으로 중앙 집중화하여 오타 및 불일치를 방지하는 좋은 설계입니다.

향후 필요시 추가로 고려할 헤더들:

  • AUTHORIZATION (인증 토큰용)
  • COOKIE (요청 쿠키 읽기용)
  • CACHE_CONTROL (캐싱 제어용)
  • CONNECTION (연결 관리용)
src/main/java/http/enums/HttpMethod.java (1)

6-11: HTTP 메서드 파싱 로직 잘 구현됨

대소문자 구분 없이 HTTP 메서드를 파싱하고, 지원하지 않는 메서드에 대해 명확한 예외를 던지는 구조가 적절합니다.

향후 필요시 추가로 고려할 메서드들:

  • PATCH (부분 업데이트용)
  • HEAD (메타데이터만 조회)
  • OPTIONS (CORS preflight용)

예시:

 public enum HttpMethod {
-    GET, POST, PUT, DELETE;
+    GET, POST, PUT, DELETE, PATCH, HEAD, OPTIONS;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dba7017 and 95ddcee.

📒 Files selected for processing (34)
  • .idea/gradle.xml (1 hunks)
  • .idea/vcs.xml (1 hunks)
  • README.md (1 hunks)
  • build.gradle (2 hunks)
  • src/main/java/db/MemoryUserRepository.java (2 hunks)
  • src/main/java/db/Repository.java (1 hunks)
  • src/main/java/http/HttpRequest.java (1 hunks)
  • src/main/java/http/HttpResponse.java (1 hunks)
  • src/main/java/http/controller/Controller.java (1 hunks)
  • src/main/java/http/controller/Route.java (1 hunks)
  • src/main/java/http/controller/Router.java (1 hunks)
  • src/main/java/http/controller/Routes.java (1 hunks)
  • src/main/java/http/controller/StaticResourceController.java (1 hunks)
  • src/main/java/http/controller/UserListController.java (1 hunks)
  • src/main/java/http/controller/UserLoginController.java (1 hunks)
  • src/main/java/http/controller/UserSignupController.java (1 hunks)
  • src/main/java/http/enums/HttpHeader.java (1 hunks)
  • src/main/java/http/enums/HttpMethod.java (1 hunks)
  • src/main/java/http/enums/HttpStatus.java (1 hunks)
  • src/main/java/http/enums/QueryKey.java (1 hunks)
  • src/main/java/http/util/HttpRequestUtils.java (1 hunks)
  • src/main/java/http/util/IOUtils.java (1 hunks)
  • src/main/java/model/User.java (1 hunks)
  • src/main/java/webserver/RequestHandler.java (1 hunks)
  • src/main/java/webserver/WebServer.java (1 hunks)
  • src/test/java/http/HttpRequestTest.java (1 hunks)
  • src/test/java/http/HttpResponseTest.java (1 hunks)
  • src/test/java/http/TestUtil.java (1 hunks)
  • src/test/resources/http/cookie_request.txt (1 hunks)
  • src/test/resources/http/get_with_query.txt (1 hunks)
  • src/test/resources/http/post_signup.txt (1 hunks)
  • webapp/index.html (1 hunks)
  • webapp/index_test.html (1 hunks)
  • webapp/user/form.html (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
src/main/java/http/controller/Controller.java (2)
src/main/java/http/HttpRequest.java (1)
  • HttpRequest (11-166)
src/main/java/http/HttpResponse.java (1)
  • HttpResponse (8-83)
src/main/java/db/Repository.java (1)
src/main/java/model/User.java (1)
  • User (5-46)
src/main/java/webserver/RequestHandler.java (4)
src/main/java/http/HttpRequest.java (1)
  • HttpRequest (11-166)
src/main/java/http/HttpResponse.java (1)
  • HttpResponse (8-83)
src/main/java/http/controller/Router.java (1)
  • Router (6-20)
src/main/java/http/controller/Routes.java (1)
  • Routes (5-16)
src/main/java/http/controller/UserLoginController.java (2)
src/main/java/model/User.java (1)
  • User (5-46)
src/main/java/db/MemoryUserRepository.java (1)
  • MemoryUserRepository (9-35)
src/main/java/db/MemoryUserRepository.java (1)
src/main/java/model/User.java (1)
  • User (5-46)
src/main/java/webserver/WebServer.java (1)
src/main/java/webserver/RequestHandler.java (1)
  • RequestHandler (16-53)
src/test/java/http/HttpRequestTest.java (2)
src/main/java/http/HttpRequest.java (1)
  • HttpRequest (11-166)
src/test/java/http/TestUtil.java (1)
  • TestUtil (6-18)
src/main/java/http/controller/StaticResourceController.java (2)
src/main/java/http/HttpRequest.java (1)
  • HttpRequest (11-166)
src/main/java/http/HttpResponse.java (1)
  • HttpResponse (8-83)
src/main/java/http/controller/UserSignupController.java (2)
src/main/java/model/User.java (1)
  • User (5-46)
src/main/java/db/MemoryUserRepository.java (1)
  • MemoryUserRepository (9-35)
src/test/java/http/HttpResponseTest.java (1)
src/main/java/http/HttpResponse.java (1)
  • HttpResponse (8-83)
🪛 markdownlint-cli2 (0.18.1)
README.md

2-2: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)

🔇 Additional comments (9)
build.gradle (2)

4-8: LGTM! Java 21 툴체인 설정이 적절합니다.

Java 21 LTS 버전으로 명시적인 툴체인 설정은 빌드 재현성과 팀 간 일관성을 보장하는 좋은 관행입니다.


17-18: LGTM! 테스트 의존성 버전이 최신 안정 버전입니다.

JUnit 5.10.2와 AssertJ 3.25.3은 2024년 2월에 릴리스된 안정 버전으로, 버그 수정 및 성능 개선이 포함되어 있습니다. Based on learnings

참고: BOM 대신 명시적 버전을 사용하는 현재 방식은 단일 JUnit 모듈만 사용하는 경우 적절합니다. 향후 여러 JUnit 모듈을 추가할 경우 BOM 사용을 고려하세요.

webapp/user/form.html (1)

58-58: LGTM!

폼 제출 방식이 POST로 변경되어 새로운 라우팅 및 컨트롤러 흐름(Routes.java, UserSignupController)과 올바르게 통합되었습니다.

webapp/index.html (1)

121-122: LGTM!

Bootstrap JS 번들 및 로컬 scripts.js 추가로 인터랙티브 컴포넌트와 커스텀 스크립트를 지원합니다. 표준적인 프론트엔드 구성입니다.

src/main/java/model/User.java (1)

1-1: LGTM!

패키지 네임스페이스 변경이 전체 리팩토링과 일치하며 로직 변경은 없습니다.

src/test/resources/http/post_signup.txt (1)

1-8: LGTM!

회원가입 POST 요청을 위한 테스트 픽스처가 올바르게 구성되어 있으며, HttpRequestTest에서 폼 URL 인코딩 파싱 검증에 사용됩니다.

src/main/java/db/Repository.java (1)

1-3: 패키지 리팩토링 확인 완료

패키지 구조가 main.java.db로 일관되게 변경되었고, User 모델의 import 경로도 올바르게 업데이트되었습니다.

src/main/java/http/controller/Controller.java (1)

8-10: 깔끔한 인터페이스 설계

Controller 인터페이스가 단일 책임을 명확히 정의하고 있습니다. HttpRequest와 HttpResponse를 파라미터로 받아 실행하는 구조는 확장 가능하고 테스트하기 좋은 설계입니다.

src/main/java/http/enums/QueryKey.java (1)

3-14: 쿼리 파라미터 키 중앙화

QueryKey enum을 통해 파라미터 이름을 중앙 집중화하여 오타를 방지하고 유지보수성을 높였습니다. HttpHeader enum과 일관된 패턴입니다.

Comment on lines +2 to +15
### 프로젝트 소개
KUIT 6기 서버 파트에서 진행하는 Custom Tomcat 프로젝트입니다. Java 소켓 프로그래밍을 활용하여 직접 웹 서버를 구현하고, 기본적인 웹 애플리케이션을 구동하는 것을 목표로 합니다.

### 주요 기능
- 사용자 관리: 회원가입, 로그인, 사용자 목록 조회 기능을 제공합니다.
- Q&A 게시판: 질문 및 답변을 등록하고 조회할 수 있는 기본적인 게시판 기능을 구현합니다.
- 정적 파일 서빙: HTML, CSS, JavaScript, 이미지 등 정적 파일을 처리하여 웹 페이지를 렌더링합니다.
- 동적 요청 처리: 사용자의 요청에 따라 동적으로 HTML을 생성하여 응답합니다.

### 기술 스택
- 언어: Java
- 빌드 도구: Gradle
- 웹 서버: 직접 구현한 Custom Web Server
- 프론트엔드: HTML, CSS No newline at end of file
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

헤딩 레벨 불일치 수정 필요

최상위 제목 바로 아래에서 H3(###)으로 건너뛰면서 markdownlint MD001 규칙을 위반합니다. H2로 맞춰 주세요.

-### 프로젝트 소개
+## 프로젝트 소개
@@
-### 주요 기능
+## 주요 기능
@@
-### 기술 스택
+## 기술 스택
📝 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
### 프로젝트 소개
KUIT 6기 서버 파트에서 진행하는 Custom Tomcat 프로젝트입니다. Java 소켓 프로그래밍을 활용하여 직접 웹 서버를 구현하고, 기본적인 웹 애플리케이션을 구동하는 것을 목표로 합니다.
### 주요 기능
- 사용자 관리: 회원가입, 로그인, 사용자 목록 조회 기능을 제공합니다.
- Q&A 게시판: 질문 및 답변을 등록하고 조회할 수 있는 기본적인 게시판 기능을 구현합니다.
- 정적 파일 서빙: HTML, CSS, JavaScript, 이미지 등 정적 파일을 처리하여 웹 페이지를 렌더링합니다.
- 동적 요청 처리: 사용자의 요청에 따라 동적으로 HTML을 생성하여 응답합니다.
### 기술 스택
- 언어: Java
- 빌드 도구: Gradle
- 웹 서버: 직접 구현한 Custom Web Server
- 프론트엔드: HTML, CSS
## 프로젝트 소개
KUIT 6기 서버 파트에서 진행하는 Custom Tomcat 프로젝트입니다. Java 소켓 프로그래밍을 활용하여 직접 웹 서버를 구현하고, 기본적인 웹 애플리케이션을 구동하는 것을 목표로 합니다.
## 주요 기능
- 사용자 관리: 회원가입, 로그인, 사용자 목록 조회 기능을 제공합니다.
- Q&A 게시판: 질문 및 답변을 등록하고 조회할 수 있는 기본적인 게시판 기능을 구현합니다.
- 정적 파일 서빙: HTML, CSS, JavaScript, 이미지 등 정적 파일을 처리하여 웹 페이지를 렌더링합니다.
- 동적 요청 처리: 사용자의 요청에 따라 동적으로 HTML을 생성하여 응답합니다.
## 기술 스택
- 언어: Java
- 빌드 도구: Gradle
- 웹 서버: 직접 구현한 Custom Web Server
- 프론트엔드: HTML, CSS
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)

2-2: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)

🤖 Prompt for AI Agents
In README.md around lines 2 to 15 the document skips from the top-level title to
H3 (###) headings which violates markdownlint MD001; change the section headings
under the main title from "###" to "##" (e.g., "### 프로젝트 소개" -> "## 프로젝트 소개" and
the following "### 주요 기능" -> "## 주요 기능", "### 기술 스택" -> "## 기술 스택") so heading
levels are consistent.

private static MemoryUserRepository memoryUserRepository;

private MemoryUserRepository() {
private MemoryUserRepository() {// 회원가입 시 url에 생기는 쿼리스트링을 받아오기
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

생성자 주석이 실제 로직과 일치하지 않습니다.

private 생성자는 파라미터를 받지 않으며 쿼리스트링 파싱 로직도 포함하지 않습니다. 주석을 제거하거나 생성자의 실제 목적(싱글톤 인스턴스 생성 방지)에 맞게 수정하세요.

🤖 Prompt for AI Agents
In src/main/java/db/MemoryUserRepository.java around line 13, the private
constructor has a misleading comment about parsing query strings; remove that
comment or replace it with a concise, accurate comment stating the constructor
is private to prevent external instantiation (e.g., singleton pattern), since it
takes no parameters and contains no parsing logic.

Comment on lines +10 to +13
if (!"true".equals(request.getCookie("logined"))) {
response.redirect("/index.html");
return;
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

쿠키 기반 인증의 보안 취약점

클라이언트가 쉽게 조작할 수 있는 단순 쿠키 값(logined=true)으로 인증을 수행하고 있습니다. 이는 심각한 보안 취약점입니다:

  • 사용자가 브라우저 개발자 도구로 logined=true 쿠키를 직접 설정하여 인증을 우회할 수 있습니다.
  • 세션 타임아웃, 세션 무효화 등의 메커니즘이 없습니다.
  • CSRF 공격에 취약합니다.

서버 측 세션 관리(세션 ID 발급, 서버 측 세션 저장소, 타임아웃)를 도입하거나, 최소한 서명된 토큰(JWT 등)을 사용하는 것을 권장합니다.

Comment on lines +14 to +24
public void execute(HttpRequest request, HttpResponse response) throws IOException {
String userId = request.getParam(QueryKey.USER_ID.getKey());
String password = request.getParam(QueryKey.PASSWORD.getKey());
User found = MemoryUserRepository.getInstance().findUserById(userId);

if (found != null && found.getPassword().equals(password)) {
response.redirectWithCookie("/index.html", "logined=true; Path=/");
} else {
response.redirect("/user/login_failed.html");
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

로그인 시도 제한 메커니즘 부재

브루트포스 공격을 방지하기 위한 메커니즘이 없습니다:

  • 로그인 시도 횟수 제한 없음
  • 계정 잠금 기능 없음
  • CAPTCHA 없음

최소한 IP 기반 또는 계정 기반 로그인 시도 제한을 추가하는 것을 권장합니다.

String password = request.getParam(QueryKey.PASSWORD.getKey());
User found = MemoryUserRepository.getInstance().findUserById(userId);

if (found != null && found.getPassword().equals(password)) {
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

평문 비밀번호 비교 및 타이밍 공격 취약점

비밀번호를 평문으로 비교하고 있어 두 가지 심각한 보안 문제가 있습니다:

  1. 평문 저장/비교: 비밀번호가 평문으로 저장되고 비교됩니다. DB가 유출되면 모든 비밀번호가 노출됩니다.
  2. 타이밍 공격: equals() 메서드는 첫 번째 불일치 문자에서 즉시 반환하므로, 공격자가 응답 시간을 측정하여 비밀번호를 추측할 수 있습니다.

해결 방법:

  • BCrypt, PBKDF2, Argon2 등의 해시 알고리즘 사용
  • 해시 비교 시 MessageDigest.isEqual() 같은 상수 시간 비교 사용
// 회원가입 시
String hashedPassword = BCrypt.hashpw(password, BCrypt.gensalt());

// 로그인 시
if (found != null && BCrypt.checkpw(password, found.getPassword())) {
    // ...
}
🤖 Prompt for AI Agents
In src/main/java/http/controller/UserLoginController.java around line 19, the
code is comparing plaintext passwords with equals(), which exposes stored
passwords and is vulnerable to timing attacks; replace plaintext storage and
comparison with a secure password-hashing scheme (e.g., BCrypt/PBKDF2/Argon2),
store only the salted hash (generate salt per-password on signup), and on login
use the library's verify function (e.g., BCrypt.checkpw) or a constant-time
comparison routine instead of equals(); also ensure the user table column can
hold the hash, update signup flow to hash passwords before saving, and add a
migration/upgrade path for existing plaintext passwords.

@@ -1,4 +1,4 @@
package http.util;
package main.java.http.util;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

비표준 패키지 네이밍 컨벤션을 재검토하세요.

main.java.http.util 패키지명은 Java 표준 컨벤션을 따르지 않습니다. main.java는 일반적으로 소스 디렉토리 경로(src/main/java)를 나타내며, 패키지명으로 사용되지 않습니다.

표준 Java 패키지 네이밍 컨벤션:

  • 역도메인 표기법 사용 (예: com.kuit.http.util, kuit.http.util)
  • 소문자만 사용
  • main, java와 같은 디렉토리 구조 키워드를 패키지명에 포함하지 않음

이 패턴이 코드베이스 전반에 적용된 것으로 보이므로, 프로젝트 전체의 패키지 구조를 재검토하고 표준 컨벤션으로 리팩토링하는 것을 권장합니다.

다음과 같이 수정할 수 있습니다:

-package main.java.http.util;
+package kuit.http.util;
📝 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
package main.java.http.util;
package kuit.http.util;
🤖 Prompt for AI Agents
src/main/java/http/util/IOUtils.java around lines 1 to 1: the package
declaration uses a non-standard name `main.java.http.util`; change it to a
proper lower-case domain-based package (e.g., `com.yourdomain.http.util` or
`kuit.http.util`), move the file into the matching directory structure under
src/main/java (e.g., src/main/java/com/yourdomain/http/util/IOUtils.java),
update the package declaration at the top of the file, and then update all
imports/usages and any build configurations (IDE project, Maven/Gradle,
module-info if present) to reflect the new package so the project compiles;
perform a project-wide refactor (IDE rename or scripted search-and-replace) to
ensure consistency across the codebase and run the build/tests to verify.

@@ -0,0 +1,49 @@
package test.java.http;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

패키지 선언이 디렉터리 구조와 불일치합니다.
이 파일도 src/test/java/http/… 아래에 있으므로 패키지가 http여야 합니다. 현재 선언(package test.java.http;)으로는 테스트 컴파일이 아예 되지 않습니다. 아래와 같이 수정해주세요.

- package test.java.http;
+ package http;
📝 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
package test.java.http;
package http;
🤖 Prompt for AI Agents
In src/test/java/http/HttpRequestTest.java around lines 1 to 1, the package
declaration "package test.java.http;" does not match the file path and prevents
compilation; change the package declaration to "package http;" (so it matches
src/test/java/http), save the file, and re-run tests/compile to verify the issue
is resolved.

@@ -0,0 +1,90 @@
package test.java.http;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

패키지 선언을 디렉터리와 일치시켜 주십시오.

소스 경로가 src/test/java/http/HttpResponseTest.java인데 패키지가 test.java.http로 선언되어 있어 테스트 컴파일 자체가 실패합니다. 디렉터리 구조와 동일한 package http; 등으로 정정해 주세요.

다음과 같이 수정하면 문제를 해결할 수 있습니다.

-package test.java.http;
+package http;
📝 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
package test.java.http;
package http;
🤖 Prompt for AI Agents
In src/test/java/http/HttpResponseTest.java around line 1, the declared package
"test.java.http" does not match the file's directory (src/test/java/http),
causing test compilation to fail; change the package declaration to "package
http;" (or the exact package that matches the directory structure), and update
any references/imports in other test files if they rely on the old package name.

Comment on lines +35 to +48
void forward_ok_writes_status_headers_and_body() throws Exception {
String body = "HELLO";
Files.writeString(WEBAPP_DIR.resolve("index_test.html"), body, UTF_8);

try (OutputStream out = outputStreamToFile("resp_forward_ok.txt")) {
HttpResponse resp = new HttpResponse(out);
resp.forward("/index.html");
}

String raw = readFile("resp_forward_ok.txt");
assertThat(raw).startsWith(HttpStatus.OK.format());
assertThat(raw).contains(HttpHeader.CONTENT_TYPE.getKey() + ": text/html");
assertThat(raw).contains(HttpHeader.CONTENT_LENGTH.getKey() + ": 5");
assertThat(raw).endsWith(body);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

테스트 대상 리소스 경로가 잘못 지정되었습니다.

테스트에서 webapp/index_test.html을 작성하지만 실제로는 /index.html을 forward 하므로 404 응답이 내려와 본문·헤더 검증이 모두 실패합니다. 테스트가 의도한 리소스를 forward 하도록 경로를 맞춰 주세요.

다음과 같이 수정하면 테스트가 의도한 데이터를 검증할 수 있습니다.

-        Files.writeString(WEBAPP_DIR.resolve("index_test.html"), body, UTF_8);
+        Files.writeString(WEBAPP_DIR.resolve("index_test.html"), body, UTF_8);
 ...
-            resp.forward("/index.html");
+            resp.forward("/index_test.html");
📝 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
void forward_ok_writes_status_headers_and_body() throws Exception {
String body = "HELLO";
Files.writeString(WEBAPP_DIR.resolve("index_test.html"), body, UTF_8);
try (OutputStream out = outputStreamToFile("resp_forward_ok.txt")) {
HttpResponse resp = new HttpResponse(out);
resp.forward("/index.html");
}
String raw = readFile("resp_forward_ok.txt");
assertThat(raw).startsWith(HttpStatus.OK.format());
assertThat(raw).contains(HttpHeader.CONTENT_TYPE.getKey() + ": text/html");
assertThat(raw).contains(HttpHeader.CONTENT_LENGTH.getKey() + ": 5");
assertThat(raw).endsWith(body);
void forward_ok_writes_status_headers_and_body() throws Exception {
String body = "HELLO";
Files.writeString(WEBAPP_DIR.resolve("index_test.html"), body, UTF_8);
try (OutputStream out = outputStreamToFile("resp_forward_ok.txt")) {
HttpResponse resp = new HttpResponse(out);
resp.forward("/index_test.html");
}
String raw = readFile("resp_forward_ok.txt");
assertThat(raw).startsWith(HttpStatus.OK.format());
assertThat(raw).contains(HttpHeader.CONTENT_TYPE.getKey() + ": text/html");
assertThat(raw).contains(HttpHeader.CONTENT_LENGTH.getKey() + ": 5");
assertThat(raw).endsWith(body);
}
🤖 Prompt for AI Agents
In src/test/java/http/HttpResponseTest.java around lines 35 to 48, the test
writes the file WEBAPP_DIR/ index_test.html but calls
resp.forward("/index.html"), causing a 404; change the test to create the
resource that will actually be forwarded (e.g., write to
WEBAPP_DIR.resolve("index.html") instead of "index_test.html") so
forward("/index.html") finds the file and the subsequent status, headers and
body assertions pass.

@@ -0,0 +1,18 @@
package test.java.http;
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

패키지 선언이 디렉터리 구조와 맞지 않습니다.
src/test/java/http/TestUtil.java에 위치한 클래스가 package test.java.http;로 선언되어 있어 컴파일 시 “bad class file” 오류가 발생합니다. 소스 루트(src/test/java) 기준 디렉터리(http/)와 동일하게 package http;로 선언을 바꿔 주세요.

- package test.java.http;
+ package http;
📝 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
package test.java.http;
package http;
🤖 Prompt for AI Agents
In src/test/java/http/TestUtil.java around lines 1 to 1, the package declaration
"package test.java.http;" does not match the file path and causes compilation
errors; update the package declaration to "package http;" so it matches the
source root (src/test/java) and the http/ directory, ensuring the file compiles
and package structure is consistent.

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.

2 participants