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