Conversation
WalkthroughHTTP 요청/응답 파서와 응답 빌더를 추가하고, Controller 인터페이스 기반의 라우팅(RequestMapper + RequestHandler)으로 서버 흐름을 전환했습니다. 여러 컨트롤러, HTTP 관련 enum·유틸·포맷타입, 테스트 및 관련 웹 리소스와 IDE 설정이 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant RH as RequestHandler
participant RM as RequestMapper
participant CTR as Controller
participant FS as FileSystem (webapp)
Note over C,RH: 컨트롤러 기반 처리 흐름
C->>RH: TCP 연결, HTTP 요청 전송
RH->>RH: HttpRequest.from(InputStream)
RH->>RM: findController(request.getPath())
RM-->>RH: Controller 또는 ForwardController(기본)
RH->>CTR: controller.execute(request, response)
alt 정적 포워드 (파일 제공)
CTR->>FS: 파일 조회/읽기
FS-->>CTR: 파일 바이트
CTR->>RH: response.forward(path)
RH-->>C: HTTP/1.1 200 + Body
else 리다이렉트/쿠키 설정
CTR->>RH: response.response302Header(location)
RH-->>C: HTTP/1.1 302 + Location (+Set-Cookie)
else 컨트롤러 없음 → 정적 자원 시도
RH->>FS: 정적 자원 제공 시도
FS-->>RH: 파일 또는 없음
RH-->>C: 200 파일 / 404 응답
end
sequenceDiagram
autonumber
participant C as Client
participant LC as LoginController
participant Repo as MemoryUserRepository
Note over C,LC: 로그인 처리 (GET/POST)
C->>LC: GET /user/login
LC-->>C: forward(/user/login.html)
C->>LC: POST /user/login (id,password)
LC->>Repo: findById(id)
Repo-->>LC: User? (있음/없음)
alt 인증 성공
LC-->>C: 302 Location:/index.html + Set-Cookie: logined=true
else 인증 실패
LC-->>C: 302 Location:/user/login_failed.html
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. 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: 16
🧹 Nitpick comments (15)
.idea/gradle.xml (1)
8-8: IDE의 gradleJvm 고정은 환경 불일치 위험이 큽니다 → 빌드(Gradle Toolchain)로 표준화하세요IntelliJ 설정(
gradleJvm="corretto-21")은 동료/CI 환경과 어긋날 수 있습니다. IDE 파일은 커밋하지 말고, Gradle Toolchain으로 자바 버전을 고정하는 것을 권장합니다. 유지가 필요 없다면 해당 옵션을 제거하세요.적용 예(옵션 제거):
- <option name="gradleJvm" value="corretto-21" />Gradle 설정 예(프로젝트 표준화):
- build.gradle.kts
java { toolchain { languageVersion.set(JavaLanguageVersion.of(21)) } } tasks.withType<Test> { useJUnitPlatform() }
- wrapper로 빌드 일관성 유지:
./gradlew --version기준으로 Gradle 8.x 이상 권장.검증 스크립트:
#!/bin/bash set -euo pipefail echo "[Gradle Wrapper 파일 확인]" fd -a -H '^gradlew$|^gradlew\.bat$|gradle/wrapper/gradle-wrapper\.jar$|gradle/wrapper/gradle-wrapper\.properties$' || true echo echo "[build 설정 내 Toolchain 사용 여부 탐지]" rg -n -C2 -S --type-add 'kts:*.kts' --type-add 'gradle:*.gradle' --type kts --type gradle 'toolchain|JavaLanguageVersion|languageVersion\.set' || echo "=> Toolchain 설정 미탐지" echo echo "[CI에서 JDK 설정 유무 확인(.github/workflows)]" fd -a -H '.github/workflows' >/dev/null 2>&1 && rg -n -C1 -S '.?setup-java|java-version' .github/workflows || echo "=> 워크플로우 없거나 JDK 설정 미탐지".idea/vcs.xml (1)
1-6: *.gitignore에 .idea/ 및 .iml 패턴 추가
현재 .gitignore에 일부 .idea 하위 파일만 제외되어 있어 .idea/vcs.xml, .idea/misc.xml, .idea/gradle.xml이 여전히 커밋됩니다. IDE 전용 파일은.idea/로 전체 무시하거나, 꼭 필요한 설정만 별도 관리하세요.src/main/java/http/util/HttpRequestUtils.java (1)
35-42: URL 디코딩 실패 시 원본 값 반환 동작을 검토해주세요.UnsupportedEncodingException 발생 시 원본 값을 반환하는 방식은 조용한 실패(silent failure)입니다. UTF-8은 표준 인코딩이므로 이 예외는 거의 발생하지 않지만, 발생 시 디코딩되지 않은 값이 그대로 사용되어 예상치 못한 동작을 유발할 수 있습니다.
로깅을 추가하거나, RuntimeException으로 래핑하여 문제를 명확히 드러내는 것을 고려해주세요.
로깅을 추가하는 예시:
+import java.util.logging.Level; +import java.util.logging.Logger; + public class HttpRequestUtils { + private static final Logger log = Logger.getLogger(HttpRequestUtils.class.getName()); + private static String decodeUrlComponent(String value) { try { return URLDecoder.decode(value, StandardCharsets.UTF_8.name()); } catch (UnsupportedEncodingException e) { + log.log(Level.WARNING, "Failed to decode URL component: " + value, e); return value; } }src/main/java/controller/LoginController.java (2)
37-37: 쿠키 이름 오타를 수정하세요."logined"는 올바른 영어 표현이 아닙니다. "loggedIn" 또는 "isLoggedIn"으로 변경하는 것을 권장합니다.
-response.addHeader(HttpHeader.SET_COOKIE, "logined=true"); +response.addHeader(HttpHeader.SET_COOKIE, "loggedIn=true");
33-43: 무차별 대입 공격(brute force) 방어가 없습니다.로그인 실패에 대한 rate limiting이나 계정 잠금 메커니즘이 없어 무차별 대입 공격에 취약합니다.
프로덕션 환경에서는 다음을 고려하세요:
- 실패 시도 횟수 제한 (예: 5분 내 5회 실패 시 계정 잠금)
- IP 기반 rate limiting
- CAPTCHA 추가
src/main/java/webserver/UrlPath.java (1)
11-13: 사용자 목록 경로 변형이 3개 존재합니다.
USER_LIST,USER_LIST_ALIAS,USER_LIST_HTML의 의미와 사용 목적을 명확히 하는 것이 좋습니다. 가능하다면 중복을 줄이고 하나의 정규 경로로 통일하는 것을 검토하세요.현재 구현이 라우팅 요구사항을 충족한다면 그대로 유지해도 무방합니다.
src/test/java/http/HttpRequestTest.java (1)
22-36: 좋습니다. POST 파싱 경로가 잘 검증되었습니다.추가로 폼 파라미터 디코딩까지 보장하려면 다음 단언을 권장합니다:
assertEquals("josh@example.com", request.getParameter("email"))src/main/java/webserver/RequestHandler.java (1)
34-36: 예외 처리 범위를 넓혀 실행 중단을 방지하세요IOException 이외 예외는 잡히지 않아 스레드가 끊길 수 있습니다.
catch (Exception e)를 추가하세요.- } catch (IOException e) { - log.log(Level.SEVERE, e.getMessage()); - } + } catch (IOException e) { + log.log(Level.SEVERE, e.getMessage()); + } catch (Exception e) { + log.log(Level.SEVERE, e.getMessage(), e); + }src/main/java/http/HttpResponse.java (4)
58-58: 표준 라이브러리 사용을 권장합니다.커스텀
readAllBytes대신 Java NIO의Files.readAllBytes(target.toPath())를 사용하면 더 간결하고 표준적입니다.다음과 같이 수정할 수 있습니다:
- byte[] body = readAllBytes(target); + byte[] body = Files.readAllBytes(target.toPath());그리고 Lines 127-134의
readAllBytes메서드는 제거할 수 있습니다.
102-110: BufferedWriter 리소스 관리를 개선할 수 있습니다.Line 103에서 매번 새로운
BufferedWriter를 생성하는데, 이를 필드로 만들어 재사용하거나, 최소한 try-with-resources를 사용하는 것이 좋습니다.다만 현재 구조에서는
out을 열린 상태로 유지해야 하므로 close하면 안 되는 상황입니다. 이 경우 명시적으로 주석을 추가하거나, Writer를 인스턴스 필드로 두는 것을 고려하세요.
117-125: 콘텐츠 타입 추측 로직이 기본적으로 잘 작동합니다.일반적인 확장자를 커버하고 있어 충분합니다. 필요하다면
.svg,.gif,.json등을 추가할 수 있지만, 선택 사항입니다.
81-88: 예외를 조용히 삼키는 것은 디버깅을 어렵게 만듭니다.IOException을 단순히 로깅만 하고 무시하면 호출자가 리다이렉트 실패를 감지할 수 없습니다. 성공 여부를 boolean으로 반환하거나, RuntimeException으로 래핑하여 재throw하는 것을 고려해보세요.
다음과 같이 수정할 수 있습니다:
- public void sendRedirect(String location) { + public void sendRedirect(String location) throws IOException { try { response302Header(location); - // Redirect 응답은 보통 바디를 비웁니다. } catch (IOException e) { - log.log(Level.SEVERE, e.getMessage()); + log.log(Level.SEVERE, "Failed to send redirect to " + location, e); + throw e; } }src/main/java/http/HttpRequest.java (3)
88-88: NumberFormatException 처리를 추가하면 좋습니다.잘못된 형식의
Content-Length헤더(예: "abc")가 오면NumberFormatException이 발생합니다. 일관성을 위해 이를 catch하여IOException으로 감싸는 것을 권장합니다.다음과 같이 수정할 수 있습니다:
String cl = headers.get(HttpHeader.CONTENT_LENGTH.value()); if (cl != null) { - int contentLength = Integer.parseInt(cl.trim()); + int contentLength; + try { + contentLength = Integer.parseInt(cl.trim()); + } catch (NumberFormatException e) { + throw new IOException("Invalid Content-Length: " + cl, e); + }
98-136: 접근자 메서드들이 잘 구현되었습니다.Line 123과 127에서
Collections.unmodifiableMap을 반환하여 불변성을 보장하는 것이 훌륭합니다.Line 106-108의
getUrl()은 현재getPath()와 동일한데, 향후 쿼리 스트링 포함 등의 확장을 고려한다면 명확한 주석을 추가하는 것이 좋습니다.
40-69: 시작 라인 파싱 시 여러 공백 처리를 개선하세요.Line 46에서 단순히 공백으로 split하면 연속된 공백이 있을 때 빈 토큰이 생길 수 있습니다. 정규식이나
split(" +")또는split("\\s+")사용을 권장합니다.- String[] tokens = startLineRaw.split(" "); + String[] tokens = startLineRaw.split("\\s+");또한, lines 59-61의 null 체크는 불필요합니다.
HttpRequestUtils.parseQueryParameter는 항상 빈 맵이라도 반환합니다 (null을 반환하지 않음).Map<String, String> queryParams = HttpRequestUtils.parseQueryParameter(query); - if (queryParams != null) { - params.putAll(queryParams); - } + params.putAll(queryParams);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (25)
.idea/gradle.xml(1 hunks).idea/vcs.xml(1 hunks)src/main/java/controller/Controller.java(1 hunks)src/main/java/controller/ForwardController.java(1 hunks)src/main/java/controller/HomeController.java(1 hunks)src/main/java/controller/ListController.java(1 hunks)src/main/java/controller/LoginController.java(1 hunks)src/main/java/controller/SignUpController.java(1 hunks)src/main/java/http/HttpHeader.java(1 hunks)src/main/java/http/HttpMethod.java(1 hunks)src/main/java/http/HttpRequest.java(1 hunks)src/main/java/http/HttpResponse.java(1 hunks)src/main/java/http/HttpStartLine.java(1 hunks)src/main/java/http/HttpStatus.java(1 hunks)src/main/java/http/util/HttpRequestUtils.java(1 hunks)src/main/java/model/UserQueryKey.java(1 hunks)src/main/java/webserver/RequestHandler.java(2 hunks)src/main/java/webserver/RequestMapper.java(1 hunks)src/main/java/webserver/UrlPath.java(1 hunks)src/test/java/http/HttpRequestTest.java(1 hunks)src/test/java/http/HttpResponseTest.java(1 hunks)src/test/resources/http/get-user-list.txt(1 hunks)src/test/resources/http/post-signup.txt(1 hunks)webapp/user/form.html(2 hunks)webapp/user/userList.html(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
src/main/java/controller/ForwardController.java (2)
src/main/java/http/HttpRequest.java (1)
HttpRequest(16-152)src/main/java/http/HttpResponse.java (1)
HttpResponse(15-135)
src/main/java/controller/Controller.java (2)
src/main/java/http/HttpRequest.java (1)
HttpRequest(16-152)src/main/java/http/HttpResponse.java (1)
HttpResponse(15-135)
src/main/java/http/HttpRequest.java (3)
src/main/java/http/util/HttpRequestUtils.java (1)
HttpRequestUtils(10-43)src/main/java/http/util/IOUtils.java (1)
IOUtils(6-21)src/main/java/http/HttpStartLine.java (1)
HttpStartLine(5-34)
src/main/java/controller/SignUpController.java (4)
src/main/java/db/MemoryUserRepository.java (1)
MemoryUserRepository(9-35)src/main/java/http/HttpRequest.java (1)
HttpRequest(16-152)src/main/java/http/HttpResponse.java (1)
HttpResponse(15-135)src/main/java/model/User.java (1)
User(5-46)
src/main/java/controller/HomeController.java (2)
src/main/java/http/HttpRequest.java (1)
HttpRequest(16-152)src/main/java/http/HttpResponse.java (1)
HttpResponse(15-135)
src/main/java/webserver/RequestHandler.java (2)
src/main/java/http/HttpRequest.java (1)
HttpRequest(16-152)src/main/java/http/HttpResponse.java (1)
HttpResponse(15-135)
src/main/java/webserver/RequestMapper.java (5)
src/main/java/controller/ForwardController.java (1)
ForwardController(10-24)src/main/java/controller/HomeController.java (1)
HomeController(8-13)src/main/java/controller/ListController.java (1)
ListController(9-25)src/main/java/controller/LoginController.java (1)
LoginController(17-45)src/main/java/controller/SignUpController.java (1)
SignUpController(16-53)
src/main/java/controller/LoginController.java (4)
src/main/java/db/MemoryUserRepository.java (1)
MemoryUserRepository(9-35)src/main/java/http/HttpRequest.java (1)
HttpRequest(16-152)src/main/java/http/HttpResponse.java (1)
HttpResponse(15-135)src/main/java/model/User.java (1)
User(5-46)
src/test/java/http/HttpRequestTest.java (1)
src/main/java/http/HttpRequest.java (1)
HttpRequest(16-152)
src/main/java/controller/ListController.java (2)
src/main/java/http/HttpRequest.java (1)
HttpRequest(16-152)src/main/java/http/HttpResponse.java (1)
HttpResponse(15-135)
🔇 Additional comments (36)
src/test/resources/http/get-user-list.txt (1)
1-5: 테스트 리소스가 올바르게 작성되었습니다.유효한 HTTP/1.1 GET 요청 형식이며, 페이지네이션을 위한 쿼리 파라미터(offset, limit)가 적절히 포함되어 있습니다. ListController의 라우팅 테스트에 활용하기 적합합니다.
webapp/user/form.html (1)
58-58: 회원가입 폼의 메서드 변경이 적절합니다.GET에서 POST로 변경한 것은 보안상 올바른 선택입니다. 민감한 정보(비밀번호 등)가 URL에 노출되지 않고 요청 본문에 포함되어 전송됩니다. 서버 측 SignUpController의 POST 요청 처리와 일치합니다.
src/main/java/http/HttpHeader.java (1)
3-19: HTTP 헤더 enum이 잘 설계되었습니다.타입 안전한 헤더 이름 관리를 위한 표준적인 enum 패턴입니다. 주요 HTTP 헤더들(Content-Type, Content-Length, Location, Set-Cookie, Cookie)을 적절히 포함하고 있으며, 불변성과 캡슐화가 올바르게 구현되어 있습니다.
src/main/java/controller/Controller.java (1)
8-10: 컨트롤러 인터페이스가 잘 설계되었습니다.단일 메서드 인터페이스로 단일 책임 원칙을 잘 따르고 있습니다. Strategy 패턴을 활용한 요청 처리 방식이며, IOException 선언을 통해 I/O 오류 처리를 명확히 하고 있습니다. 모든 컨트롤러 구현체가 이 인터페이스를 통해 일관된 방식으로 처리됩니다.
src/main/java/http/util/HttpRequestUtils.java (1)
11-33: 쿼리 파라미터 파싱 로직이 개선되었습니다.LinkedHashMap을 사용하여 파라미터 순서를 보존하고, 명시적인 null/empty 체크와 URL 디코딩을 추가했습니다. 이전 구현 대비 가독성과 견고성이 향상되었습니다.
src/main/java/webserver/RequestMapper.java (1)
19-26: 중복 매핑 없음 확인
UrlPath 상수 값(USER_LIST, USER_LIST_ALIAS, USER_LIST_HTML)이 모두 고유하여 중복 매핑이 없습니다. 별도 조치 불필요합니다.src/test/resources/http/post-signup.txt (1)
1-9: Content-Length 일치 확인됨
본문 길이(68바이트)와 선언된 Content-Length(68)가 일치하며, 테스트 리소스 형식과 URL 인코딩이 적절합니다.src/main/java/controller/HomeController.java (1)
8-13: 구현이 올바릅니다.간단하고 명확한 구현입니다. 홈 페이지로 포워딩하는 로직이 정확합니다.
src/main/java/model/UserQueryKey.java (1)
3-18: 타입 안전한 쿼리 키 관리입니다.Enum을 사용하여 쿼리 파라미터 키를 중앙화한 것은 좋은 패턴입니다. 오타 방지 및 리팩토링이 용이합니다.
src/main/java/controller/ForwardController.java (1)
10-24: 정적 리소스 포워딩 로직이 올바릅니다.경로 정규화와 기본값 처리가 적절합니다.
HttpResponse.forward()가 경로 순회 공격을 방어하므로 추가 보안 검증은 불필요합니다.src/main/java/http/HttpStatus.java (1)
3-23: HTTP 상태 코드 enum 구현이 깔끔합니다.현재 사용 사례(200, 302, 404)에 충분한 구현입니다. 향후 필요시 추가 상태 코드(201 Created, 400 Bad Request, 401 Unauthorized, 500 Internal Server Error 등)를 쉽게 확장할 수 있습니다.
src/main/java/http/HttpMethod.java (1)
3-14: Enum 비교 메서드가 올바르게 구현되었습니다.
isEqual(HttpMethod)는 enum 참조 비교(==)를 사용하여 정확하고,isEqual(String)는 null 안전하며 대소문자를 무시하여 robust합니다.참고:
isEqual메서드는 편의 메서드이며,==또는equalsIgnoreCase를 직접 사용해도 되지만 일관된 API 제공 차원에서 적절합니다.src/test/java/http/HttpResponseTest.java (1)
18-42: forward 테스트가 올바르게 구현되었습니다.정적 파일 응답에 대한 테스트가 상태 코드, Content-Type, Content-Length, body 내용을 모두 검증하여 포괄적입니다.
src/main/java/webserver/UrlPath.java (1)
3-28: URL 경로를 중앙화한 설계가 좋습니다.하드코딩된 문자열을 제거하고 타입 안전한 경로 참조를 제공합니다. 유지보수성이 향상됩니다.
src/test/java/http/HttpRequestTest.java (1)
38-50: LGTMGET 쿼리 파라미터/바디 처리 검증이 명확합니다.
src/main/java/http/HttpStartLine.java (1)
16-21: 좋습니다. Start-Line 파싱 팩토리 로직이 명확합니다.대문자화/기본값 처리 모두 적절합니다.
src/main/java/webserver/RequestHandler.java (1)
26-33: RequestMapper.findController는 항상 non-null Controller 반환
findController구현에서defaultController를 항상 반환하도록 돼 있어 null 체크가 불필요합니다.Likely an incorrect or invalid review comment.
src/main/java/http/HttpResponse.java (9)
15-24: 클래스 구조가 깔끔합니다.응답 빌더 패턴으로 적절하게 설계되었으며,
LinkedHashMap을 사용해 헤더 순서를 보존하는 것도 좋은 선택입니다.
26-32: 헤더 추가 메서드가 잘 구현되었습니다.
HttpHeaderenum 오버로드를 제공하여 타입 안전성을 높인 점이 좋습니다.
67-79: 리다이렉트 응답 처리가 올바릅니다.Set-Cookie 헤더를 보존하면서 302 응답을 구성하는 로직이 정확합니다.
90-100: 404 응답 처리가 적절합니다.상태 코드와 헤더 설정이 올바르게 구현되었습니다.
112-115: 바디 쓰기 로직이 간결하고 정확합니다.
15-24: LGTM!생성자와 필드 초기화가 올바르게 구현되었습니다. LinkedHashMap을 사용하여 헤더 순서를 보존하는 것도 좋은 선택입니다.
26-32: LGTM!헤더 추가 메서드가 깔끔하게 구현되었습니다. HttpHeader enum을 지원하는 오버로드 메서드는 타입 안정성을 높여줍니다.
67-79: LGTM!302 리다이렉트 응답 로직이 올바르게 구현되었습니다. Set-Cookie 헤더를 보존하는 처리도 적절합니다.
90-100: LGTM!404 응답 처리가 올바르게 구현되었습니다. 간단한 HTML 메시지로 충분합니다.
src/main/java/http/HttpRequest.java (10)
16-33: 불변 설계가 훌륭합니다.모든 필드를 final로 선언하고 private 생성자를 사용하여 팩토리 메서드를 통한 생성을 강제하는 것이 좋은 패턴입니다.
35-38: 팩토리 메서드 설계가 깔끔합니다.UTF-8 인코딩을 명시적으로 지정한 것이 좋습니다.
40-69: 시작 라인 파싱 로직이 견고합니다.쿼리 파라미터 추출과 검증이 잘 구현되어 있습니다. Line 66의
HttpStartLine.of가 빈 경로를 "/"로 정규화해주는 것으로 보이므로 엣지 케이스도 처리됩니다.
71-80: 헤더 파싱이 HTTP 스펙에 맞게 구현되었습니다.첫 번째 콜론을 기준으로 분리하여 값에 콜론이 포함될 수 있는 경우도 올바르게 처리합니다.
138-151: 쿠키 파싱이 올바르게 구현되었습니다.요청의 Cookie 헤더 형식에 맞게 세미콜론으로 구분하고,
splitlimit을 2로 설정하여 값에 "="가 포함된 경우도 올바르게 처리합니다.
16-33: LGTM!불변 설계로 잘 구현되었습니다. private 생성자와 final 필드를 사용하여 안전한 객체를 만들었습니다.
35-38: LGTM!InputStream을 UTF-8로 BufferedReader로 변환하는 것은 HTTP 표준에 맞는 올바른 선택입니다.
71-80: 헤더 파싱이 올바르게 구현되었습니다.잘못된 형식의 헤더를 조용히 건너뛰는 관대한 방식입니다. 더 엄격한 검증이 필요하다면 예외를 던지는 것을 고려할 수 있지만, 현재 구현도 합리적입니다.
98-136: LGTM!접근자 메서드들이 잘 구현되었습니다.
Collections.unmodifiableMap을 사용하여 불변성을 보장하는 것이 좋습니다.참고:
getUrl()과getPath()가 현재 동일한 값을 반환하는데, 향후 쿼리 스트링을 포함한 전체 URL을 반환하도록 확장할 계획이라면 명확한 문서화를 권장합니다.
138-151: 쿠키 파싱이 값이 없는 쿠키를 건너뜁니다.Line 146에서
kv.length == 2만 허용하므로 값이 비어있는 쿠키(예:sessionid=)를 무시합니다. HTTP 쿠키는 빈 값을 가질 수 있으므로 이를 지원해야 합니다.다음과 같이 수정하세요:
String[] pairs = cookieHeader.split(";"); for (String pair : pairs) { String[] kv = pair.split("=", 2); - if (kv.length == 2) { - result.put(kv[0].trim(), kv[1].trim()); + if (kv.length >= 1 && !kv[0].trim().isEmpty()) { + String key = kv[0].trim(); + String value = kv.length > 1 ? kv[1].trim() : ""; + result.put(key, value); } }Likely an incorrect or invalid review comment.
| String userId = request.getParameter(UserQueryKey.USER_ID.key()); | ||
| User user = repository.findUserById(userId); | ||
| log.log(Level.INFO, "Login attempt userId={0}", userId); | ||
| if (user != null && user.getPassword().equals(request.getParameter(UserQueryKey.PASSWORD.key()))) { |
There was a problem hiding this comment.
입력 검증 및 null 안전성 검증이 필요합니다.
request.getParameter()가 null을 반환할 수 있으므로, Line 34와 Line 36에서 NPE가 발생할 수 있습니다. userId와 password에 대한 null 체크를 추가하세요.
다음 diff를 적용하여 null 체크를 추가하세요:
String userId = request.getParameter(UserQueryKey.USER_ID.key());
+if (userId == null || userId.isEmpty()) {
+ log.log(Level.INFO, "Login failed: missing userId");
+ response.response302Header(UrlPath.USER_LOGIN_FAILED.value());
+ return;
+}
+
User user = repository.findUserById(userId);
log.log(Level.INFO, "Login attempt userId={0}", userId);
-if (user != null && user.getPassword().equals(request.getParameter(UserQueryKey.PASSWORD.key()))) {
+String password = request.getParameter(UserQueryKey.PASSWORD.key());
+if (password == null || password.isEmpty()) {
+ log.log(Level.INFO, "Login failed userId={0}: missing password", userId);
+ response.response302Header(UrlPath.USER_LOGIN_FAILED.value());
+ return;
+}
+if (user != null && user.getPassword().equals(password)) {📝 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 = request.getParameter(UserQueryKey.USER_ID.key()); | |
| User user = repository.findUserById(userId); | |
| log.log(Level.INFO, "Login attempt userId={0}", userId); | |
| if (user != null && user.getPassword().equals(request.getParameter(UserQueryKey.PASSWORD.key()))) { | |
| String userId = request.getParameter(UserQueryKey.USER_ID.key()); | |
| if (userId == null || userId.isEmpty()) { | |
| log.log(Level.INFO, "Login failed: missing userId"); | |
| response.response302Header(UrlPath.USER_LOGIN_FAILED.value()); | |
| return; | |
| } | |
| User user = repository.findUserById(userId); | |
| log.log(Level.INFO, "Login attempt userId={0}", userId); | |
| String password = request.getParameter(UserQueryKey.PASSWORD.key()); | |
| if (password == null || password.isEmpty()) { | |
| log.log(Level.INFO, "Login failed userId={0}: missing password", userId); | |
| response.response302Header(UrlPath.USER_LOGIN_FAILED.value()); | |
| return; | |
| } | |
| if (user != null && user.getPassword().equals(password)) { |
🤖 Prompt for AI Agents
In src/main/java/controller/LoginController.java around lines 33 to 36,
request.getParameter(...) can return null causing NPEs; ensure you validate
userId and password before use by: check for null/empty userId after fetching it
and short-circuit (log and return an error/unauthorized response) if missing;
when comparing passwords, null-check the retrieved user and the password
parameter (or use a null-safe comparison such as
Objects.equals(user.getPassword(), passwordParam)) so you never call methods on
a null reference; update the log to reflect missing parameters and handle the
early return flow accordingly.
| String userId = request.getParameter(UserQueryKey.USER_ID.key()); | ||
| User user = repository.findUserById(userId); | ||
| log.log(Level.INFO, "Login attempt userId={0}", userId); | ||
| if (user != null && user.getPassword().equals(request.getParameter(UserQueryKey.PASSWORD.key()))) { |
There was a problem hiding this comment.
🧩 Analysis chain
평문 비밀번호 비교는 보안 위험입니다.
현재 구현은 평문으로 비밀번호를 비교합니다. 실제 프로덕션 환경에서는 BCrypt, Argon2 등의 해시 알고리즘을 사용하여 비밀번호를 안전하게 비교해야 합니다.
학습/데모 목적이라면 현재 구현이 적절할 수 있지만, 실제 배포 환경에서는 비밀번호 해싱을 반드시 구현해야 합니다.
🏁 Script executed:
#!/bin/bash
# Search for BCrypt usage in Java files
rg -n --type=java 'BCrypt' -g '*.java'
# Search for PasswordEncoder usage in Java files
rg -n --type=java 'PasswordEncoder' -g '*.java'
# Display imports and password-related declarations in LoginController.java
sed -n '1,80p' src/main/java/controller/LoginController.javaLength of output: 1638
비밀번호 평문 비교 제거 및 해시 알고리즘 적용
src/main/java/controller/LoginController.java:36
현재 사용자 비밀번호를 평문으로 직접 비교하고 있어 보안 취약점이 있습니다. BCrypt(또는 Argon2) 기반 해시 및 비교 로직으로 변경해야 합니다.
🤖 Prompt for AI Agents
In src/main/java/controller/LoginController.java around line 36, the code
directly compares plain-text passwords which is insecure; change it to use a
password-hashing check (e.g., BCrypt or Argon2). Replace the plain equality
check with a call to a password encoder's match method (for Spring, inject/use
BCryptPasswordEncoder or Argon2PasswordEncoder and call
encoder.matches(request.getParameter(PASSWORD), user.getPassword())). Ensure
user.getPassword() contains the stored hash, handle possible nulls, and do not
log the raw password; if the project does not yet persist hashed passwords,
migrate stored passwords to hashes before enabling this check.
|
|
||
| public class SignUpController implements Controller { | ||
| private static final Logger log = Logger.getLogger(SignUpController.class.getName()); | ||
| private final Repository repository = MemoryUserRepository.getInstance(); |
There was a problem hiding this comment.
MemoryUserRepository는 스레드 안전하지 않습니다(싱글턴/저장 Map 모두).
요청은 멀티스레드로 처리되므로, 현재 HashMap과 비동기 안전한 getInstance 구현은 경쟁 상태, 데이터 손상 위험이 있습니다. ConcurrentHashMap과 안전한 싱글턴으로 교체하세요.
아래와 같이 리팩터링(별도 파일 수정):
// in MemoryUserRepository.java
import java.util.concurrent.ConcurrentHashMap;
public class MemoryUserRepository implements Repository {
private final Map<String, User> users = new ConcurrentHashMap<>();
private static final MemoryUserRepository INSTANCE = new MemoryUserRepository();
private MemoryUserRepository() {}
public static MemoryUserRepository getInstance() {
return INSTANCE;
}
public void addUser(User user) { users.put(user.getUserId(), user); }
// 기타 메서드 동일
}🤖 Prompt for AI Agents
In src/main/java/controller/SignUpController.java around line 18, the controller
currently uses MemoryUserRepository.getInstance() which returns a
non-thread-safe singleton backed by a HashMap; replace MemoryUserRepository with
a thread-safe implementation: change the backing Map to a
java.util.concurrent.ConcurrentHashMap, implement an eager, thread-safe
singleton (private constructor + private static final INSTANCE and a simple
getInstance() returning INSTANCE), and keep repository methods the same but
operating on the ConcurrentHashMap so concurrent requests cannot corrupt state.
| @Override | ||
| public void execute(HttpRequest request, HttpResponse response) throws IOException { | ||
| HttpMethod method = request.getMethod(); | ||
| if (method != HttpMethod.GET && method != HttpMethod.POST) { | ||
| response.send404(request.getPath()); | ||
| return; | ||
| } | ||
|
|
||
| String userId = normalized(request, UserQueryKey.USER_ID); | ||
| String password = normalized(request, UserQueryKey.PASSWORD); | ||
| String name = normalized(request, UserQueryKey.NAME); | ||
| String email = normalized(request, UserQueryKey.EMAIL); | ||
|
|
||
| if (userId.isEmpty() || password.isEmpty() || name.isEmpty() || email.isEmpty()) { | ||
| log.log(Level.WARNING, "Missing sign-up parameters method={0} userId={1}", new Object[]{method, userId}); | ||
| response.response302Header(UrlPath.USER_SIGNUP_FORM.value()); | ||
| return; | ||
| } | ||
|
|
||
| User user = new User( | ||
| userId, | ||
| password, | ||
| name, | ||
| email); | ||
| repository.addUser(user); | ||
| log.log(Level.INFO, "User signup method={0} userId={1}", new Object[]{method, user.getUserId()}); | ||
| response.response302Header(UrlPath.INDEX.value()); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
가입은 POST 전용으로 제한하고, GET은 폼 포워딩으로 분리하세요. 중복 가입도 막아야 합니다.
현재 GET에서도 모든 파라미터가 채워지면 가입이 진행됩니다. 이는 보안/정합성 측면에서 위험합니다. 가입은 POST에서만 처리하고, GET은 폼을 포워딩하세요. 또한 동일 userId 덮어쓰기를 방지하기 위해 중복 체크가 필요합니다.
아래와 같이 분기 구조를 단순화해 주세요(중복 체크는 Repository 인터페이스에 findUserById가 있다면 추가).
- public void execute(HttpRequest request, HttpResponse response) throws IOException {
- HttpMethod method = request.getMethod();
- if (method != HttpMethod.GET && method != HttpMethod.POST) {
- response.send404(request.getPath());
- return;
- }
-
- String userId = normalized(request, UserQueryKey.USER_ID);
- String password = normalized(request, UserQueryKey.PASSWORD);
- String name = normalized(request, UserQueryKey.NAME);
- String email = normalized(request, UserQueryKey.EMAIL);
-
- if (userId.isEmpty() || password.isEmpty() || name.isEmpty() || email.isEmpty()) {
- log.log(Level.WARNING, "Missing sign-up parameters method={0} userId={1}", new Object[]{method, userId});
- response.response302Header(UrlPath.USER_SIGNUP_FORM.value());
- return;
- }
-
- User user = new User(
- userId,
- password,
- name,
- email);
- repository.addUser(user);
- log.log(Level.INFO, "User signup method={0} userId={1}", new Object[]{method, user.getUserId()});
- response.response302Header(UrlPath.INDEX.value());
- }
+ public void execute(HttpRequest request, HttpResponse response) throws IOException {
+ HttpMethod method = request.getMethod();
+
+ // GET: 폼만 제공
+ if (method == HttpMethod.GET) {
+ response.forward(UrlPath.USER_SIGNUP_FORM.value());
+ return;
+ }
+ // POST 이외: 거부
+ if (method != HttpMethod.POST) {
+ response.send404(request.getPath());
+ return;
+ }
+
+ String userId = normalized(request, UserQueryKey.USER_ID);
+ String password = normalized(request, UserQueryKey.PASSWORD);
+ String name = normalized(request, UserQueryKey.NAME);
+ String email = normalized(request, UserQueryKey.EMAIL);
+
+ if (userId.isEmpty() || password.isEmpty() || name.isEmpty() || email.isEmpty()) {
+ log.log(Level.WARNING, "Missing sign-up parameters method={0} userId={1}", new Object[]{method, userId});
+ response.response302Header(UrlPath.USER_SIGNUP_FORM.value());
+ return;
+ }
+
+ // TODO: Repository에 findUserById가 있다면 중복 체크 추가
+ // if (repository.findUserById(userId) != null) {
+ // response.response302Header(UrlPath.USER_SIGNUP_FORM.value());
+ // return;
+ // }
+
+ User user = new User(userId, password, name, email);
+ repository.addUser(user);
+ log.log(Level.INFO, "User signup method={0} userId={1}", new Object[]{method, user.getUserId()});
+ response.response302Header(UrlPath.INDEX.value());
+ }Repository에 중복 조회 메서드 존재 여부 확인용 스크립트:
🏁 Script executed:
#!/bin/bash
# Repository 인터페이스/구현에 사용자 조회 메서드가 있는지 확인
rg -nP -C2 '(interface|class)\s+Repository\b' | sed -n '1,120p'
rg -nP -C3 'class\s+MemoryUserRepository\b'
rg -nP -C3 'findUserById\s*\('Length of output: 2375
GET 요청은 폼 렌더링으로만 처리하고, POST에서만 가입 로직을 수행하세요
repository.findUserById(String userId)를 사용해 중복 userId를 검사하고, 중복 시 가입을 차단해야 합니다.
🤖 Prompt for AI Agents
In src/main/java/controller/SignUpController.java around lines 20 to 47, the
controller currently runs signup logic for both GET and POST and does not check
for duplicate userIds; update it so GET only renders the signup form and all
signup processing runs only on POST, validate parameters inside the POST branch,
call repository.findUserById(userId) before creating a new User and if a user
already exists log a warning and redirect back to the signup form (no creation),
otherwise create and add the user and redirect to the index; keep the existing
logging and redirects for success/failure paths.
| private static byte[] readAllBytes(File f) throws IOException { | ||
| try (FileInputStream fis = new FileInputStream(f)) { | ||
| byte[] data = new byte[(int) f.length()]; | ||
| int read = fis.read(data); | ||
| if (read != data.length) throw new IOException("Incomplete file read"); | ||
| return data; | ||
| } | ||
| } |
There was a problem hiding this comment.
단일 read 호출로 전체 파일을 읽지 못할 수 있습니다.
Line 130의 fis.read(data)는 한 번의 호출로 전체 배열을 채운다는 보장이 없습니다. 특히 네트워크 파일 시스템이나 느린 디스크에서는 부분 읽기가 발생할 수 있습니다.
다음과 같이 수정하는 것을 권장합니다:
private static byte[] readAllBytes(File f) throws IOException {
- try (FileInputStream fis = new FileInputStream(f)) {
- byte[] data = new byte[(int) f.length()];
- int read = fis.read(data);
- if (read != data.length) throw new IOException("Incomplete file read");
- return data;
- }
+ // Java 7+ Files.readAllBytes를 사용하거나, 루프로 전체 읽기 보장
+ return Files.readAllBytes(f.toPath());
}또는 루프를 사용한 안전한 읽기:
private static byte[] readAllBytes(File f) throws IOException {
try (FileInputStream fis = new FileInputStream(f)) {
byte[] data = new byte[(int) f.length()];
- int read = fis.read(data);
- if (read != data.length) throw new IOException("Incomplete file read");
+ int offset = 0;
+ int remaining = data.length;
+ while (remaining > 0) {
+ int read = fis.read(data, offset, remaining);
+ if (read < 0) throw new IOException("Unexpected EOF");
+ offset += read;
+ remaining -= read;
+ }
return data;
}
}🤖 Prompt for AI Agents
In src/main/java/http/HttpResponse.java around lines 127 to 134, the single-call
fis.read(data) is unsafe because read may return fewer bytes than requested;
replace this with a robust read loop that continues reading until the buffer is
filled or EOF (checking for -1) and throws if EOF reached prematurely, or simply
use Files.readAllBytes(f.toPath()) to get the full file contents; also guard
against f.length() changing by either reading into a ByteArrayOutputStream
(reading chunks) or verifying the final byte count matches the expected length
before returning.
| private final Controller defaultController = new ForwardController(); | ||
|
|
||
| public RequestMapper() { | ||
| // controllers.put(UrlPath.ROOT.value(), new HomeController()); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
주석 처리된 코드를 제거해주세요.
주석 처리된 코드는 버전 관리 시스템에서 관리되므로 제거하는 것이 좋습니다. 필요 시 Git 히스토리에서 확인할 수 있습니다.
- // controllers.put(UrlPath.ROOT.value(), new HomeController());
HomeController homeController = new HomeController();📝 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.
| // controllers.put(UrlPath.ROOT.value(), new HomeController()); | |
| HomeController homeController = new HomeController(); |
🤖 Prompt for AI Agents
In src/main/java/webserver/RequestMapper.java around line 18, remove the
commented-out line "// controllers.put(UrlPath.ROOT.value(), new
HomeController());" from the file so only active code remains; rely on VCS
history if the code is needed later and commit the change with a brief message
indicating removal of dead/commented code.
| <link href="https://cdn.jsdelivr.net/npm/bootstrap@5.3.0-alpha1/dist/css/bootstrap.min.css" rel="stylesheet"> | ||
| <link href="../css/styles.css" rel="stylesheet"> | ||
| </head> | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
Bootstrap JS 제거가 기능에 영향을 주는지 확인이 필요합니다.
Bootstrap JavaScript 번들이 제거되었습니다. 현재 페이지의 네비게이션 바에는 토글 버튼과 드롭다운 메뉴가 있는데(14-16행, 20-26행), 이들은 Bootstrap JS에 의존하는 컴포넌트입니다. 모바일 환경에서 네비게이션 메뉴가 정상 작동하는지 확인해주세요.
다음 스크립트로 Bootstrap JS에 의존하는 컴포넌트 사용 여부를 확인할 수 있습니다:
🏁 Script executed:
#!/bin/bash
# Description: Check for Bootstrap JS-dependent components in HTML files
rg -n "data-bs-toggle|dropdown-toggle|collapse|navbar-toggler" webapp/Length of output: 3865
Bootstrap JS 번들이 제거되어 네비게이션 바 토글·드롭다운이 작동하지 않습니다.
webapp/user/userList.html (15행, 19행, 22행) 등에서 data-bs-toggle, collapse, navbar-toggler, dropdown-toggle를 사용 중이므로 Bootstrap JavaScript 번들을 다시 포함하거나 대체 스크립트를 적용하세요.
🤖 Prompt for AI Agents
In webapp/user/userList.html around line 10 (and referenced lines 15, 19, 22),
Bootstrap JS bundle has been removed causing navbar toggles and dropdowns
(data-bs-toggle, collapse, navbar-toggler, dropdown-toggle) to stop working;
restore interactive behavior by re-adding the Bootstrap JavaScript bundle (or
equivalent Bundle + Popper) to the page — include the appropriate <script>
tag(s) for the Bootstrap JS (and Popper if required) after the page’s DOM and
CSS are loaded, or implement an alternative JavaScript that initializes the
collapse and dropdown components using Bootstrap’s JS API.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/java/webserver/RequestHandler.java (3)
30-37: 불필요한 쿼리 스트링 처리 로직입니다.
HttpRequest.from()메서드는 이미 쿼리 스트링을 파싱하고 경로에서 제거합니다.request.getPath()는 쿼리 스트링이 제거된 경로를 반환하므로, 34-37번 라인의 수동 처리는 중복됩니다. 또한HttpRequest.from()이 유효하지 않은 요청에 대해 IOException을 던지므로, 31번 라인의 null 체크도 불필요할 가능성이 높습니다.다음과 같이 단순화할 수 있습니다:
- String url = request.getPath(); - if (url == null){ - url = "/"; - } - int q = url.indexOf('?'); - if (q>=0) { - url = url.substring(0, q); - } + String url = request.getPath();
50-60: 컨트롤러 기반 라우팅 로직이 적절합니다.RequestMapper를 통한 컨트롤러 조회와 정적 자원 처리 흐름이 명확합니다. 44-48번 라인의 return 문이 수정된다면, 이 로직은 올바르게 동작할 것입니다.
53번 라인의 긴 조건문은 유지보수를 위해 헬퍼 메서드로 추출하는 것을 고려해보세요:
private boolean isStaticResource(String path) { return path.endsWith(".css") || path.endsWith(".js") || path.endsWith(".png") || path.endsWith(".jpg") || path.endsWith(".gif") || path.endsWith(".ico"); }그리고 사용 시:
- if (resource.endsWith(".css") || resource.endsWith(".js") || resource.endsWith(".png") || resource.endsWith(".jpg") || resource.endsWith(".gif") || resource.endsWith(".ico")) { + if (isStaticResource(resource)) {
17-17: RequestMapper 내부 상태 보유 및 공유 안전성 검토
RequestMapper는 mutable HashMap과 Controller 인스턴스를 인스턴스 필드에 보유해 상태ful합니다.
• 싱글톤(static final)으로 공유하려면
– controllers 맵을 Map.of/Collections.unmodifiableMap 등으로 불변화
– Controller 구현이 thread-safe함을 보장
• 별도 조치 없이 현행 유지하려면 각 RequestHandler마다 인스턴스 생성 방식을 유지하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/webserver/RequestHandler.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/webserver/RequestHandler.java (2)
src/main/java/http/HttpRequest.java (1)
HttpRequest(16-152)src/main/java/http/HttpResponse.java (1)
HttpResponse(15-135)
🔇 Additional comments (1)
src/main/java/webserver/RequestHandler.java (1)
39-42: 루트 경로 처리 로직이 올바릅니다.루트 경로와 index.html 요청을 정적 파일로 포워딩하고 early return으로 추가 처리를 방지하는 로직이 적절합니다.
| if(url.endsWith(".html")) { | ||
| String resource = url.startsWith("/") ? url.substring(1) : url; | ||
| // 앞에 / 가 붙으면 제거해주는 역할 | ||
| response.forward(resource); | ||
| } |
There was a problem hiding this comment.
치명적: HTML 파일 처리 후 return 문이 누락되었습니다.
HTML 파일을 forward한 후 return 문이 없어서 실행이 계속됩니다. 이로 인해:
- 47번 라인에서
response.forward(resource)로 HTML을 응답 - 50번 라인에서 컨트롤러를 찾음 (null일 가능성 높음)
- 53번 라인의 확장자 목록에 .html이 없어서
- 56번 라인에서
response.send404()를 다시 호출
결과적으로 하나의 요청에 대해 두 개의 HTTP 응답(200 OK + 404 Not Found)이 전송되어 잘못된 형식의 응답이 생성되고 클라이언트 오류가 발생합니다.
다음과 같이 return 문을 추가하세요:
if(url.endsWith(".html")) {
String resource = url.startsWith("/") ? url.substring(1) : url;
// 앞에 / 가 붙으면 제거해주는 역할
response.forward(resource);
+ 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.
| if(url.endsWith(".html")) { | |
| String resource = url.startsWith("/") ? url.substring(1) : url; | |
| // 앞에 / 가 붙으면 제거해주는 역할 | |
| response.forward(resource); | |
| } | |
| if (url.endsWith(".html")) { | |
| String resource = url.startsWith("/") ? url.substring(1) : url; | |
| // 앞에 / 가 붙으면 제거해주는 역할 | |
| response.forward(resource); | |
| return; | |
| } |
🤖 Prompt for AI Agents
In src/main/java/webserver/RequestHandler.java around lines 44 to 48, after
forwarding .html resources the code currently continues execution which can lead
to a second response being sent; stop further processing by adding an immediate
return right after response.forward(resource) so the method exits once an HTML
response is forwarded, preventing subsequent controller lookup and the 404 send.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/controller/ListController.java (1)
12-16: 쿠키 기반 인증의 보안 개선을 고려하세요.현재 "logined" 쿠키 값만으로 인증을 확인하고 있습니다. 쿠키는 클라이언트가 조작할 수 있어 보안에 취약할 수 있습니다. 프로덕션 환경에서는 서버 측 세션 ID를 사용하는 것이 더 안전합니다.
학습 프로젝트로 보이므로 현재 구현도 괜찮지만, 향후 개선 시 참고하시기 바랍니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/controller/ListController.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/controller/ListController.java (2)
src/main/java/http/HttpRequest.java (1)
HttpRequest(16-152)src/main/java/http/HttpResponse.java (1)
HttpResponse(15-135)
🔇 Additional comments (1)
src/main/java/controller/ListController.java (1)
24-28: 이전 리뷰 이슈가 해결되었습니다!
/user/list와/user/list.html경로를 모두user/list.html로 올바르게 포워딩하고 있어, 이전에 지적된 404 위험이 해결되었습니다.
| // 4) 그 외는 필요에 따라 404 또는 다른 처리 | ||
| response.send404(normalized); |
There was a problem hiding this comment.
🧩 Analysis chain
404 처리 로직이 실제로 실행되는지 검증이 필요합니다.
ListController가 /user/list 관련 경로만 처리해야 한다면, 30-31번 줄의 404 처리는 실행되지 않을 수 있습니다. RequestMapper가 어떤 경로를 이 컨트롤러로 라우팅하는지 확인이 필요합니다.
다음 스크립트로 라우팅 매핑을 확인하세요:
🏁 Script executed:
#!/bin/bash
# RequestMapper에서 ListController로 라우팅되는 경로 확인
rg -n -A 3 -B 3 'ListController' --type=javaLength of output: 2022
아래 스크립트로 UrlPath enum 정의와 ListController 구현을 확인하세요:
🏁 Script executed:
#!/bin/bash
# UrlPath enum 정의 및 사용자 리스트 관련 경로 확인
rg -n 'USER_LIST' --type=java
# ListController.execute 구현 전체 보기
sed -n '1,200p' src/main/java/controller/ListController.javaLength of output: 2099
Alias 경로(/user/userList, /user/userList.html) 처리를 추가해야 합니다.
RequestMapper에서 USER_LIST_ALIAS, USER_LIST_HTML이 ListController로 라우팅되지만 execute 메서드는 “user/list”와 “user/list.html”만 포워딩하며 그 외(별칭)는 send404를 호출합니다. normalized.equals 비교에 alias 값도 포함하거나 UrlPath를 활용해 처리 로직을 확장하세요.
🤖 Prompt for AI Agents
In src/main/java/controller/ListController.java around lines 30-31, the execute
method currently only forwards "user/list" and "user/list.html" and calls
response.send404 for other normalized paths, but RequestMapper routes alias
paths like USER_LIST_ALIAS and USER_LIST_HTML to this controller; update the
path comparison to include those alias values (or use UrlPath utility to
normalize/compare canonical paths) so that comparisons accept "/user/userList"
and "/user/userList.html" as valid and forward them to the same view instead of
sending a 404. Ensure you add the alias constants to the equals checks (or
convert normalized to UrlPath and match against canonical path variants) and
preserve existing forwarding behavior for the two original paths.
| // String resource = "user/list.html"; | ||
| // if (UrlPath.USER_LIST.value().equals(path) || UrlPath.USER_LIST_HTML.value().equals(path)) { | ||
| // resource = path.startsWith("/") ? path.substring(1) : path; | ||
| // } | ||
| // response.forward(resource); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
주석 처리된 코드를 삭제하세요.
이전 구현이 주석으로 남아있습니다. 버전 관리 시스템(Git)에서 히스토리를 확인할 수 있으므로, 주석 코드는 삭제하는 것이 코드 가독성에 좋습니다.
-
-// String resource = "user/list.html";
-// if (UrlPath.USER_LIST.value().equals(path) || UrlPath.USER_LIST_HTML.value().equals(path)) {
-// resource = path.startsWith("/") ? path.substring(1) : path;
-// }
-// response.forward(resource);📝 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 resource = "user/list.html"; | |
| // if (UrlPath.USER_LIST.value().equals(path) || UrlPath.USER_LIST_HTML.value().equals(path)) { | |
| // resource = path.startsWith("/") ? path.substring(1) : path; | |
| // } | |
| // response.forward(resource); |
🤖 Prompt for AI Agents
In src/main/java/controller/ListController.java around lines 33 to 37, remove
the block of commented-out code (the old resource assignment and conditional
forwarding) — delete those commented lines entirely to improve readability and
rely on Git history for past implementations; then run project build/tests and
commit the change with a message like "Remove commented-out legacy code in
ListController".
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/main/java/controller/SignUpController.java (3)
18-18: 이전 리뷰에서 지적된 스레드 안전성 문제가 여전히 해결되지 않았습니다.MemoryUserRepository는 여전히 HashMap과 비동기 안전하지 않은 싱글턴 패턴을 사용하고 있어 멀티스레드 환경에서 경쟁 상태와 데이터 손상 위험이 있습니다. 이전 리뷰 코멘트를 참고하여 ConcurrentHashMap과 안전한 싱글턴 패턴으로 교체해주세요.
53-60: 중복 userId 검사가 누락되었습니다.현재 코드는 동일한 userId를 가진 사용자가 이미 존재하는지 확인하지 않고 바로 저장하여 기존 사용자 정보를 덮어쓸 수 있습니다. 이전 리뷰에서 지적된 사항이지만 아직 구현되지 않았습니다.
다음과 같이 중복 체크를 추가하세요:
if (userId.isEmpty() || password.isEmpty() || name.isEmpty() || email.isEmpty()) { log.log(Level.WARNING, "Missing sign-up parameters method={0} userId={1}", new Object[]{method, userId}); response.response302Header(UrlPath.USER_SIGNUP_FORM.value()); return; } + // 중복 사용자 체크 + if (repository.findUserById(userId) != null) { + log.log(Level.WARNING, "User already exists userId={0}", userId); + response.response302Header(UrlPath.USER_SIGNUP_FORM.value()); + return; + } + User user = new User( userId, password, name, email);
31-35: GET 요청 시 회원가입 폼을 제공해야 합니다.현재 GET 요청이
/index.html로 리다이렉트되고 있어 사용자가 회원가입 폼에 접근할 수 없습니다. 이전 리뷰 코멘트에서 제안한 대로 GET 요청은 회원가입 폼을 포워딩해야 합니다.다음과 같이 수정하세요:
if (method == HttpMethod.GET) { - // response.forward("user/form.html"); - response.response302Header("/index.html"); + response.forward(UrlPath.USER_SIGNUP_FORM.value()); return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/controller/SignUpController.java(1 hunks)src/main/java/http/HttpResponse.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/http/HttpResponse.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/controller/SignUpController.java (4)
src/main/java/db/MemoryUserRepository.java (1)
MemoryUserRepository(9-35)src/main/java/http/HttpRequest.java (1)
HttpRequest(16-152)src/main/java/http/HttpResponse.java (1)
HttpResponse(15-134)src/main/java/model/User.java (1)
User(5-46)
🔇 Additional comments (2)
src/main/java/controller/SignUpController.java (2)
37-51: 파라미터 검증 로직이 적절합니다.비POST 메소드 거부, 파라미터 추출 및 검증, 실패 시 적절한 로깅과 리다이렉트 처리가 잘 되어 있습니다.
63-66: 헬퍼 메소드 구현이 깔끔합니다.파라미터 추출 로직을 중앙화하여 코드 중복을 방지하고, null 처리와 공백 제거가 적절히 구현되었습니다.
| // Requirement 2 : GET 방식으로 회원가입하기 | ||
| // if (method != HttpMethod.GET && method != HttpMethod.POST) { | ||
| // response.send404(request.getPath()); | ||
| // return; | ||
| // } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
주석 처리된 코드를 제거하세요.
더 이상 사용하지 않는 코드는 버전 관리 시스템에 기록되어 있으므로 삭제하는 것이 좋습니다. 코드베이스를 깔끔하게 유지할 수 있습니다.
다음과 같이 주석 처리된 코드를 제거하세요:
- // Requirement 2 : GET 방식으로 회원가입하기
-// if (method != HttpMethod.GET && method != HttpMethod.POST) {
-// response.send404(request.getPath());
-// return;
-// }
-
// Requirement 3 : POST 방식으로 회원가입하기📝 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.
| // Requirement 2 : GET 방식으로 회원가입하기 | |
| // if (method != HttpMethod.GET && method != HttpMethod.POST) { | |
| // response.send404(request.getPath()); | |
| // return; | |
| // } | |
| // Requirement 3 : POST 방식으로 회원가입하기 |
🤖 Prompt for AI Agents
In src/main/java/controller/SignUpController.java around lines 24 to 28, remove
the commented-out HTTP method check block (the three-line commented code that
checks if method is GET or POST) entirely — delete those commented lines from
the file, ensure no other code depends on them, recompile/run tests to verify
nothing breaks, and commit the cleaned-up file.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
src/main/java/controller/ListController.java (1)
20-44: 주석 처리된 코드와 도달 불가능한 코드를 제거하세요.Lines 20-42의 주석 처리된 코드는 Git 히스토리로 관리할 수 있으므로 제거해야 합니다. 또한 Line 44의
send404호출은 Line 18에서 이미forward를 호출하고return한 후이므로 절대 실행되지 않습니다.다음과 같이 정리하세요:
response.forward(UrlPath.USER_LIST_HTML.value()); - -// String path = request.getPath(); -// if (path == null) path = "/user/list"; -// int q = path.indexOf('?'); -// if (q >= 0) path = path.substring(0, q); -// String normalized = path.startsWith("/") ? path.substring(1) : path; -// -// // 3) 표준 경로만 서비스 (user/list, user/list.html, user/list/ 허용) -// if (normalized.equals("user/list") || normalized.equals("user/list.html") || normalized.equals("user/list/")) { -// response.forward("user/list.html"); // 실제 파일로 고정 -// return; -// } -// -// // 4) 과거/실수 경로 보정: /user/userList* → /user/list 로 리다이렉트 -// if (normalized.equals("user/userList") || normalized.equals("user/userList.html") || normalized.equals("user/userList/")) { -// response.response302Header("/user/list.html"); -// return; -// } - -// if (normalized.startsWith("user/userList")) { -// response.forward("user/list.html"); -// return; -// } - - // 4) 그 외는 필요에 따라 404 또는 다른 처리 - response.send404("/user/userList"); - }src/main/java/controller/LoginController.java (2)
33-36: 입력 검증이 누락되어 NullPointerException 위험이 있습니다.Line 33과 Line 36에서
request.getParameter()가 null을 반환할 수 있으므로 NPE가 발생할 수 있습니다. 이미 이전 리뷰에서 지적된 사항입니다.
36-36: 평문 비밀번호 비교는 보안 취약점입니다.Line 36에서 평문 비밀번호를 직접 비교하고 있습니다. BCrypt 또는 Argon2와 같은 해시 알고리즘을 사용해야 합니다. 이미 이전 리뷰에서 지적된 사항입니다.
src/main/java/webserver/RequestMapper.java (1)
30-39: 주석 처리된 코드를 제거하세요.Lines 30-39의 주석 처리된 이전 구현은 Git 히스토리로 관리할 수 있으므로 제거하는 것이 코드 가독성에 좋습니다.
public Controller findController(String rawUrl) { -// if (url == null) { -// return defaultController; -// } -// Controller controller = controllers.get(url); -// if (controller != null) { -// return controller; -// } -// if (url.endsWith(".html")) { -// return defaultController; -// } String url = normalize(rawUrl);
🧹 Nitpick comments (2)
src/main/java/http/HttpRequest.java (1)
59-61: 불필요한 null 체크를 제거하세요.
HttpRequestUtils.parseQueryParameter는 항상 비어있지 않은 Map을 반환하므로(관련 코드 참조), Line 59의 null 체크는 불필요합니다.다음과 같이 단순화할 수 있습니다:
Map<String, String> queryParams = HttpRequestUtils.parseQueryParameter(query); -if (queryParams != null) { - params.putAll(queryParams); -} +params.putAll(queryParams);src/main/java/webserver/RequestMapper.java (1)
43-44: 하드코딩된 URL 별칭 로직을 UrlPath로 중앙화하세요.Lines 43-44의 수동 URL 별칭 매핑은 유지보수를 어렵게 만듭니다. 이 로직은
UrlPathenum 또는 별도의 설정으로 중앙화하는 것이 좋습니다.예를 들어,
UrlPath에 별칭 관계를 정의하거나 Map 기반 별칭 설정을 추가할 수 있습니다:// UrlPath에 별칭 메서드 추가 public String getCanonicalPath() { // USER_LIST_ALIAS -> USER_LIST의 canonical path 반환 }또는 생성자에서 별칭 매핑을 설정:
public RequestMapper() { // ... 기존 매핑 ... // 별칭 처리를 위한 중앙화된 매핑 Map<String, String> aliases = Map.of( "/user/userlist", "/user/list", "/user/userlist.html", "/user/list.html" ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/controller/ListController.java(1 hunks)src/main/java/controller/LoginController.java(1 hunks)src/main/java/http/HttpRequest.java(1 hunks)src/main/java/webserver/RequestMapper.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/main/java/controller/ListController.java (2)
src/main/java/http/HttpRequest.java (1)
HttpRequest(16-161)src/main/java/http/HttpResponse.java (1)
HttpResponse(15-134)
src/main/java/webserver/RequestMapper.java (5)
src/main/java/controller/ForwardController.java (1)
ForwardController(10-24)src/main/java/controller/HomeController.java (1)
HomeController(8-13)src/main/java/controller/ListController.java (1)
ListController(9-47)src/main/java/controller/LoginController.java (1)
LoginController(17-45)src/main/java/controller/SignUpController.java (1)
SignUpController(16-67)
src/main/java/controller/LoginController.java (4)
src/main/java/db/MemoryUserRepository.java (1)
MemoryUserRepository(9-35)src/main/java/http/HttpRequest.java (1)
HttpRequest(16-161)src/main/java/http/HttpResponse.java (1)
HttpResponse(15-134)src/main/java/model/User.java (1)
User(5-46)
src/main/java/http/HttpRequest.java (3)
src/main/java/http/util/HttpRequestUtils.java (1)
HttpRequestUtils(10-43)src/main/java/http/util/IOUtils.java (1)
IOUtils(6-21)src/main/java/http/HttpStartLine.java (1)
HttpStartLine(5-34)
| public String getCookie(String name) { | ||
| String cookieHeader = getHeader("Cookie"); // 요청 헤더에서 꺼내기 | ||
| if (cookieHeader == null) return null; | ||
| for (String pair : cookieHeader.split(";")) { | ||
| String[] kv = pair.trim().split("=", 2); | ||
| if (kv.length == 2 && kv[0].trim().equals(name)) { | ||
| return kv[1].trim(); | ||
| } | ||
| } | ||
| return null; | ||
| // return cookies.get(name); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
중복된 쿠키 파싱 로직과 주석 처리된 코드를 정리하세요.
getCookie 메서드가 parseCookies 헬퍼와 동일한 로직을 중복 구현하고 있습니다. Line 140의 주석 처리된 코드는 cookies 필드를 사용하지 않음을 나타내며, 이는 Line 82에서 파싱한 cookies 맵이 실제로 사용되지 않음을 의미합니다.
다음 중 하나를 선택하세요:
옵션 1: 파싱된 cookies 맵을 활용
public String getCookie(String name) {
- String cookieHeader = getHeader("Cookie");
- if (cookieHeader == null) return null;
- for (String pair : cookieHeader.split(";")) {
- String[] kv = pair.trim().split("=", 2);
- if (kv.length == 2 && kv[0].trim().equals(name)) {
- return kv[1].trim();
- }
- }
- return null;
- // return cookies.get(name);
+ return cookies.get(name);
}옵션 2: cookies 필드를 제거하고 현재 로직 유지
- Line 20의
cookies필드 제거 - Line 82의
parseCookies호출 제거 - Line 147-160의
parseCookies메서드 제거 - 생성자에서
cookies파라미터 제거
📝 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.
| public String getCookie(String name) { | |
| String cookieHeader = getHeader("Cookie"); // 요청 헤더에서 꺼내기 | |
| if (cookieHeader == null) return null; | |
| for (String pair : cookieHeader.split(";")) { | |
| String[] kv = pair.trim().split("=", 2); | |
| if (kv.length == 2 && kv[0].trim().equals(name)) { | |
| return kv[1].trim(); | |
| } | |
| } | |
| return null; | |
| // return cookies.get(name); | |
| } | |
| public String getCookie(String name) { | |
| return cookies.get(name); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/http/HttpRequest.java around lines 130 to 141, getCookie
duplicates the cookie-parsing logic and leaves a commented return; instead use
the already-parsed cookies map: replace the loop with a single return
cookies.get(name) (remove the commented code), ensure the constructor still
calls parseCookies and that parseCookies populates the cookies field, remove the
duplicated parsing code only (keep parseCookies and the cookies field), and run
a quick compile to confirm no unused-field or null-safety issues (add a null
check on cookies if needed).
| int q = url.indexOf('?'); if (q >= 0) url = url.substring(0, q); | ||
| if (!url.startsWith("/")) url = "/" + url; | ||
| if (url.endsWith("/") && url.length() > 1) url = url.substring(0, url.length()-1); | ||
| return url.toLowerCase(); // 대소문자 흔들림 방지 |
There was a problem hiding this comment.
대소문자 정규화가 파일 시스템과 충돌할 수 있습니다.
Line 63의 toLowerCase() 호출은 URL을 소문자로 정규화하지만, Linux/Unix와 같은 대소문자를 구분하는 파일 시스템에서는 실제 파일명과 일치하지 않을 수 있습니다. 예를 들어, User.html이라는 파일이 있을 때 /User.html 요청이 /user.html로 정규화되어 404가 발생할 수 있습니다.
다음 중 하나를 선택하세요:
옵션 1: 대소문자 정규화 제거 (권장)
- return url.toLowerCase();
+ return url;옵션 2: 컨트롤러 매핑에만 적용하고 ForwardController는 원본 유지
- 매핑 키는 소문자로 저장
rawUrl을 정규화하지 않고 컨트롤러에 전달하여 ForwardController가 원본 경로를 사용하도록 함
📝 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.
| return url.toLowerCase(); // 대소문자 흔들림 방지 | |
| return url; |
🤖 Prompt for AI Agents
In src/main/java/webserver/RequestMapper.java around line 63, the call to
url.toLowerCase() normalizes case and can break case-sensitive file systems
(e.g., User.html -> /user.html causing 404); remove the toLowerCase()
normalization so the original URL case is preserved, or if you prefer Option 2,
apply toLowerCase() only when building controller mapping keys while keeping and
passing the original rawUrl unchanged to the ForwardController; update related
variables and usages so mapping lookups use the normalized key but file
forwarding uses the original URL.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/http/HttpResponse.java (1)
122-134: 콘텐츠 타입 추정 로직이 기본 요구사항을 충족합니다.일반적인 웹 리소스 타입에 대한 MIME 타입 매핑이 잘 구현되어 있습니다.
필요하다면 향후 폰트 파일(.woff, .woff2, .ttf), 비디오(.mp4, .webm) 등 추가 타입을 고려할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/http/HttpResponse.java(1 hunks)
🔇 Additional comments (2)
src/main/java/http/HttpResponse.java (2)
73-84: 리다이렉트 헤더 처리가 적절합니다.302 응답에서 기존 헤더를 초기화하고
SET_COOKIE만 보존하는 로직이 잘 구현되었습니다. 리다이렉트에 필요한 최소한의 헤더만 유지하는 것이 올바른 접근입니다.
95-105: 404 응답 처리가 적절합니다.기본적인 404 응답 구현이 깔끔하게 되어 있습니다. 필요한 헤더와 간단한 HTML 본문을 포함하고 있어 기본 기능으로는 충분합니다.
| // if (contentType == null) { | ||
| // contentType = guessContentType(resourcePath); | ||
| // } |
There was a problem hiding this comment.
주석 처리된 코드를 제거하세요.
사용하지 않는 주석 처리된 코드는 코드베이스를 어지럽히고 혼란을 야기할 수 있습니다. 더 이상 필요하지 않다면 제거하는 것이 좋습니다.
🤖 Prompt for AI Agents
In src/main/java/http/HttpResponse.java around lines 60 to 62, remove the
commented-out block that checks and assigns contentType (the three commented
lines) so the file contains no unused commented code; delete those lines and
recompile/format to ensure no leftover whitespace or warnings.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
src/main/java/controller/LoginController.java (1)
30-33: 입력 검증 및 평문 비밀번호 처리 개선 필요이 부분에 대한 두 가지 중요한 이슈가 이전 리뷰에서 이미 지적되었습니다:
request.getParameter()가 null을 반환할 수 있어 NPE 위험이 있습니다.- 평문 비밀번호 비교는 보안 취약점입니다.
이전 리뷰 코멘트를 참고하여 수정해 주세요.
src/main/java/http/HttpResponse.java (5)
34-77: 대용량 파일 처리 및 코드 정리 필요이 메서드에 대해 이전 리뷰에서 지적된 대용량 파일 메모리 문제를 해결해 주세요.
53-68: 주석 처리된 코드를 모두 제거하세요.Lines 53-68에 여러 블록의 주석 처리된 코드가 있습니다. 사용하지 않는 코드는 버전 관리 시스템에 기록되므로 삭제하는 것이 좋습니다.
다음 줄들을 삭제하세요:
- Lines 53, 61-64, 66-68
92-99: IOException 처리 개선 필요이전 리뷰에서 지적된 대로, IOException을 삼켜버리지 말고 상위로 전파하거나 더 명확한 에러 처리를 해야 합니다.
113-121: BufferedWriter 리소스 누수 해결 필요이전 리뷰에서 지적된 BufferedWriter 리소스 누수 문제를 해결해 주세요.
142-149: 파일 읽기 로직 개선 필요이전 리뷰에서 지적된 대로, 단일
read()호출로는 전체 파일을 읽는다는 보장이 없습니다.Files.readAllBytes()를 사용하거나 루프로 안전하게 읽어야 합니다.
🧹 Nitpick comments (4)
src/main/java/controller/LoginController.java (2)
25-28: 보안 모니터링을 위한 로깅 추가를 고려하세요.POST가 아닌 메서드로 로그인 시도가 들어올 경우를 로깅하면 잠재적인 공격 시도를 모니터링하는 데 도움이 될 수 있습니다.
다음과 같이 로깅을 추가할 수 있습니다:
if (request.getMethod() != HttpMethod.POST) { + log.log(Level.WARNING, "Invalid method for login: {0}", request.getMethod()); response.response302Header(UrlPath.USER_LOGIN_FAILED.value()); return; }
34-34: 쿠키 보안 속성이 잘 설정되어 있습니다.
HttpOnly와SameSite=Lax속성을 사용하는 것은 XSS와 CSRF 공격을 방어하는 좋은 보안 관행입니다.선택사항: 쿠키의
Path=/는 모든 경로에서 쿠키를 전송하므로, 필요한 경로로만 제한하는 것을 고려해 보세요. 또한 프로덕션 환경에서는Secure속성도 추가하는 것이 좋습니다.src/main/java/http/FormatType.java (1)
25-33: LGTM!파일명으로 Content-Type을 찾는 로직이 올바르게 구현되어 있습니다. 대소문자를 구분하지 않는 매칭과 기본값 처리가 적절합니다.
선택사항: 성능이 중요한 경우, 확장자를 키로 하는 정적 Map을 미리 생성해두면 조회 성능을 개선할 수 있습니다. 하지만 현재 구현도 파일 개수가 적으므로 충분히 실용적입니다.
src/main/java/http/HttpResponse.java (1)
128-140: 중복된 guessContentType 메서드 제거 고려
FormatType.fromFilename()메서드가 동일한 기능을 제공하므로, 이 private 메서드는 더 이상 필요하지 않습니다. 코드 중복을 줄이기 위해 제거를 고려해 보세요.실제로 Line 55에서
FormatType.fromFilename()을 이미 사용하고 있으므로, 이 메서드는 사용되지 않을 가능성이 높습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/controller/ListController.java(1 hunks)src/main/java/controller/LoginController.java(1 hunks)src/main/java/http/CookieName.java(1 hunks)src/main/java/http/FormatType.java(1 hunks)src/main/java/http/HttpResponse.java(1 hunks)src/main/java/webserver/RequestMapper.java(1 hunks)src/main/java/webserver/UrlPath.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/webserver/UrlPath.java
- src/main/java/webserver/RequestMapper.java
- src/main/java/controller/ListController.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/controller/LoginController.java (2)
src/main/java/db/MemoryUserRepository.java (1)
MemoryUserRepository(9-35)src/main/java/model/User.java (1)
User(5-46)
🔇 Additional comments (5)
src/main/java/http/CookieName.java (1)
1-8: LGTM!쿠키 이름을 enum으로 관리하는 것은 좋은 설계입니다. 타입 안전성을 제공하고 오타를 방지할 수 있습니다.
src/main/java/controller/LoginController.java (1)
18-23: LGTM!GET 요청에 대한 로그인 페이지 포워딩 로직이 명확하고 적절합니다.
src/main/java/http/FormatType.java (1)
4-14: LGTM!일반적인 웹 포맷에 대한 MIME 타입 매핑이 잘 정의되어 있습니다. HTML, JS, JSON에 charset을 명시한 것도 좋은 관행입니다.
src/main/java/http/HttpResponse.java (2)
22-32: LGTM!생성자와 헤더 추가 메서드가 깔끔하게 구현되어 있습니다. HttpHeader enum을 받는 오버로드도 타입 안전성을 제공합니다.
79-90: LGTM!302 리다이렉트 구현이 올바릅니다. Set-Cookie 헤더를 보존하는 로직이 특히 좋습니다.
2 participants
연휴 이용해서 나머지 끝장내보겠습니다..
풀 수 있을거 같은데 안되는 부분이 있으니 답답하군요 연휴 이용해서 계속해보겠습니다
Summary by CodeRabbit
신기능
변경사항
테스트
잡일