On Thu, 20 Nov 2025 14:13:50 GMT, Volkan Yazici <[email protected]> wrote:
> Add guards against `HttpHeaders::firstValueAsLong` failures.
> `H3MalformedResponseTest` is overhauled – migrated to JUnit, reinforced with
> exception type tests, etc. – but not all `firstValueAsLong` changes are
> verified with new additional tests. `test/jdk/java/net/httpclient/` tests
> still do pass.
Changes requested by dfuchs (Reviewer).
src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java line
284:
> 282: long clen;
> 283: try {
> 284: clen = readContentLength(headers, "", 0);
This does not look right - we should keep -1 as meaning that content length was
not found.
src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java line
289:
> 287: }
> 288:
> 289: // Read if there are and less than `MAX_IGNORE` bytes
some missing words here...
src/java.net.http/share/classes/jdk/internal/net/http/Http1Response.java line
291:
> 289: // Read if there are and less than `MAX_IGNORE` bytes
> 290: if (clen > 0 && clen <= MAX_IGNORE) {
> 291: return readBody(discarding(), !request.isWebSocket(),
> executor);
This does not look right, we should not close the connection if we have
`Content-Length: 0`
src/java.net.http/share/classes/jdk/internal/net/http/common/Utils.java line
1349:
> 1347: // -- toAsciiString-like support to encode path and query URI
> segments
> 1348:
> 1349: public static Long readContentLength(HttpHeaders headers, String
> errorPrefix, long defaultIfMissing) throws ProtocolException {
I would prefer to use `OptionalLong` rather than nullable `Long`. Or maybe it's
an oversight and the method is intended to return `long`?
There seems to be quite some noise generated by trying to avoid using
OptionalLong and I'm not sure I understand the reason.
Using Optional/OptionalLong makes it clearer whether the header was found or
not.
-------------
PR Review: https://git.openjdk.org/jdk/pull/28431#pullrequestreview-3500010745
PR Review Comment: https://git.openjdk.org/jdk/pull/28431#discussion_r2556006255
PR Review Comment: https://git.openjdk.org/jdk/pull/28431#discussion_r2556003159
PR Review Comment: https://git.openjdk.org/jdk/pull/28431#discussion_r2556008868
PR Review Comment: https://git.openjdk.org/jdk/pull/28431#discussion_r2556028080
