Open
Conversation
WalkthroughHTTP request parsing, routing, and response generation were introduced. New classes handle start line, headers, methods, URL mapping, and request/response. A RequestMapper dispatches to controllers (home, signup, login, list, forward). RequestHandler now delegates to this flow. IDE JDK/Gradle settings updated. Minor HTML changes to index and form. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant RH as RequestHandler
participant HR as HttpRequest
participant HM as HttpMapper
participant CT as Controller
participant HS as HttpResponse
C->>RH: TCP request
RH->>HR: parse start line, headers, body
RH->>HS: create response stream
RH->>HM: new RequestMapper(HR, HS)
HM->>HM: select controller by URL/method
RH->>HM: proceed()
HM->>CT: execute(request, response)
CT->>HS: forward()/redirect()
HS-->>C: HTTP response
sequenceDiagram
autonumber
participant C as Client (POST /user/signup)
participant RH as RequestHandler
participant HM as RequestMapper
participant SU as SignUpController
participant HS as HttpResponse
C->>RH: POST /user/signup (form body)
RH->>HM: map to SignUpController
HM->>SU: execute(request, response)
SU->>HS: redirect to /index.html (302)
HS-->>C: 302 with Location
sequenceDiagram
autonumber
participant C as Client (POST /user/login)
participant RH as RequestHandler
participant HM as RequestMapper
participant LG as LoginController
participant HS as HttpResponse
C->>RH: POST /user/login (credentials)
RH->>HM: map to LoginController
HM->>LG: execute(request, response)
alt password match
LG->>HS: 302 /index.html (+ Set-Cookie)
else mismatch
LG->>HS: 302 /user/login_failed.html
end
HS-->>C: Redirect response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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: 19
🧹 Nitpick comments (4)
src/main/java/http/HttpResponse.java (1)
19-22: String 비교 시 equals 대신 경로 기반 라우팅을 권장합니다.특정 경로(
"/user/login_failed.html")에 대해 하드코딩된 문자열 비교로 분기하는 것은 유지보수를 어렵게 만듭니다. 새로운 특수 경로가 추가될 때마다 이 메서드를 수정해야 합니다.경로별 응답 전략을 외부에서 주입받거나, 별도의 메서드로 분리하는 것을 고려해보세요.
src/main/java/http/controller/SignUpController.java (1)
20-20: 메서드 이름이 용도와 맞지 않습니다.회원가입 성공 후
redirectLogin()메서드를 호출하는 것은 의미상 혼란스럽습니다. 이 메서드는 로그인 쿠키를 설정하지만, 회원가입만으로 자동 로그인되는 것이 의도된 동작인지 명확하지 않습니다.의도에 따라:
- 회원가입 후 자동 로그인이 맞다면 메서드명을
redirect()또는redirectWithLogin()으로 변경- 자동 로그인이 아니라면 로그인 페이지로 리다이렉트하도록 수정
response.response302Header(dos, "/user/login.html");src/main/java/http/HttpRequest.java (1)
33-35: 메서드 이름을 표준에 맞게 변경하세요.HTTP에서는 일반적으로 "URL" 대신 "URI" 또는 "path"라는 용어를 사용합니다.
getUrl()메서드 이름이getUri()또는getPath()로 변경되면 더 명확하고 표준에 부합합니다.- public String getUrl(){ + public String getPath(){ return startLine.getTarget(); }src/main/java/http/RequestMapper.java (1)
27-28: 동일한 컨트롤러를 중복 매핑하고 있습니다.
URL.DEFAULT와URL.INDEX모두HomeController의 새 인스턴스로 매핑되고 있습니다. 두 경로가 동일한 동작을 하는 경우 하나의 인스턴스를 재사용하는 것이 메모리 효율적입니다.+ HomeController homeController = new HomeController(); controllerMap.put("forward", new ForwardController()); - controllerMap.put(URL.DEFAULT.getUrl(), new HomeController()); - controllerMap.put(URL.INDEX.getUrl(), new HomeController()); + controllerMap.put(URL.DEFAULT.getUrl(), homeController); + controllerMap.put(URL.INDEX.getUrl(), homeController); controllerMap.put(URL.USER_SIGNUP.getUrl(), new SignUpController());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
.idea/gradle.xml(1 hunks).idea/misc.xml(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/RequestMapper.java(1 hunks)src/main/java/http/controller/Controller.java(1 hunks)src/main/java/http/controller/ForwardController.java(1 hunks)src/main/java/http/controller/HomeController.java(1 hunks)src/main/java/http/controller/ListController.java(1 hunks)src/main/java/http/controller/LoginController.java(1 hunks)src/main/java/http/controller/SignUpController.java(1 hunks)src/main/java/webserver/Path.java(1 hunks)src/main/java/webserver/RequestHandler.java(1 hunks)src/main/java/webserver/URL.java(1 hunks)webapp/index.html(1 hunks)webapp/user/form.html(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
src/main/java/http/controller/HomeController.java (2)
src/main/java/http/HttpRequest.java (1)
HttpRequest(8-44)src/main/java/http/HttpResponse.java (1)
HttpResponse(9-83)
src/main/java/http/controller/LoginController.java (6)
src/main/java/db/MemoryUserRepository.java (1)
MemoryUserRepository(9-35)src/main/java/http/HttpRequest.java (1)
HttpRequest(8-44)src/main/java/http/HttpResponse.java (1)
HttpResponse(9-83)src/main/java/http/util/HttpRequestUtils.java (1)
HttpRequestUtils(8-20)src/main/java/http/util/IOUtils.java (1)
IOUtils(6-21)src/main/java/model/User.java (1)
User(5-46)
src/main/java/http/controller/Controller.java (2)
src/main/java/http/HttpRequest.java (1)
HttpRequest(8-44)src/main/java/http/HttpResponse.java (1)
HttpResponse(9-83)
src/main/java/http/controller/SignUpController.java (5)
src/main/java/db/MemoryUserRepository.java (1)
MemoryUserRepository(9-35)src/main/java/http/HttpRequest.java (1)
HttpRequest(8-44)src/main/java/http/HttpResponse.java (1)
HttpResponse(9-83)src/main/java/http/util/HttpRequestUtils.java (1)
HttpRequestUtils(8-20)src/main/java/model/User.java (1)
User(5-46)
src/main/java/http/controller/ListController.java (2)
src/main/java/http/HttpRequest.java (1)
HttpRequest(8-44)src/main/java/http/HttpResponse.java (1)
HttpResponse(9-83)
src/main/java/http/HttpRequest.java (3)
src/main/java/http/util/IOUtils.java (1)
IOUtils(6-21)src/main/java/http/HttpHeader.java (1)
HttpHeader(8-41)src/main/java/http/HttpStartLine.java (1)
HttpStartLine(7-30)
src/main/java/http/controller/ForwardController.java (2)
src/main/java/http/HttpRequest.java (1)
HttpRequest(8-44)src/main/java/http/HttpResponse.java (1)
HttpResponse(9-83)
src/main/java/webserver/RequestHandler.java (5)
src/main/java/db/MemoryUserRepository.java (1)
MemoryUserRepository(9-35)src/main/java/http/HttpRequest.java (1)
HttpRequest(8-44)src/main/java/http/HttpResponse.java (1)
HttpResponse(9-83)src/main/java/http/RequestMapper.java (1)
RequestMapper(10-37)src/main/java/http/controller/ForwardController.java (1)
ForwardController(8-13)
🔇 Additional comments (3)
webapp/user/form.html (1)
58-58: 회원가입 폼의 POST 메서드 사용이 적절합니다.비밀번호와 같은 민감한 정보를 전송하는 회원가입 폼에 POST 메서드를 사용한 것은 올바른 선택입니다.
src/main/java/http/controller/Controller.java (1)
8-10: 깔끔한 컨트롤러 인터페이스 설계입니다.단일 책임을 가진 간결한 인터페이스로, 여러 컨트롤러 구현체에서 일관된 계약을 제공합니다.
src/main/java/http/controller/HomeController.java (1)
8-13: 간단하고 명확한 컨트롤러 구현입니다.홈 페이지로 포워딩하는 로직이 명확하게 구현되어 있습니다.
Comment on lines
+42
to
+44
| if(logined.equals("true")){ | ||
| response.forward( "/user/list.html"); | ||
| } |
There was a problem hiding this comment.
로그인되지 않은 경우 응답이 비어 있습니다.
Line 42에서 logined 쿠키가 없거나 "true"가 아닌 경우 아무 응답도 쓰지 않은 채 메서드가 종료되어 클라이언트가 무기한 대기하거나 연결이 끊어집니다. 최소한 로그인 페이지로 포워드하거나 실패 페이지로 리다이렉트하여 명시적 응답을 내려야 합니다.
다음과 같이 보완해 주세요:
- if(logined.equals("true")){
- response.forward( "/user/list.html");
- }
+ if ("true".equals(logined)) {
+ response.forward(URL.USER_USERLIST_HTML.getUrl());
+ return;
+ }
+ response.forward(URL.USER_LOGIN_HTML.getUrl());📝 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
| if(logined.equals("true")){ | |
| response.forward( "/user/list.html"); | |
| } | |
| if ("true".equals(logined)) { | |
| response.forward(URL.USER_USERLIST_HTML.getUrl()); | |
| return; | |
| } | |
| response.forward(URL.USER_LOGIN_HTML.getUrl()); |
🤖 Prompt for AI Agents
In src/main/java/http/controller/ListController.java around lines 42 to 44, the
current code only forwards to /user/list.html when logined.equals("true") and
leaves the method with no response when the logined cookie is missing or not
"true"; add an explicit else branch that handles unauthenticated requests by
forwarding or redirecting to the login page (e.g., forward to /user/login.html
or sendRedirect("/user/login.html")), and ensure you null-check the logined
cookie string before calling equals to avoid NPE (use "true".equals(logined) or
a null check); also consider setting an appropriate HTTP status (e.g., 302 for
redirect or 401/403 if preferred).
Comment on lines
+14
to
+21
| public void execute(HttpRequest request, HttpResponse response) throws IOException { | ||
| MemoryUserRepository db = MemoryUserRepository.getInstance(); | ||
| Map<String, String> userInfos = HttpRequestUtils | ||
| .parseQueryParameter(request.getBody()); | ||
| db.addUser(new User(userInfos.get("userId") | ||
| , userInfos.get("password"), userInfos.get("name"), userInfos.get("email"))); | ||
| response.redirectLogin("/index.html"); | ||
| } |
There was a problem hiding this comment.
입력 검증이 전혀 없어 NullPointerException이 발생할 수 있습니다.
userInfos.get()로 가져온 값들에 대한 null 체크나 유효성 검증이 없습니다. 필수 필드가 누락되거나 중복된 사용자 ID가 입력되면 런타임 오류가 발생합니다.
다음과 같이 검증 로직을 추가하세요:
public void execute(HttpRequest request, HttpResponse response) throws IOException {
MemoryUserRepository db = MemoryUserRepository.getInstance();
Map<String, String> userInfos = HttpRequestUtils
.parseQueryParameter(request.getBody());
+
+ // 필수 필드 검증
+ String userId = userInfos.get("userId");
+ String password = userInfos.get("password");
+ String name = userInfos.get("name");
+ String email = userInfos.get("email");
+
+ if (userId == null || userId.trim().isEmpty() ||
+ password == null || password.trim().isEmpty() ||
+ name == null || name.trim().isEmpty() ||
+ email == null || email.trim().isEmpty()) {
+ response.forward("/user/form.html"); // 또는 에러 페이지
+ return;
+ }
+
+ // 중복 체크
+ if (db.findUserById(userId) != null) {
+ response.forward("/user/form.html"); // 또는 에러 메시지
+ return;
+ }
+
- db.addUser(new User(userInfos.get("userId")
- , userInfos.get("password"), userInfos.get("name"), userInfos.get("email")));
+ db.addUser(new User(userId, password, name, email));
response.redirectLogin("/index.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
| public void execute(HttpRequest request, HttpResponse response) throws IOException { | |
| MemoryUserRepository db = MemoryUserRepository.getInstance(); | |
| Map<String, String> userInfos = HttpRequestUtils | |
| .parseQueryParameter(request.getBody()); | |
| db.addUser(new User(userInfos.get("userId") | |
| , userInfos.get("password"), userInfos.get("name"), userInfos.get("email"))); | |
| response.redirectLogin("/index.html"); | |
| } | |
| public void execute(HttpRequest request, HttpResponse response) throws IOException { | |
| MemoryUserRepository db = MemoryUserRepository.getInstance(); | |
| Map<String, String> userInfos = HttpRequestUtils | |
| .parseQueryParameter(request.getBody()); | |
| // 필수 필드 검증 | |
| String userId = userInfos.get("userId"); | |
| String password = userInfos.get("password"); | |
| String name = userInfos.get("name"); | |
| String email = userInfos.get("email"); | |
| if (userId == null || userId.trim().isEmpty() || | |
| password == null || password.trim().isEmpty() || | |
| name == null || name.trim().isEmpty() || | |
| email == null || email.trim().isEmpty()) { | |
| response.forward("/user/form.html"); // 또는 에러 페이지 | |
| return; | |
| } | |
| // 중복 체크 | |
| if (db.findUserById(userId) != null) { | |
| response.forward("/user/form.html"); // 또는 에러 메시지 | |
| return; | |
| } | |
| db.addUser(new User(userId, password, name, email)); | |
| response.redirectLogin("/index.html"); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/http/controller/SignUpController.java around lines 14 to 21,
the controller calls userInfos.get(...) and directly adds a new User without any
validation which can cause NullPointerException or allow missing/invalid fields
and duplicate userIds; add input validation to (1) check required fields
(userId, password, name, email) are non-null/non-empty (trim strings) and return
a 400 Bad Request or render a signup error if validation fails, (2) validate
email format/length/basic rules if desired, (3) check MemoryUserRepository for
existing userId and return a 409 Conflict or appropriate error if duplicate, and
only call db.addUser(...) after all checks pass; ensure any early-return uses
response to set status/body or redirect to an error page and avoid calling
addUser with null values to prevent NPEs.
Comment on lines
+21
to
+28
| final String line = br.readLine(); | ||
| if (line.equals("")) { | ||
| break; | ||
| } | ||
| lines.add(line); | ||
| // header info | ||
| if (line.startsWith("Content-Length")) { | ||
| requestContentLength = Integer.parseInt(line.split(": ")[1]); |
There was a problem hiding this comment.
널 체크 없이 빈 줄을 판별하고 있습니다.
Line 22에서 BufferedReader.readLine()이 null을 반환할 수 있는데, 곧바로 line.equals("")를 호출하면 NPE가 발생합니다. 클라이언트가 연결을 끊거나 잘못된 요청을 보낼 때 서버가 바로 죽습니다.
다음과 같이 보정해 주세요:
- final String line = br.readLine();
- if (line.equals("")) {
+ final String line = br.readLine();
+ if (line == null || line.isEmpty()) {
break;
}📝 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
| final String line = br.readLine(); | |
| if (line.equals("")) { | |
| break; | |
| } | |
| lines.add(line); | |
| // header info | |
| if (line.startsWith("Content-Length")) { | |
| requestContentLength = Integer.parseInt(line.split(": ")[1]); | |
| final String line = br.readLine(); | |
| if (line == null || line.isEmpty()) { | |
| break; | |
| } | |
| lines.add(line); | |
| // header info | |
| if (line.startsWith("Content-Length")) { | |
| requestContentLength = Integer.parseInt(line.split(": ")[1]); |
🤖 Prompt for AI Agents
In src/main/java/http/HttpHeader.java around lines 21 to 28, the code calls
line.equals("") without checking for null which can cause an NPE when
BufferedReader.readLine() returns null; change the logic to first test for null
(e.g., if (line == null || line.isEmpty()) break) and only thereafter process
the line, and when parsing headers like Content-Length validate the presence of
":" and trim parts before Integer.parseInt to avoid exceptions on malformed
headers.
Comment on lines
+4
to
+5
| GET("get"), | ||
| POST("post"); |
There was a problem hiding this comment.
HTTP 메서드는 대문자여야 합니다.
HTTP 명세(RFC 7231)에 따르면 메서드 이름은 대소문자를 구분하며, 표준 메서드는 모두 대문자로 정의됩니다(GET, POST). 현재 소문자 "get", "post"를 사용하면 실제 HTTP 요청의 메서드와 매칭되지 않습니다.
다음과 같이 수정하세요:
public enum HttpMethod {
- GET("get"),
- POST("post");
+ GET("GET"),
+ POST("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.
Suggested change
| GET("get"), | |
| POST("post"); | |
| public enum HttpMethod { | |
| GET("GET"), | |
| POST("POST"); |
🤖 Prompt for AI Agents
In src/main/java/http/HttpMethod.java around lines 4 to 5, the enum uses
lowercase string values ("get", "post") which do not match HTTP spec; change
those to uppercase ("GET", "POST") so the enum value strings match actual HTTP
method names, and update any code that depends on the previous lowercase
representations (parsing, comparisons, tests) to use the new uppercase strings.
Comment on lines
+19
to
+21
| public static HttpRequest from(BufferedReader br) throws IOException { | ||
| String requestLine = br.readLine(); | ||
| if (requestLine == null || requestLine.isEmpty()) return null; |
There was a problem hiding this comment.
Null 반환 대신 예외 처리를 고려하세요.
요청 라인이 null이거나 비어있을 때 null을 반환하면 호출자에서 NPE가 발생할 수 있습니다. 빈 요청은 일반적으로 클라이언트가 연결을 조기에 닫은 경우이므로, 명시적인 예외를 던지거나 Optional을 반환하여 호출자가 이를 명확히 처리하도록 하는 것이 안전합니다.
다음과 같이 수정을 고려하세요:
- if (requestLine == null || requestLine.isEmpty()) return null;
+ if (requestLine == null || requestLine.isEmpty()) {
+ throw new IllegalArgumentException("요청 라인이 비어있습니다");
+ }또는 Optional 반환:
- public static HttpRequest from(BufferedReader br) throws IOException {
+ public static Optional<HttpRequest> from(BufferedReader br) throws IOException {
String requestLine = br.readLine();
- if (requestLine == null || requestLine.isEmpty()) return null;
+ if (requestLine == null || requestLine.isEmpty()) return Optional.empty();
HttpHeader httpHeader = HttpHeader.from(br);
String body = IOUtils.readData(br, httpHeader.getLengthOfContent());
- return new HttpRequest(HttpStartLine.from(requestLine), httpHeader, body);
+ return Optional.of(new HttpRequest(HttpStartLine.from(requestLine), httpHeader, body));
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/main/java/http/HttpRequest.java around lines 19-21, avoid returning null
when the request line is null/empty; instead throw a clear checked exception
(e.g., new IOException("Empty or closed request stream: request line is null or
empty")) so callers can't get an NPE and must handle the condition; replace the
null/empty early return with that exception and update any callers to
catch/propagate the IOException (alternatively, if you prefer Optional, change
the method signature to return Optional<HttpRequest> and return Optional.empty()
here and adjust callers accordingly).
| initializeMap(); | ||
| this.request = request; | ||
| this.response = response; | ||
| if(request.getMethod().equals("get") && request.getUrl().endsWith(".html")) { |
There was a problem hiding this comment.
HTTP 메서드 비교가 대소문자를 구분합니다.
request.getMethod().equals("get")은 대소문자를 구분하므로 "GET", "Get" 등은 매칭되지 않습니다. HTTP 메서드는 대소문자를 구분하지 않아야 하므로 equalsIgnoreCase를 사용하거나, HttpMethod enum을 직접 비교하는 것이 안전합니다.
- if(request.getMethod().equals("get") && request.getUrl().endsWith(".html")) {
+ if(request.getMethod().equalsIgnoreCase("GET") && request.getUrl().endsWith(".html")) {
this.controller = controllerMap.get("forward");
}📝 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
| if(request.getMethod().equals("get") && request.getUrl().endsWith(".html")) { | |
| if(request.getMethod().equalsIgnoreCase("GET") && request.getUrl().endsWith(".html")) { | |
| this.controller = controllerMap.get("forward"); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/http/RequestMapper.java around line 20, the HTTP method check
uses request.getMethod().equals("get") which is case-sensitive and will miss
"GET" or other casings; change it to a case-insensitive comparison (e.g.
equalsIgnoreCase("GET")) or, preferably, compare against a HttpMethod enum/value
from the request object; update the conditional accordingly so method matching
is case-insensitive and retains the existing URL check for ".html".
Comment on lines
+25
to
+32
| private void initializeMap(){ | ||
| controllerMap.put("forward",new ForwardController()); | ||
| controllerMap.put(URL.DEFAULT.getUrl(),new HomeController()); | ||
| controllerMap.put(URL.INDEX.getUrl(),new HomeController()); | ||
| controllerMap.put(URL.USER_SIGNUP.getUrl(),new SignUpController()); | ||
| controllerMap.put(URL.USER_LOGIN.getUrl(),new LoginController()); | ||
| controllerMap.put(URL.USER_USERLIST.getUrl(),new ListController()); | ||
| } |
There was a problem hiding this comment.
static map을 매 요청마다 초기화하는 것은 비효율적이고 위험합니다.
controllerMap은 static 필드이지만 initializeMap()이 생성자에서 호출되어 매 요청마다 맵을 다시 채웁니다. 이는:
- 불필요한 객체 생성으로 성능 낭비
- 멀티스레드 환경에서 동시성 문제 발생 가능 (race condition)
static 초기화 블록으로 한 번만 초기화하세요:
public class RequestMapper {
- private static final Map<String, Controller> controllerMap = new HashMap<>();
+ private static final Map<String, Controller> controllerMap;
+
+ static {
+ controllerMap = new HashMap<>();
+ controllerMap.put("forward", new ForwardController());
+ controllerMap.put(URL.DEFAULT.getUrl(), new HomeController());
+ controllerMap.put(URL.INDEX.getUrl(), new HomeController());
+ controllerMap.put(URL.USER_SIGNUP.getUrl(), new SignUpController());
+ controllerMap.put(URL.USER_LOGIN.getUrl(), new LoginController());
+ controllerMap.put(URL.USER_USERLIST.getUrl(), new ListController());
+ }
+
private final Controller controller;
private final HttpRequest request;
private final HttpResponse response;
public RequestMapper(HttpRequest request, HttpResponse response) {
- initializeMap();
this.request = request;
this.response = response;
if(request.getMethod().equals("get") && request.getUrl().endsWith(".html")) {
this.controller = controllerMap.get("forward");
}else this.controller = controllerMap.get(request.getUrl());
}
- private void initializeMap(){
- controllerMap.put("forward",new ForwardController());
- controllerMap.put(URL.DEFAULT.getUrl(),new HomeController());
- controllerMap.put(URL.INDEX.getUrl(),new HomeController());
- controllerMap.put(URL.USER_SIGNUP.getUrl(),new SignUpController());
- controllerMap.put(URL.USER_LOGIN.getUrl(),new LoginController());
- controllerMap.put(URL.USER_USERLIST.getUrl(),new ListController());
- }📝 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
| private void initializeMap(){ | |
| controllerMap.put("forward",new ForwardController()); | |
| controllerMap.put(URL.DEFAULT.getUrl(),new HomeController()); | |
| controllerMap.put(URL.INDEX.getUrl(),new HomeController()); | |
| controllerMap.put(URL.USER_SIGNUP.getUrl(),new SignUpController()); | |
| controllerMap.put(URL.USER_LOGIN.getUrl(),new LoginController()); | |
| controllerMap.put(URL.USER_USERLIST.getUrl(),new ListController()); | |
| } | |
| public class RequestMapper { | |
| private static final Map<String, Controller> controllerMap; | |
| static { | |
| controllerMap = new HashMap<>(); | |
| controllerMap.put("forward", new ForwardController()); | |
| controllerMap.put(URL.DEFAULT.getUrl(), new HomeController()); | |
| controllerMap.put(URL.INDEX.getUrl(), new HomeController()); | |
| controllerMap.put(URL.USER_SIGNUP.getUrl(), new SignUpController()); | |
| controllerMap.put(URL.USER_LOGIN.getUrl(), new LoginController()); | |
| controllerMap.put(URL.USER_USERLIST.getUrl(), new ListController()); | |
| } | |
| private final Controller controller; | |
| private final HttpRequest request; | |
| private final HttpResponse response; | |
| public RequestMapper(HttpRequest request, HttpResponse response) { | |
| this.request = request; | |
| this.response = response; | |
| if (request.getMethod().equals("get") && request.getUrl().endsWith(".html")) { | |
| this.controller = controllerMap.get("forward"); | |
| } else { | |
| this.controller = controllerMap.get(request.getUrl()); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/main/java/http/RequestMapper.java around lines 25 to 32, the static
controllerMap is being reinitialized per instance in initializeMap(), which
causes unnecessary object creation and potential race conditions; move the
population logic into a static initialization block (or initialize the map
inline) so the map is filled exactly once at class load time, change
initializeMap to a static/private helper or remove it, stop calling it from the
constructor, and optionally wrap the map with Collections.unmodifiableMap (or
use a thread-safe immutable map) to ensure safe read-only access by multiple
threads.
Comment on lines
+3
to
+21
| public enum Path { | ||
| INDEX_FILE("webapp/index.html"), | ||
| QNA_SHOW_FILE("webapp/qna/show.html"), | ||
| USER_FORM_FILE("webapp/user/form.html"), | ||
| USER_LOGIN_FILE("webapp/user/login.html"), | ||
| USER_LOGIN_FAIL_FILE("webapp/user/login_failed.html"), | ||
| USER_LIST_FILE("webapp/user/list.html"), | ||
| CSS_FILE("webapp/css/styles.css"); | ||
|
|
||
| private final String path; | ||
|
|
||
| public String getPath() { | ||
| return path; | ||
| } | ||
|
|
||
| Path(String path) { | ||
| this.path = path; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
❓ Verification inconclusive
Path enum이 실제로 사용되지 않고 있습니다.
경로를 중앙화하려는 의도는 좋으나, 이 enum이 실제 코드에서 사용되지 않고 있습니다. 예를 들어 HomeController는 여전히 "/index.html" 같은 하드코딩된 문자열을 사용합니다.
다음 스크립트로 Path enum의 사용 여부를 확인하세요:
만약 사용되지 않는다면:
- 컨트롤러들이 이 enum을 사용하도록 리팩토링하거나
- 당장 사용하지 않는다면 enum을 제거하는 것을 권장합니다.
🏁 Script executed:
#!/bin/bash
# Path enum이 실제로 참조되는지 확인
rg -n --type=java 'Path\.(INDEX_FILE|USER_FORM_FILE|USER_LOGIN_FILE)' -g '!Path.java'Length of output: 148
아래 스크립트를 실행해 Path enum이 실제로 참조되는지 확인하세요:
🏁 Script executed:
#!/bin/bash
# 모든 Java 파일에서 Path enum 사용 여부 확인
rg -n 'Path\.(INDEX_FILE|QNA_SHOW_FILE|USER_FORM_FILE|USER_LOGIN_FILE|USER_LOGIN_FAIL_FILE|USER_LIST_FILE|CSS_FILE)' -g '*.java' -g '!Path.java'Length of output: 0
아래 스크립트로 static import 및 일반 참조를 추가 확인하세요:
#!/bin/bash
# static import로 사용되는지 확인
rg -n 'import static .+Path' -g '*.java'
# 일반 참조 확인 (Path.java 제외)
rg -n '\bPath\b' -g '*.java' -g '!src/main/java/webserver/Path.java'아래 스크립트를 실행해 webserver.Path enum 사용 여부를 재확인하세요:
🏁 Script executed:
#!/bin/bash
# webserver.Path import 여부 확인
rg -n 'import\s+(static\s+)?webserver\.Path' -g '*.java'
# Path.<CONSTANT> 일반 참조 확인 (Path.java 제외)
rg -n '\bPath\.(INDEX_FILE|QNA_SHOW_FILE|USER_FORM_FILE|USER_LOGIN_FILE|USER_LOGIN_FAIL_FILE|USER_LIST_FILE|CSS_FILE)\b' -g '*.java' -g '!src/main/java/webserver/Path.java'Length of output: 0
Path enum이 사용되지 않음 – enum 제거 또는 사용 적용 필요
Path enum에 정의된 상수들이 코드에서 한 건도 참조되지 않습니다. 컨트롤러(예: HomeController)에서 하드코딩된 경로 문자열을 이 enum으로 교체하거나, 사용하지 않는다면 src/main/java/webserver/Path.java를 제거하세요.
🤖 Prompt for AI Agents
In src/main/java/webserver/Path.java around lines 3 to 21, the Path enum is
defined but not referenced anywhere; either replace hard-coded view/static path
strings with this enum across controllers (e.g., update HomeController and any
other controllers to return or reference Path.<CONSTANT>.getPath() instead of
string literals) or remove this file if you prefer literals; to fix: search the
codebase for hard-coded "webapp/..." strings, change each occurrence to the
corresponding Path enum constant (import webserver.Path where needed), recompile
and run tests to ensure no missing references, or if choosing removal, delete
src/main/java/webserver/Path.java and remove any related imports/usages and
verify build succeeds.
Comment on lines
+37
to
+41
| HttpRequest httpRequest = HttpRequest.from(br); | ||
| HttpResponse httpResponse = new HttpResponse(dos); | ||
|
|
||
| private void response200Header(DataOutputStream dos, int lengthOfBodyContent) { | ||
| try { | ||
| dos.writeBytes("HTTP/1.1 200 OK \r\n"); | ||
| dos.writeBytes("Content-Type: text/html;charset=utf-8\r\n"); | ||
| dos.writeBytes("Content-Length: " + lengthOfBodyContent + "\r\n"); | ||
| dos.writeBytes("\r\n"); | ||
| } catch (IOException e) { | ||
| log.log(Level.SEVERE, e.getMessage()); | ||
| } | ||
| } | ||
| RequestMapper requestMapper = new RequestMapper(httpRequest, httpResponse); | ||
| requestMapper.proceed(); |
There was a problem hiding this comment.
빈 요청 처리 없이 매퍼에 넘기고 있습니다.
Line 37에서 HttpRequest.from(br)가 null을 반환할 수 있는데, 바로 RequestMapper에 전달하면 내부에서 httpRequest.getMethod() 호출 시 NPE가 발생합니다. 연결 유지 중 빈 라인이 들어오는 순간 서버가 터집니다.
다음과 같이 널 요청을 걸러 주세요:
HttpRequest httpRequest = HttpRequest.from(br);
+ if (httpRequest == null) {
+ return;
+ }
HttpResponse httpResponse = new HttpResponse(dos);📝 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
| HttpRequest httpRequest = HttpRequest.from(br); | |
| HttpResponse httpResponse = new HttpResponse(dos); | |
| private void response200Header(DataOutputStream dos, int lengthOfBodyContent) { | |
| try { | |
| dos.writeBytes("HTTP/1.1 200 OK \r\n"); | |
| dos.writeBytes("Content-Type: text/html;charset=utf-8\r\n"); | |
| dos.writeBytes("Content-Length: " + lengthOfBodyContent + "\r\n"); | |
| dos.writeBytes("\r\n"); | |
| } catch (IOException e) { | |
| log.log(Level.SEVERE, e.getMessage()); | |
| } | |
| } | |
| RequestMapper requestMapper = new RequestMapper(httpRequest, httpResponse); | |
| requestMapper.proceed(); | |
| HttpRequest httpRequest = HttpRequest.from(br); | |
| if (httpRequest == null) { | |
| return; | |
| } | |
| HttpResponse httpResponse = new HttpResponse(dos); | |
| RequestMapper requestMapper = new RequestMapper(httpRequest, httpResponse); | |
| requestMapper.proceed(); |
| <div class="col-md-5"></div> | ||
| <div class="col-md-5"> | ||
| <ul class="pagination" style="display:align-items-center;"> | ||
| <ul class="pagination" class="pagination d-flex align-items-center"> |
There was a problem hiding this comment.
중복된 class 속성을 수정하세요.
<ul> 엘리먼트에 class 속성이 두 번 선언되어 있습니다. HTML에서는 하나의 속성을 한 번만 선언해야 합니다.
다음과 같이 수정하세요:
- <ul class="pagination" class="pagination d-flex align-items-center">
+ <ul class="pagination d-flex align-items-center">🤖 Prompt for AI Agents
In webapp/index.html around line 99, the <ul> element declares the class
attribute twice which is invalid HTML; remove the duplicate attribute by merging
both class value tokens into a single class attribute (e.g., combine
"pagination" and "d-flex align-items-center" into one class attribute) so the
element has one class attribute containing all space-separated class names.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
제가 수요일부터 본가였어서... 핑계지만 긴 시간을 투자 못 했습니다.
더러운 코드지만 최대한 요구사항을 구현하려고 노력했고,
일단 제출 안 하는 것보단 하는 게 낫다고 판단하여 제출합니다.
열심히 배우겠습니다...ㅜㅜ
Summary by CodeRabbit