RFR: 8328919: Add BodyHandlers / BodySubscribers methods to handle excessive server input [v3]

Daniel Fuchs dfuchs at openjdk.org
Tue Jan 14 20:48:47 UTC 2025


On Tue, 14 Jan 2025 20:04:40 GMT, Volkan Yazıcı <duke at openjdk.org> wrote:

>> Adds `limiting()` factory methods to `HttpResponse.Body{Handlers,Subscribers}` to handle excessive server input in `HttpClient`. I would appreciate your input whether `discardExcess` should be kept or dropped. I plan to file a CSR once there is an agreement on the PR.
>
> Volkan Yazıcı has updated the pull request incrementally with four additional commits since the last revision:
> 
>  - Make `LimitingSubscriber` stateful
>  - Improve Javadoc
>  - Replace `IllegalStateException` with an `IOException`
>  - Remove `discardExcess` argument

Thanks for taking into account the feedback! Some more comments.

src/java.net.http/share/classes/java/net/http/HttpResponse.java line 1400:

> 1398:          */
> 1399:         public static <T> BodySubscriber<T> limiting(BodySubscriber<T> downstreamSubscriber, long capacity) {
> 1400:             Objects.requireNonNull(downstreamSubscriber, "downstreamSubscriber");

Strange to check for null but not for capacity < 0 ?

src/java.net.http/share/classes/jdk/internal/net/http/LimitingSubscriber.java line 55:

> 53:     private final AtomicReference<State> stateRef = new AtomicReference<>();
> 54: 
> 55:     private long length;

should be volatile or AtomicLong

src/java.net.http/share/classes/jdk/internal/net/http/LimitingSubscriber.java line 107:

> 105: 
> 106:         // Otherwise, trigger failure
> 107:         else if (stateRef.compareAndSet(subscribed, State.Terminated.INSTANCE)) {

I would prefer to keep the regular style:


if (condition) {
    // do something
} else {
    // do something else
}

-------------

Changes requested by dfuchs (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/23096#pullrequestreview-2550965206
PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1915575983
PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1915568330
PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1915566451


More information about the net-dev mailing list