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

Volkan Yazıcı duke at openjdk.org
Wed Jan 15 08:49:22 UTC 2025


On Tue, 14 Jan 2025 20:45:13 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> 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
>
> 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 ?

Right. Fixed in c5186ba2c0ff11ac90ef00d90abe1d9a095f7185.

> 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

Fixed in 29174d163fbc85d8c308eb757edcfa4ba9ef3942.

> 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
> }

Fixed in bb577e9ba8a6ce8ee24e8371c4e34aeaceeb6808.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1916149415
PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1916149729
PR Review Comment: https://git.openjdk.org/jdk/pull/23096#discussion_r1916149952


More information about the net-dev mailing list