Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .idea/gradle.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion .idea/misc.xml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 41 additions & 0 deletions src/main/java/http/HttpHeader.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package http;

import java.io.BufferedReader;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;

public class HttpHeader {
private final List<String> lines;
private final int lengthOfContent;

public HttpHeader(List<String> lines, int lengthOfContent) {
this.lines = lines;
this.lengthOfContent = lengthOfContent;
}

public static HttpHeader from(BufferedReader br) throws IOException {
int requestContentLength = 0;
List<String> lines = new ArrayList<>();
while (true) {
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]);
Comment on lines +21 to +28
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

널 체크 없이 빈 줄을 판별하고 있습니다.
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.

}
}
return new HttpHeader(lines, requestContentLength);
}

public int getLengthOfContent() {
return lengthOfContent;
}

public List<String> getLines() {
return lines;
}
}
16 changes: 16 additions & 0 deletions src/main/java/http/HttpMethod.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package http;

public enum HttpMethod {
GET("get"),
POST("post");
Comment on lines +4 to +5
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

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.


private final String method;

public String getMethod() {
return method;
}

HttpMethod(String method) {
this.method = method;
}
}
44 changes: 44 additions & 0 deletions src/main/java/http/HttpRequest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package http;

import http.util.IOUtils;

import java.io.BufferedReader;
import java.io.IOException;

public class HttpRequest {
private final HttpStartLine startLine;
private final HttpHeader header;
private final String body;

private HttpRequest(HttpStartLine startLine, HttpHeader header, String body) {
this.startLine = startLine;
this.header = header;
this.body = body;
}

public static HttpRequest from(BufferedReader br) throws IOException {
String requestLine = br.readLine();
if (requestLine == null || requestLine.isEmpty()) return null;
Comment on lines +19 to +21
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

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).


HttpHeader httpHeader = HttpHeader.from(br);
String body = IOUtils.readData(br, httpHeader.getLengthOfContent());
Comment on lines +23 to +24
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

Content-Length 값 검증이 필요합니다.

getLengthOfContent()가 음수이거나 비정상적으로 큰 값을 반환할 경우, IOUtils.readData에서 문제가 발생할 수 있습니다. 악의적인 요청이나 잘못된 헤더로부터 보호하기 위해 길이 값을 검증해야 합니다.

다음과 같이 검증 로직을 추가하세요:

         HttpHeader httpHeader = HttpHeader.from(br);
+        int contentLength = httpHeader.getLengthOfContent();
+        if (contentLength < 0) {
+            throw new IllegalArgumentException("Content-Length가 음수입니다: " + contentLength);
+        }
+        if (contentLength > 10_485_760) { // 10MB 제한 예시
+            throw new IllegalArgumentException("Content-Length가 너무 큽니다: " + contentLength);
+        }
-        String body = IOUtils.readData(br, httpHeader.getLengthOfContent());
+        String body = contentLength > 0 ? IOUtils.readData(br, contentLength) : "";
📝 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
HttpHeader httpHeader = HttpHeader.from(br);
String body = IOUtils.readData(br, httpHeader.getLengthOfContent());
HttpHeader httpHeader = HttpHeader.from(br);
int contentLength = httpHeader.getLengthOfContent();
if (contentLength < 0) {
throw new IllegalArgumentException("Content-Length가 음수입니다: " + contentLength);
}
if (contentLength > 10_485_760) { // 10MB 제한 예시
throw new IllegalArgumentException("Content-Length가 너무 큽니다: " + contentLength);
}
String body = contentLength > 0 ? IOUtils.readData(br, contentLength) : "";
🤖 Prompt for AI Agents
In src/main/java/http/HttpRequest.java around lines 23–24, validate the value
returned by httpHeader.getLengthOfContent() before passing it to
IOUtils.readData: ensure it is non-negative and does not exceed a reasonable
maximum (e.g., a MAX_CONTENT_LENGTH constant), and handle invalid values by
logging and returning a 400/closing the connection or treating length as 0 as
appropriate for the app; then call IOUtils.readData with the validated length
only.


return new HttpRequest(HttpStartLine.from(requestLine), httpHeader, 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

🧩 Analysis chain

HttpStartLine.from에서 발생할 수 있는 예외를 처리하세요.

HttpStartLine.from(requestLine)은 잘못된 형식의 요청 라인(예: 공백이 부족하거나 잘못된 HTTP 메서드)에서 ArrayIndexOutOfBoundsException 또는 IllegalArgumentException을 던질 수 있습니다. 이러한 예외를 처리하지 않으면 서버가 크래시할 수 있습니다.

다음 스크립트로 HttpStartLine.from의 예외 처리를 확인하세요:

예외 처리를 추가하는 것을 권장합니다:

+        try {
-        return new HttpRequest(HttpStartLine.from(requestLine), httpHeader, body);
+            return new HttpRequest(HttpStartLine.from(requestLine), httpHeader, body);
+        } catch (IllegalArgumentException | ArrayIndexOutOfBoundsException e) {
+            throw new IllegalArgumentException("잘못된 HTTP 요청 라인: " + requestLine, e);
+        }

🏁 Script executed:

#!/bin/bash
# HttpStartLine.from의 구현을 확인하여 예외 처리 확인
rg -A 10 'public static HttpStartLine from' --type java

Length of output: 766


HttpStartLine.from(requestLine) 호출부에 예외 처리 추가
잘못된 요청 라인에서 발생 가능한 ArrayIndexOutOfBoundsException과 IllegalArgumentException을 catch해 메시지와 함께 새로운 IllegalArgumentException으로 감싸서 던져야 합니다.

+        try {
             return new HttpRequest(HttpStartLine.from(requestLine), httpHeader, body);
+        } catch (ArrayIndexOutOfBoundsException | IllegalArgumentException e) {
+            throw new IllegalArgumentException("잘못된 HTTP 요청 라인: " + requestLine, e);
+        }
📝 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
return new HttpRequest(HttpStartLine.from(requestLine), httpHeader, body);
try {
return new HttpRequest(HttpStartLine.from(requestLine), httpHeader, body);
} catch (ArrayIndexOutOfBoundsException | IllegalArgumentException e) {
throw new IllegalArgumentException("잘못된 HTTP 요청 라인: " + requestLine, e);
}
🤖 Prompt for AI Agents
In src/main/java/http/HttpRequest.java around line 26, the call to
HttpStartLine.from(requestLine) can throw ArrayIndexOutOfBoundsException or
IllegalArgumentException for malformed request lines; wrap that call in a
try-catch that catches both exceptions and rethrows a new
IllegalArgumentException with a clear message (e.g., "Invalid HTTP request line:
" + requestLine) and pass the caught exception as the cause so the original
stack trace is preserved.

}

public String getMethod(){
return startLine.getMethod();
}

public String getUrl(){
return startLine.getTarget();
}

public String getBody() {
return body;
}

public HttpHeader getHeader(){
return header;
}
}
83 changes: 83 additions & 0 deletions src/main/java/http/HttpResponse.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package http;

import java.io.DataOutputStream;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;

public class HttpResponse {
private final DataOutputStream dos;

public HttpResponse(DataOutputStream dos) {
this.dos = dos;
}

public void forward(String path) throws IOException {
File file = new File("webapp" + path);
int fileLength = (int) file.length();
if(path.equals("/user/login_failed.html")){
responseLoginFailedHeader(dos, fileLength);
}
else response200Header(dos, fileLength);
responseBody(dos, Files.readAllBytes(Paths.get(file.getPath())));
}
Comment on lines +16 to +24
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

경로 검증이 없어 보안 취약점이 있습니다.

forward() 메서드가 경로 검증 없이 파일을 읽고 있어 경로 순회(Path Traversal) 공격에 취약합니다. 예를 들어 path"/../../../etc/passwd"와 같은 값이면 시스템 파일이 노출될 수 있습니다.

다음과 같이 경로를 정규화하고 검증하세요:

 public void forward(String path) throws IOException {
+    // 경로 정규화 및 검증
+    Path normalizedPath = Paths.get("webapp" + path).normalize();
+    if (!normalizedPath.startsWith(Paths.get("webapp").toAbsolutePath())) {
+        throw new SecurityException("Invalid path: " + path);
+    }
-    File file = new File("webapp" + path);
+    File file = normalizedPath.toFile();
+    if (!file.exists() || !file.isFile()) {
+        throw new IOException("File not found: " + path);
+    }
     int fileLength = (int) file.length();
🤖 Prompt for AI Agents
In src/main/java/http/HttpResponse.java around lines 16 to 24, the forward()
method reads files directly from "webapp" + path without validation, allowing
path traversal (e.g. "../../../etc/passwd"); normalize and validate the
requested path before reading: resolve the target File to its canonical path,
resolve the canonical path of the webapp root, ensure the target canonical path
startsWith the webapp root canonical path and that the file exists and is a
regular file; if validation fails, return an appropriate error response (e.g.,
404 or 400) instead of reading the file.


public void redirectLogin(String path) {
responseLoginHeader(dos, "/index.html");
}

public void redirectLoginFail(String path) {
response302Header(dos, "/user/login_failed.html"); }

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) {

}
}
Comment on lines +34 to +42
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

예외를 무시하지 마세요.

IOException을 빈 catch 블록으로 잡아서 무시하고 있습니다. 이는 응답 전송 실패를 감지할 수 없게 만들어 디버깅을 어렵게 하고, 클라이언트는 불완전한 응답을 받을 수 있습니다.

최소한 로깅을 추가하거나 예외를 다시 던지세요:

     } catch (IOException e) {
-
+        throw new RuntimeException("Failed to write response header", e);
     }

같은 문제가 lines 50-52, 60-61, 71-72, 79-81에도 있습니다.

📝 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
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) {
}
}
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) {
throw new RuntimeException("Failed to write response header", e);
}
}
🤖 Prompt for AI Agents
In src/main/java/http/HttpResponse.java around lines 34-42 (and also at lines
50-52, 60-61, 71-72, 79-81) the code swallows IOException with empty catch
blocks; update each catch to either log the exception (using the class logger or
System.err with a clear message including context) and then rethrow or wrap and
rethrow (e.g., throw new UncheckedIOException(e)) so callers can detect
failures; ensure the method signature is adjusted to declare throws IOException
if you choose to propagate the checked exception, or consistently wrap with a
runtime exception if you prefer unchecked propagation.


private void responseLoginHeader(DataOutputStream dos, String path) {
try {
dos.writeBytes("HTTP/1.1 302 Found \r\n");
dos.writeBytes("Location: " + path + "\r\n");
dos.writeBytes("Set-Cookie: logined=true; Path=/; HttpOnly; SameSite=Lax\r\n");
dos.writeBytes("\r\n");
} catch (IOException e) {

}
}

private void response302Header(DataOutputStream dos, String path) {
try {
dos.writeBytes("HTTP/1.1 302 Found \r\n");
dos.writeBytes("Location: " + path);
dos.writeBytes("\r\n");
} catch (IOException e) {
}
}
Comment on lines +55 to +62
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

Location 헤더 뒤에 \r\n이 누락되었습니다.

Line 58에서 Location 헤더 뒤에 \r\n을 추가하지 않고 바로 line 59에서 빈 줄을 보내고 있습니다. HTTP 명세에 따르면 각 헤더 라인은 \r\n으로 끝나야 합니다.

         dos.writeBytes("HTTP/1.1 302 Found \r\n");
-        dos.writeBytes("Location: " + path);
+        dos.writeBytes("Location: " + path + "\r\n");
         dos.writeBytes("\r\n");
📝 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 response302Header(DataOutputStream dos, String path) {
try {
dos.writeBytes("HTTP/1.1 302 Found \r\n");
dos.writeBytes("Location: " + path);
dos.writeBytes("\r\n");
} catch (IOException e) {
}
}
private void response302Header(DataOutputStream dos, String path) {
try {
dos.writeBytes("HTTP/1.1 302 Found \r\n");
dos.writeBytes("Location: " + path + "\r\n");
dos.writeBytes("\r\n");
} catch (IOException e) {
}
}
🤖 Prompt for AI Agents
In src/main/java/http/HttpResponse.java around lines 55 to 62, the Location
header is written without terminating CRLF so the header line is malformed;
change the write sequence to append "\r\n" immediately after the Location value
(i.e., write "Location: " + path + "\r\n") and keep the following blank line to
terminate headers, ensuring proper HTTP header formatting and still handle
IOExceptions appropriately.


private void responseLoginFailedHeader(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("Set-Cookie: logined=; Path=/; Max-Age=0; HttpOnly; SameSite=Lax\r\n");
dos.writeBytes("Content-Length: " + lengthOfBodyContent + "\r\n");
dos.writeBytes("\r\n");
} catch (IOException e) {
}
}

private void responseBody(DataOutputStream dos, byte[] body) {
try {
dos.write(body, 0, body.length);
dos.flush();
} catch (IOException e) {

}
}
}
30 changes: 30 additions & 0 deletions src/main/java/http/HttpStartLine.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package http;

import webserver.URL;

import java.util.Optional;

public class HttpStartLine {
private final HttpMethod method;
private final URL target;
private final String version;

public HttpStartLine(HttpMethod method, Optional<URL> target, String version) {
this.method = method;
this.target = target.orElse(null);
this.version = version;
}
Comment on lines +12 to +16
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

Optional 파라미터를 받고 null을 저장하는 것은 안티패턴입니다.

생성자가 Optional<URL>을 받지만 orElse(null)로 null을 저장하면 Optional의 목적이 무효화됩니다. 필드를 Optional로 유지하거나, 파라미터를 nullable로 받고 명시적으로 null을 허용하는 것이 더 명확합니다.

방안 1 (권장): 필드를 Optional로 유지

     private final HttpMethod method;
-    private final URL target;
+    private final Optional<URL> target;
     private final String version;
 
     public HttpStartLine(HttpMethod method, Optional<URL> target, String version) {
         this.method = method;
-        this.target = target.orElse(null);
+        this.target = target;
         this.version = version;
     }

그리고 getter도 수정:

     public String getTarget(){
-        return target.getUrl();
+        return target.map(URL::getUrl).orElse("");
     }

방안 2: 파라미터를 nullable로 변경

-    public HttpStartLine(HttpMethod method, Optional<URL> target, String version) {
+    public HttpStartLine(HttpMethod method, URL target, String version) {
         this.method = method;
-        this.target = target.orElse(null);
+        this.target = target;
         this.version = version;
     }
🤖 Prompt for AI Agents
In src/main/java/http/HttpStartLine.java around lines 12 to 16, the constructor
currently accepts Optional<URL> but stores target as null via orElse(null),
which breaks Optional semantics; change the code so the field stays
Optional<URL> and the constructor assigns this.target = target (i.e., preserve
the Optional), update the getter to return Optional<URL>, and then update any
call sites to handle Optional; alternatively (if you prefer nullable APIs)
change the constructor parameter type from Optional<URL> to URL (allowing null)
and keep the field as URL, update the getter to return @Nullable URL, and adjust
call sites accordingly—prefer the first option and refactor usages to handle
Optional.


public static HttpStartLine from(String startLine){
String[] tokens = startLine.split(" ");
return new HttpStartLine(HttpMethod.valueOf(tokens[0]), URL.fromPath(tokens[1]), tokens[2]);
}
Comment on lines +18 to +21
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

split 결과 배열의 길이를 검증하세요.

startLine.split(" ")의 결과가 3개 미만의 요소를 가질 경우 ArrayIndexOutOfBoundsException이 발생합니다. 또한 HttpMethod.valueOfURL.fromPath에서도 예외가 발생할 수 있습니다.

다음과 같이 검증 및 예외 처리를 추가하세요:

     public static HttpStartLine from(String startLine){
         String[] tokens = startLine.split(" ");
+        if (tokens.length != 3) {
+            throw new IllegalArgumentException(
+                "잘못된 HTTP 요청 라인 형식: " + startLine + " (예상: METHOD PATH VERSION)"
+            );
+        }
+        try {
-        return new HttpStartLine(HttpMethod.valueOf(tokens[0]), URL.fromPath(tokens[1]), tokens[2]);
+            return new HttpStartLine(
+                HttpMethod.valueOf(tokens[0]), 
+                URL.fromPath(tokens[1]), 
+                tokens[2]
+            );
+        } catch (IllegalArgumentException e) {
+            throw new IllegalArgumentException(
+                "HTTP 요청 라인 파싱 실패: " + startLine, e
+            );
+        }
     }
📝 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 static HttpStartLine from(String startLine){
String[] tokens = startLine.split(" ");
return new HttpStartLine(HttpMethod.valueOf(tokens[0]), URL.fromPath(tokens[1]), tokens[2]);
}
public static HttpStartLine from(String startLine){
String[] tokens = startLine.split(" ");
if (tokens.length != 3) {
throw new IllegalArgumentException(
"잘못된 HTTP 요청 라인 형식: " + startLine + " (예상: METHOD PATH VERSION)"
);
}
try {
return new HttpStartLine(
HttpMethod.valueOf(tokens[0]),
URL.fromPath(tokens[1]),
tokens[2]
);
} catch (IllegalArgumentException e) {
throw new IllegalArgumentException(
"HTTP 요청 라인 파싱 실패: " + startLine, e
);
}
}
🤖 Prompt for AI Agents
In src/main/java/http/HttpStartLine.java around lines 18 to 21, validate the
result of startLine.split(" ") to ensure it has exactly 3 tokens (or at least 3)
before indexing, and trim the input to avoid empty tokens; if validation fails
throw an IllegalArgumentException with a clear message. Also guard calls to
HttpMethod.valueOf and URL.fromPath by catching IllegalArgumentException (or
whatever those methods throw) and rethrowing a new IllegalArgumentException that
includes the original exception as the cause and a helpful message indicating
whether the problem was an invalid HTTP method or an invalid URL/path. Ensure
the method documents/propagates the IllegalArgumentException so callers receive
a clear, validated error instead of ArrayIndexOutOfBoundsException or raw
parsing exceptions.


public String getMethod() {
return method.getMethod();
}

public String getTarget(){
return target.getUrl();
}
Comment on lines +27 to +29
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

NPE 위험: target이 null일 수 있습니다.

Line 14에서 target이 null로 설정될 수 있으므로, target.getUrl()을 호출하면 NullPointerException이 발생합니다.

위의 Line 12-16에 대한 리뷰 코멘트에서 제안한 수정사항을 적용하면 이 문제도 함께 해결됩니다. Optional을 유지하는 방안을 권장합니다.

🤖 Prompt for AI Agents
In src/main/java/http/HttpStartLine.java around lines 27 to 29, getTarget()
calls target.getUrl() but target can be null, causing an NPE; change the method
to return Optional<String> instead of String, return
Optional.of(target.getUrl()) when target is non-null and Optional.empty() when
it is null, add the java.util.Optional import, and update any callers to handle
the Optional result.

}
37 changes: 37 additions & 0 deletions src/main/java/http/RequestMapper.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package http;

import http.controller.*;
import webserver.URL;

import java.io.IOException;
import java.util.HashMap;
import java.util.Map;

public class RequestMapper {
private static final Map<String, Controller> controllerMap = new HashMap<>();
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")) {
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

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".

this.controller = controllerMap.get("forward");
}else this.controller = controllerMap.get(request.getUrl());
}
Comment on lines +16 to +23
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

매핑되지 않은 경로에 대한 null 체크가 누락되었습니다.

Line 22에서 controllerMap.get(request.getUrl())이 null을 반환할 수 있지만, controller가 null인 경우에 대한 처리가 없습니다. Line 35의 controller.execute에서 NullPointerException이 발생합니다.

다음과 같이 null 체크를 추가하거나 기본 컨트롤러를 설정하세요:

     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());
+        } else {
+            this.controller = controllerMap.get(request.getUrl());
+            if (this.controller == null) {
+                throw new IllegalArgumentException("매핑되지 않은 경로: " + request.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
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());
}
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());
if (this.controller == null) {
throw new IllegalArgumentException("매핑되지 않은 경로: " + request.getUrl());
}
}
}
🤖 Prompt for AI Agents
In src/main/java/http/RequestMapper.java around lines 16 to 23, the constructor
assigns controller = controllerMap.get(request.getUrl()) without handling the
case where that call returns null; later controller.execute at line 35 will
throw a NullPointerException. Add a null check after retrieving from the map and
either assign a default/fallback controller (e.g., a 404/NotFound controller or
the existing "forward" controller) or throw a controlled exception so controller
is never null before execute; ensure the chosen fallback provides a clear
response and avoid changing other method contracts.


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());
}
Comment on lines +25 to +32
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

static map을 매 요청마다 초기화하는 것은 비효율적이고 위험합니다.

controllerMap은 static 필드이지만 initializeMap()이 생성자에서 호출되어 매 요청마다 맵을 다시 채웁니다. 이는:

  1. 불필요한 객체 생성으로 성능 낭비
  2. 멀티스레드 환경에서 동시성 문제 발생 가능 (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.


public void proceed() throws IOException {
controller.execute(request, response);
}
}
10 changes: 10 additions & 0 deletions src/main/java/http/controller/Controller.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package http.controller;

import http.HttpRequest;
import http.HttpResponse;

import java.io.IOException;

public interface Controller {
public void execute(HttpRequest request, HttpResponse response) throws IOException;
}
13 changes: 13 additions & 0 deletions src/main/java/http/controller/ForwardController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package http.controller;

import http.HttpRequest;
import http.HttpResponse;

import java.io.IOException;

public class ForwardController implements Controller{
@Override
public void execute(HttpRequest request, HttpResponse response) throws IOException {
response.forward(request.getUrl());
}
}
13 changes: 13 additions & 0 deletions src/main/java/http/controller/HomeController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package http.controller;

import http.HttpRequest;
import http.HttpResponse;

import java.io.IOException;

public class HomeController implements Controller{
@Override
public void execute(HttpRequest request, HttpResponse response) throws IOException {
response.forward("/index.html");
}
}
46 changes: 46 additions & 0 deletions src/main/java/http/controller/ListController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package http.controller;

import http.HttpRequest;
import http.HttpResponse;
import webserver.URL;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.List;

public class ListController implements Controller{
@Override
public void execute(HttpRequest request, HttpResponse response) throws IOException {
String logined = "";
int i = 0;
List<String> lines = request.getHeader().getLines();
while (true) {
if( i== lines.size()) {
break;
}
final String line = lines.get(i++);
if (line.isEmpty()) {
break;
}
if (line.startsWith("Cookie: ")) {
String cookieHeader = line.substring("Cookie:".length()).trim();
String[] pairs = cookieHeader.split(";");
for (String pair : pairs) {
String[] kv = pair.trim().split("=", 2); // 최대 2개만 split
if (kv.length == 2) {
String name = kv[0].trim();
String value = kv[1].trim();
if (name.equals("logined")) {
logined = value;
}
}
}
}
}
if(logined.equals("true")){
response.forward( "/user/list.html");
}
Comment on lines +42 to +44
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

로그인되지 않은 경우 응답이 비어 있습니다.
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).

}
}
29 changes: 29 additions & 0 deletions src/main/java/http/controller/LoginController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package http.controller;

import db.MemoryUserRepository;
import http.HttpRequest;
import http.HttpResponse;
import http.util.HttpRequestUtils;
import http.util.IOUtils;
import model.User;

import java.io.IOException;
import java.util.List;
import java.util.Map;

public class LoginController implements Controller {
@Override
public void execute(HttpRequest request, HttpResponse response) throws IOException {
MemoryUserRepository db = MemoryUserRepository.getInstance();
Map<String, String> userInfos = HttpRequestUtils
.parseQueryParameter(request.getBody());
User user = db.findUserById(userInfos.get("userId"));
if (user == null) {
response.redirectLoginFail("/user/login_failed.html");
return;
}
if (user.getPassword().equals(userInfos.get("password"))) {
response.redirectLogin("/index.html");
} else response.redirectLoginFail("/user/login_failed.html");
}
}
Loading