fix: code review improvements

[dpsoft]

{
"analysis": [
{
"id": 1,
"category": "BUG",
"title": "Query parameter parsing crashes on parameters without a value",
"file": "src/main/java/io/github/dpsoft/ap/functions/Functions.java",
"lines": "22-24",
"issue": "In splitQueryParams, the code unconditionally calls pair.indexOf(\"=\") and then uses the result as an index for substring. If a query parameter has no = sign (e.g., ?flag or a malformed query string), indexOf returns -1, causing pair.substring(0, -1) to throw a StringIndexOutOfBoundsException. This crashes the HTTP handler for any request whose query string contains a bare key or a malformed pair, returning no error to the caller because the exception propagates unhandled out of handle().",
"fix": "Guard against idx == -1 and treat such pairs as boolean flags (value = "") or skip them:\n\njava\nfor (String pair : pairs) {\n final int idx = pair.indexOf(\"=\");\n if (idx < 0) continue; // skip malformed pair without '='\n final var key = URLDecoder.decode(pair.substring(0, idx), StandardCharsets.UTF_8);\n final var value = URLDecoder.decode(pair.substring(idx + 1), StandardCharsets.UTF_8);\n queryPairs.put(key, value);\n}\n"
},
{
"id": 2,
"category": "BUG",
"title": "HTTP 404 response body is never written, leaving the connection in an undefined state",
"file": "src/main/java/io/github/dpsoft/ap/handler/AsyncProfilerHandler.java",
"lines": "28-30",
"issue": "When the requested path is not in the configured context set, the handler calls sendResponseHeaders(HTTP_NOT_FOUND, 0L) and then exchange.close(). Passing 0L as the content-length to sendResponseHeaders signals a chunked / streaming response, not an empty body. The JDK HttpServer implementation requires that either a body is subsequently written to the response body stream or a negative content-length (-1) is passed to indicate zero bytes. Because the body stream is never opened or closed here, the client may never receive a complete HTTP response (the headers frame is sent but the body is never terminated), causing clients to hang. Additionally, any IOException from sendResponseHeaders is silently swallowed because exchange.close() is called without flushing.",
"fix": "Pass -1 as the response length (meaning no body) and ensure the exchange is always closed:\n\njava\nif (!configuration.handler.context().contains(path)) {\n exchange.sendResponseHeaders(HttpURLConnection.HTTP_NOT_FOUND, -1);\n exchange.close();\n return;\n}\n"
},
{
"id": 3,
"category": "MISSING_TEST",
"title": "No tests exist anywhere in the project — core parsing logic is completely untested",
"file": "src/main/java/io/github/dpsoft/ap/command/Command.java",
"lines": "46-72",
"issue": "The repository contains zero test sources (there is no src/test directory at all). The most critical pure-logic class, Command, has several non-trivial parsing methods — getOutput, getEventType, getDuration, getEventParams, and asFormatString — that are exercised only at runtime. Edge cases like an unrecognised output value falling back to JFR, Go-mode defaulting to PPROF, params splitting and --key=value → --key,value rewriting, and the seconds alias for duration are completely untested. The bug in Functions.splitQueryParams (issue #1) would have been caught immediately by a unit test.",
"fix": "Add a JUnit 5 test class covering the key branches of Command.from():\n\njava\n// src/test/java/io/github/dpsoft/ap/command/CommandTest.java\nclass CommandTest {\n\n private static AgentConfiguration config() {\n return AgentConfiguration.instance();\n }\n\n @Test\n void defaultOutputIsJfrWhenParamAbsentAndNotGoMode() {\n var cmd = Command.from(\"profile\", Map.of(), config());\n assertEquals(Command.Output.JFR, cmd.output);\n }\n\n @Test\n void defaultDurationIs30Seconds() {\n var cmd = Command.from(\"profile\", Map.of(), config());\n assertEquals(Duration.ofSeconds(30), cmd.getDuration());\n }\n\n @Test\n void secondsParamIsRespectedAsDurationAlias() {\n var cmd = Command.from(\"profile\", Map.of(\"seconds\", \"60\"), config());\n assertEquals(Duration.ofSeconds(60), cmd.getDuration());\n }\n\n @Test\n void unknownOutputFallsBackToJfr() {\n var cmd = Command.from(\"profile\", Map.of(\"output\", \"unknown\"), config());\n assertEquals(Command.Output.JFR, cmd.output);\n }\n\n @Test\n void eventParamsAreSplitAndPrefixed() {\n var cmd = Command.from(\"profile\", Map.of(\"params\", \"minwidth=1,reverse\"), config());\n assertTrue(cmd.eventParams.contains(\"--minwidth\"));\n assertTrue(cmd.eventParams.contains(\"1\"));\n }\n}\n"
}
]
}