On Fri, 18 Apr 2025 18:49:19 GMT, Chen Liang <li...@openjdk.org> wrote:
>> Hi, >> >> Please find here a PR for the implementation of JEP [JDK-8291976: HTTP/3 for >> the HTTP Client API](https://bugs.openjdk.org/browse/JDK-8291976). >> >> The CSR can be viewed at [JDK-8350588: Implement HTTP/3 for the HTTP Client >> API](https://bugs.openjdk.org/browse/JDK-8350588) >> >> This JEP proposes to enhance the HttpClient implementation to support HTTP/3. >> It adds a non-exposed / non-exported internal implementation of the QUIC >> protocol based on DatagramChannel and the SunJSSE SSLContext provider. > > src/java.net.http/share/classes/java/net/http/HttpRequest.java line 357: > >> 355: * @since TBD >> 356: */ >> 357: public default <T> Builder setOption(HttpRequestOption<T> >> option, T value) { return this; } > > Bikeshed: we usually omit the set prefix for builders. I actually hesitated quite a bit on this one. We have a precedent with `Builder.setHeader(name, value)` - where the `setHeader` method replaces any value associated with the name with the given value, while `header(name, value)` would just add another value to the list of values associated with the name. I didn't want to give the impression that you could provide several values to the same option - and thus opted for naming that method `setOption`. I could let me be convinced to rename it if there is a strong concensus. > src/java.net.http/share/classes/java/net/http/HttpResponse.java line 937: > >> 935: HttpRequest initiatingRequest, >> 936: HttpRequest pushPromiseRequest, >> 937: PushId pushid, > > (Warning: I know little about HTTP, my assumptions and thus whole argument > may be invalid below!) > > This feels like users need to use push ids as units of synchronization on > this handler, such as tracking this with a ConcurrentHashMap. > > Is it possible for users to instead provide a `Function<PushId, > PushPromiseHandler>` in `HttpClient::sendAsync`? I think this might make it > easier for users as they no longer need to think about concurrency for > different ids. PushId is an HTTP/3 concept. It would be strange to have a generic method (sendAsync) that you could call with any requests, but with a parameter that could only be used for HTTP/3. We don't have pushIds for HTTP/2. I don't think we would want to take this route. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24751#discussion_r2054622121 PR Review Comment: https://git.openjdk.org/jdk/pull/24751#discussion_r2054623922
