Skip to content
Closed
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
56 changes: 56 additions & 0 deletions src/main/java/org/example/httpparser/HttpParser.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package org.example.httpparser;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.util.HashMap;
import java.util.Map;

public class HttpParser {
public HttpRequest parse(InputStream in) throws IOException {
BufferedReader reader = new BufferedReader(new InputStreamReader(in));
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

Mixing BufferedReader and raw InputStream reads corrupts the body.

BufferedReader reads ahead into an internal buffer (default 8 KiB). When you later call in.readNBytes(length) on the underlying InputStream (line 50), those bytes have already been consumed by the BufferedReader, so the body read will either block, return wrong data, or return fewer bytes than expected.

Read the body through the same BufferedReader, or avoid BufferedReader entirely and parse the request line / headers byte-by-byte from the InputStream.

Also applies to: 46-51

🤖 Prompt for AI Agents
In `@src/main/java/org/example/httpparser/HttpParser.java` at line 12, The code
currently creates a BufferedReader (BufferedReader reader = new
BufferedReader(new InputStreamReader(in))) but later reads the body from the
underlying InputStream via in.readNBytes(length), which leads to corruption
because BufferedReader has already read ahead; fix by using a single stream for
both headers and body: either (A) read the request line and headers with
BufferedReader (reader.readLine(...)) and then read the body from the same
reader (reader.read(char[] / readFully) and convert to bytes using the request
charset) or (B) stop using BufferedReader entirely and implement header parsing
byte-by-byte from the InputStream, then use in.readNBytes(length) for the body;
update the code paths that reference BufferedReader and the in.readNBytes(...)
call so they consistently use the same source.


// 1. Request Line
String requestLine = reader.readLine();
if (requestLine == null || requestLine.isEmpty()) {
throw new IOException("The request is empty");
}

String[] parts = requestLine.split(" ");
String method = parts[0];
String fullPath = parts[1];
String version = parts[2];
Comment on lines +20 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 | 🟠 Major

No validation on request-line parts — ArrayIndexOutOfBoundsException on malformed input.

split(" ") may yield fewer than 3 elements for a malformed request line (e.g. "GET /path"). Accessing parts[1] or parts[2] then throws.

🛡️ Proposed fix
         String[] parts = requestLine.split(" ");
+        if (parts.length < 3) {
+            throw new IOException("Malformed request line: " + requestLine);
+        }
         String method = parts[0];
         String fullPath = parts[1];
         String version = parts[2];
📝 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
String[] parts = requestLine.split(" ");
String method = parts[0];
String fullPath = parts[1];
String version = parts[2];
String[] parts = requestLine.split(" ");
if (parts.length < 3) {
throw new IOException("Malformed request line: " + requestLine);
}
String method = parts[0];
String fullPath = parts[1];
String version = parts[2];
🤖 Prompt for AI Agents
In `@src/main/java/org/example/httpparser/HttpParser.java` around lines 20 - 23,
Validate the parsed request-line before indexing: change the split to
requestLine.split(" ", 3) and check that the resulting parts array has length >=
3 (or otherwise treat as malformed), then handle the error path instead of
directly accessing parts[1]/parts[2]; update the code around HttpParser (the
requestLine, parts, method, fullPath, version variables) to throw or return a
parse error (e.g., an HttpParseException / specific error response) when the
request-line is malformed.


String path;
String query = null;

int qIndex = fullPath.indexOf('?');
if (qIndex >= 0) {
path = fullPath.substring(0, qIndex);
query = fullPath.substring(qIndex + 1);
} else {
path = fullPath;
}

// 2. Headers
Map<String, String> headers = new HashMap<>();
String line;
while (!(line = reader.readLine()).isEmpty()) {
int colon = line.indexOf(':');
String key = line.substring(0, colon).trim();
String value = line.substring(colon + 1).trim();
headers.put(key, value);
Comment on lines +40 to +43
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

Missing colon guard — malformed header line crashes the parser.

If a header line contains no :, indexOf returns -1 and substring(0, -1) throws StringIndexOutOfBoundsException.

🛡️ Proposed fix
             int colon = line.indexOf(':');
+            if (colon < 0) {
+                throw new IOException("Malformed header: " + line);
+            }
             String key = line.substring(0, colon).trim();
🤖 Prompt for AI Agents
In `@src/main/java/org/example/httpparser/HttpParser.java` around lines 40 - 43,
The parser currently assumes every header line contains ':' and will throw on
malformed lines; update the parsing in HttpParser (the block using 'int colon =
line.indexOf(':')', 'String key', 'String value', and 'headers.put') to guard
against colon == -1: if colon < 0 either skip the malformed line (or treat the
whole trimmed line as a header key with an empty value) and do not call
substring with -1, ensuring no StringIndexOutOfBoundsException is thrown.

}
Comment on lines +37 to +44
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

HTTP headers are case-insensitive — HashMap lookup will miss alternate casings.

Per RFC 7230, header field names are case-insensitive. A client sending content-length (lowercase) would bypass the Content-Length check on line 48. Consider using a TreeMap with case-insensitive ordering:

♻️ Suggested change
-        Map<String, String> headers = new HashMap<>();
+        Map<String, String> headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
📝 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
Map<String, String> headers = new HashMap<>();
String line;
while (!(line = reader.readLine()).isEmpty()) {
int colon = line.indexOf(':');
String key = line.substring(0, colon).trim();
String value = line.substring(colon + 1).trim();
headers.put(key, value);
}
Map<String, String> headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
String line;
while (!(line = reader.readLine()).isEmpty()) {
int colon = line.indexOf(':');
String key = line.substring(0, colon).trim();
String value = line.substring(colon + 1).trim();
headers.put(key, value);
}
🤖 Prompt for AI Agents
In `@src/main/java/org/example/httpparser/HttpParser.java` around lines 37 - 44,
The headers map in HttpParser is case-sensitive (headers variable) which breaks
RFC 7230; change headers to use a case-insensitive key lookup (e.g., replace new
HashMap<>() with a case-insensitive Map implementation such as new
TreeMap<>(String.CASE_INSENSITIVE_ORDER) or normalize keys to lower-case when
inserting) and ensure any subsequent lookups (e.g., the Content-Length check
that reads from headers) use that same case-insensitive map; update the
header-parsing loop to put keys into the chosen case-insensitive map (or
toLowerCase() keys) so alternate casings like "content-length" are recognized.

Comment on lines +39 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 | 🔴 Critical

readLine() can return nullNullPointerException when the connection closes early.

If the client disconnects before sending the blank line that terminates headers, readLine() returns null and .isEmpty() throws NPE.

🐛 Proposed fix
-        while (!(line = reader.readLine()).isEmpty()) {
+        while ((line = reader.readLine()) != null && !line.isEmpty()) {
🤖 Prompt for AI Agents
In `@src/main/java/org/example/httpparser/HttpParser.java` around lines 39 - 44,
In HttpParser where headers are read (the loop calling reader.readLine()), guard
against readLine() returning null before calling isEmpty() — capture the return
into a variable, check for null (treat as EOF/early close) and break or throw an
IOException as appropriate, then proceed to parse the line; also ensure you
handle a missing ':' by validating colon != -1 before using substring to avoid
further exceptions when building the headers map.


// 3. Body
byte[] body = new byte[0];
if (headers.containsKey("Content-Length")) {
int length = Integer.parseInt(headers.get("Content-Lenght"));
body = in.readNBytes(length);
Comment on lines +48 to +50
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

Typo: "Content-Lenght""Content-Length" — body is never read.

Line 48 correctly checks "Content-Length", but line 49 fetches "Content-Lenght" (misspelled). This always returns null, so Integer.parseInt(null) throws NumberFormatException for every request that has a body.

🐛 Fix the typo
         if (headers.containsKey("Content-Length")) {
-            int length = Integer.parseInt(headers.get("Content-Lenght"));
+            int length = Integer.parseInt(headers.get("Content-Length"));
             body = in.readNBytes(length);
         }
📝 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 (headers.containsKey("Content-Length")) {
int length = Integer.parseInt(headers.get("Content-Lenght"));
body = in.readNBytes(length);
if (headers.containsKey("Content-Length")) {
int length = Integer.parseInt(headers.get("Content-Length"));
body = in.readNBytes(length);
🤖 Prompt for AI Agents
In `@src/main/java/org/example/httpparser/HttpParser.java` around lines 48 - 50,
In HttpParser, the headers Map check uses "Content-Length" but the fetch uses
the misspelled key "Content-Lenght", causing Integer.parseInt(null) and no body
read; fix by retrieving the header with the correct key ("Content-Length")
before parsing and then call in.readNBytes(length) to populate body (refer to
headers, length, body, and in.readNBytes in HttpParser).

}

return new HttpRequest(method, path, query, version, headers, body);

}
}
12 changes: 12 additions & 0 deletions src/main/java/org/example/httpparser/HttpRequest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package org.example.httpparser;

import java.util.Map;

public record HttpRequest(
String method,
String path,
String queryString,
String httpVersion,
Map<String, String> headers,
byte[] body
) {}