On Wed, 15 Jan 2025 14:27:09 GMT, Jaikiran Pai <[email protected]> wrote:
>> Volkan Yazıcı has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove concurrency measures (methods are accessed serially due to the
>> Reactive Streams spec)
>
> src/java.net.http/share/classes/java/net/http/HttpResponse.java line 757:
>
>> 755: * and passed to the given downstream {@code BodyHandler}}
>> 756: * <p>
>> 757: * If the number of bytes received exceeds the maximum number
>> of bytes
>
> Although the `BodyHandlers` and `BodySubscribers` already note that they are
> applicable for reponse bodies, should we be adding any clarification in this
> new API, stating that the limit applies only to the body after the headers
> are read (however big those may be in bytes)?
Maybe we could talk of "body bytes".
> src/java.net.http/share/classes/java/net/http/HttpResponse.java line 758:
>
>> 756: * <p>
>> 757: * If the number of bytes received exceeds the maximum number
>> of bytes
>> 758: * desired as indicated by the given {@code capacity},
>
> Should we consider simplifying this sentence to "If the number of bytes
> received exceeds the given {@code capacity}, ..."?
Good suggestion.
> src/java.net.http/share/classes/jdk/internal/net/http/LimitingSubscriber.java
> line 118:
>
>> 116: private boolean allocateLength(List<ByteBuffer> buffers) {
>> 117: long bufferLength =
>> buffers.stream().mapToLong(Buffer::remaining).sum();
>> 118: long nextLength = Math.addExact(length, bufferLength);
>
> `Math.addExact` throws a `ArithmeticException` if there's an overflow during
> the addition. In its current form, this code can end up propagating the
> exception from here. Instead we should add a try/catch block to catch it and
> return false (implying the capacity has exceeded).
Good catch!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1916810288
PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1916804438
PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1916807262
