Query params: parse and store separately in HttpRequest

[Ericthilen]

Fixes #102

• Extracts path separately from query string
• Parses query params after '?', separated by '&'
• Supports multiple values per key
• URL-decodes keys and values using UTF-8
• Stores query params separately in HttpRequest

Summary by CodeRabbit

• New Features
  • Query parameters are now parsed and accessible on incoming HTTP requests.
  • Request routing uses the extracted path only for more accurate file/target resolution.
  • Client IP addresses are automatically captured and attached to request context for downstream use.

[coderabbitai]

📝 Walkthrough

Walkthrough

ConnectionHandler now extracts the request path and parses URL-decoded query parameters, passes them into HttpRequest, and captures client IP as a request attribute. HttpRequest gains a new queryParams field, constructors that accept query parameters, and getters to access them.

Changes

Cohort / File(s)

Summary

Request parsing & model src/main/java/org/example/ConnectionHandler.java, src/main/java/org/example/httpparser/HttpRequest.java

ConnectionHandler: extracts pathOnly, parses URL-decoded query params without split(), uses request.setAttribute("clientIp", ...), adjusts IpFilter init/use, and passes parsed params into HttpRequest. HttpRequest: adds private final Map<String,List<String>> queryParams, new constructors accepting queryParams, and getQueryParams() / getQueryParam(String) accessors. Note: public HttpRequest constructor signature expanded.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• * Move HTTP handling to a dedicated ConnectionHandler #50: Directly related — previously introduced ConnectionHandler behavior that this PR extends with path extraction and query-param parsing.

Suggested reviewers

• eeebbaandersson
• eraiicphu

Poem

🐰 I hopped along the URI trail,
Decoded crumbs in every tail,
Keys and values tucked in rows,
Path alone now proudly shows,
HttpRequest holds what the rabbit knows.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 5.26% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main changes: parsing and storing query parameters separately in HttpRequest.
Linked Issues check	✅ Passed	The implementation fully addresses issue #102: parses query parameters after '?', handles multiple values per key, and URL-decodes keys/values using UTF-8.
Out of Scope Changes check	✅ Passed	All changes are within scope: query parameter parsing/storage in HttpRequest and related updates to ConnectionHandler to extract and pass parsed parameters.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/Query-Parameters

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/org/example/ConnectionHandler.java`:
- Around line 117-121: The extractPath method returns an empty string for URIs
like "?a=1" — update extractPath to normalize an empty path to "/" by returning
"/" when uri has no path before the '?' (i.e., if substring(0, q) yields empty).
Also update the query-parsing logic referenced around lines 135-143 (the loop
that splits on '&' into segments) to skip empty segments and ignore empty keys
(continue when segment is empty or when key length==0 after splitting on '='),
so consecutive separators like "&&" or leading '&' don't produce empty-key
params; apply these checks in the query parsing routine that iterates over
segments.
- Around line 123-160: parseQueryParams currently calls
URLDecoder.decode(key/value) which can throw IllegalArgumentException on
malformed percent-encoding; wrap the call to parseQueryParams(...) inside the
request handling path (the method that calls parseQueryParams in
ConnectionHandler) with a try-catch that catches IllegalArgumentException and
returns a controlled 400 Bad Request response. Alternatively, modify
parseQueryParams itself to catch IllegalArgumentException around
URLDecoder.decode and rethrow a specific checked exception (or return an error
indicator) so the caller can map it to a 400; reference URLDecoder.decode and
parseQueryParams when making the change.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa1599a and e4a4f90.

📒 Files selected for processing (2)

• src/main/java/org/example/ConnectionHandler.java
• src/main/java/org/example/httpparser/HttpRequest.java

[coderabbitai]

🧹 Nitpick comments (1)

src/main/java/org/example/ConnectionHandler.java (1)

147-161: Consider skipping empty keys after decoding.

Empty pairs are correctly skipped, but a query like ?=value will produce an entry with an empty-string key. Consider also skipping entries where the key is empty after URL decoding.

Suggested enhancement

             key = URLDecoder.decode(key, StandardCharsets.UTF_8);
             value = URLDecoder.decode(value, StandardCharsets.UTF_8);
 
+            if (key.isEmpty()) {
+                start = ampIndex + 1;
+                continue;
+            }
+
             params.computeIfAbsent(key, k -> new ArrayList<>()).add(value);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/main/java/org/example/ConnectionHandler.java` around lines 147 - 161, The
parsing currently accepts a pair like "?=value" because it checks pair.isEmpty()
but not whether the decoded key is empty; in ConnectionHandler (the query-pair
loop using variables pair, equalsIndex, key, value, ampIndex, start) extract the
raw key and raw value as you already do, then URL-decode the key (and decode the
value) using the same decoder used elsewhere (e.g., URLDecoder with UTF-8), and
if the decoded key is empty do a continue to skip the entry; replace any
subsequent use of key/value with the decoded versions.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/main/java/org/example/ConnectionHandler.java`:
- Around line 147-161: The parsing currently accepts a pair like "?=value"
because it checks pair.isEmpty() but not whether the decoded key is empty; in
ConnectionHandler (the query-pair loop using variables pair, equalsIndex, key,
value, ampIndex, start) extract the raw key and raw value as you already do,
then URL-decode the key (and decode the value) using the same decoder used
elsewhere (e.g., URLDecoder with UTF-8), and if the decoded key is empty do a
continue to skip the entry; replace any subsequent use of key/value with the
decoded versions.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e4a4f90 and c0c9ca6.

📒 Files selected for processing (1)

• src/main/java/org/example/ConnectionHandler.java

[alicewersen-rgb]

Looking good! The code is clean, simple and the comments explain the logic well.

[EdvinSandgren]

I agree with the previous review, looks good.