On Fri, 18 Jul 2025 09:40:44 GMT, Volkan Yazici <vyaz...@openjdk.org> wrote:
>> src/java.net.http/share/classes/jdk/internal/net/http/RequestPublishers.java >> line 433: >> >>> 431: this.channel = Objects.requireNonNull(channel, "channel"); >>> 432: long fileSize = fileSize(channel); >>> 433: Objects.checkFromIndexSize(offset, length, fileSize); >> >> My preference would be to move these file size determination and the >> offset/length bounds check to the call site of this constructor. That way >> it's much more visible at the place where this behaviour is specified (by >> the public API). > > My motivation for keeping these checks at the `FileChannelPublisher::new` was > > 1. `FilePublisher` in the same file follows this convention too > 2. Even though `jdk.internal.net.http` is not exported, the class is > `public`. I wanted to ensure `FileChannelPublisher::new` doesn't accept > invalid input > > @jaikiran, given these, > > 1. Do you still prefer me moving the checks to the `ofFileChannel`? > 2. If yes, do you also want me to duplicate the checks in > `FileChannelPublisher::new` too? Hello Volkan, given what you note, what you currently have is fine. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26155#discussion_r2222207478
