HTTP 2 client API

Sat Mar 21 10:44:46 UTC 2015

Hi Michael

Please find my feedback on the current API docs below. It's divided in 3 
parts: API (mostly questions), documentation (mostly suggestions for 
clarifications) and spelling.
Thanks in advance for your consideration.

Kind regards, Anthony

=== API ===
* will there be support for HTTP 2.0 stream prioritization? For example 
by providing a method requestHttp2(int priority) in 
HttpClient.Builder/HttpRequest.Builder? This way one could create 2 
clients with a different priority: a low-priority one for background 
downloads of documents/large files, and a high-priority one for status 
updates. Then the 2 clients would use the same TCP connection, but 
different streams with different priorities.

* which convenience methods will be added to existing classes, if any? 
I'm particularly thinking of URL, which already has 
openConnection/openStream convenience methods for the old API.

* how is compression (the 
Accept-Encoding/Content-Encoding/Transfer-Encoding headers) handled? Is 
this handled transparently by the HttpClient? For example, if I request 
a text document, will the client automatically send a header 
"Accept-Encoding: gzip, deflate" (this is the current default in 
Firefox)? And once I receive the response, transparently decompress it, 
so I can just do "response.body(asString())"?

* how is caching handled? Is there anything analog to "useCaches" in 
java.net.URLConnection?

* HttpClient.Builder: concerning "followRedirects", I feel there are 2 
more options that ought to be possible to be set: "same-protocol" (i.e. 
the current java.net.HttpURLConnection behavior) and "secure" (all 
redirects are allowed, except for redirects from https to http). A 
possible solution might be to introduce a "RedirectPolicy" enum with 4 
constants: NEVER, ALWAYS, SAME_PROTOCOL, SECURE. Then the method would 
become "followRedirects(RedirectPolicy policy)".

* HttpClient.Builder: the documentation for "requestHttp2" says: "If 
that fails, then following requests will not attempt to upgrade again." 
How is it determined that the upgrade to HTTP/2 fails? For example, is 
the client able to distinguish "the upgrade to HTTP/2 failed" from "some 
error occured which caused the request to fail" (e.g. the server is 
down)? And how can I force to attempt to upgrade again, for a specific 
origin and/or all origins? For example in the case of application 
servers, having to restart the JVM just to have the HTTP/2 upgrade to be 
attempted again, seems rather harsh.

* HttpHeaders: why is the method "firstValueAsInt" and not 
"firstValueAsLong"? If I want to download e.g. the .iso file of Oracle 
Linux, the value of Content-Length will be too big and a 
NumberFormatException will be thrown.

* HttpHeaders: why is there only a special case for "firstValueAsInt"? 
Personally, I would like some more methods to be added:
** java.time.OffsetDateTime firstValueAsDate(String name, 
Supplier<OffsetDateTime> defaultValue) which parses headers such as 
Expired & Last-Modified, using the datetime format recommended by RFC 
7231 (i.e. the format defined in RFC 5322)
** javax.activation.MimeType firstValueAsMimeType(String name, 
Supplier<MimeType> defaultValue) which parses headers such as 
Content-Type & Accept-Patch
** List<java.net.HttpCookie> allCookies() which returns a list of all 
cookies

* HttpHeaders: for the proposed first*() methods, please also consider 
the signature: Optional<T> firstValueAsT(String name). This way, the 
application developer can decide whether to use a default value or to 
throw an exception (or anything else) when the header is absent.

* HttpRequest/HttpResponse: why weren't the static methods added to the 
interfaces 
(HttpRequestBodyProcessor/HttpResponseBodyProcessor/MultiResponseProcessor) 
instead? In my opinion, this would be more logical, and would simplify 
the API of HttpRequest/HttpResponse.

* HttpRequest.Builder: add validation to header/method/setHeader 
(according to the rules in RFC 7230) & throw an IllegalArgumentException 
if validation fails

* HttpRequest.Builder: for "method", how is the parameter handled? Since 
method is case-sensitive ( cf. 
https://tools.ietf.org/html/rfc7230#section-3.1.1 ), creating a request 
with "delete" may fail, simply because it should be "DELETE". I think it 
would be good to at least add a Javadoc note about the case-sensitivity, 
and that the standardized methods ( 
http://tools.ietf.org/html/rfc7231#section-4.1 ) are defined as 
all-uppercase.

* HttpRequest: I'd propose to rename fromByteArray(Iterator<byte[]>) to 
fromByteArrays

* HttpRequest::fromString and HttpResponse::asString should take a 
java.nio.charset.Charset as parameter

* HttpResponse: in my opinion, "asFile" should take a Path as parameter 
& the parameter name should be "file", instead of "filename"

* HttpResponse: concerning the "asString" methods: they refer to "the 
character set specified in the Content-encoding response header". 
However, this should be "the character set specified in the charset 
parameter of the Content-type response header".

* WebSocket: the documentation says "Once the WebSocket is available, 
WebSocketMessages can be created and sent either blocking or 
asynchronously.". What does it mean to create a message asynchronously? 
And how can I send a message asynchronously (without manually creating a 
CompletableFuture), as there isn't a "sendAsync" method in WebSocket?

* WebSocket: in "Asynchronous example", I believe "sockets" should be 
defined as "new CopyOnWriteArrayList<>()", since LinkedList is not 
thread-safe? Also, I personally would declare "futures" as "new 
ArrayList<>()" (unless there's a compelling reason to use LinkedList of 
which I'm unaware?).

* please consider adding a HttpResponseBodyProcessor implementation 
"asDefined()", which uses the same mechanism as 
java.net.URLConnection::getContent (i.e. using the Content-Type header & 
ContentHandlers) to determine the object to return. This would allow for 
easy migration from the old API to the new API. (the "defined" in the 
method name refers to the fact that the value of the Content-Type header 
is used)

* please consider adding a HttpResponseBodyProcessor implementation 
"asFileDownload(Path downloadDirectory, OpenOption... openOptions)". 
This would determine the filename automatically, exactly as browsers do 
by inspecting e.g. the Content-Disposition header.

=== documentation ===
* as I'm sure you are aware, the package Javadoc should be updated to 
document the new API. I also think it would be good to clarify its 
relation to the old API, JAX-RS and Java API for WebSocket (the latter 2 
of which will, I assume, in future versions use this API as the basis 
for their Client API implementations).

* HttpClient: "Request builders are then created by calling request(URI) 
if associated with an application created client."
rephrase as "Request builders that are associated with an 
application-created client, are created by calling request(URI)."

* HttpClient.Builder: "If set, the first request to an origin server 
using "http" URLs will attempt to upgrade to HTTP version 2. If that 
fails, then following requests will not attempt to upgrade again."
** make "origin server" a hyperlink to 
https://tools.ietf.org/html/rfc6454#section-4
** explicitly state "following requests to the same origin server"

* HttpRequest:  "A request's uri, headers and body can be set."
change "uri" to "URI" & link it to java.net.URI

* HttpRequest: currently, all examples specify ".body(noBody())". This 
gives the impression it's actually required. I propose to remove this 
from all examples of GET requests (especially the one in the "Two 
simple, example HTTP interactions" at the top).

* HttpRequest::fromString and HttpResponse::asString: replace 
"ISO8859_1" with ISO-8859-1 and link to 
java.nio.charset.StandardCharsets.ISO_8859_1

* HttpResponse: "such as String, byte arrays, Files."
change "Files" to "files", since "Files" suggests that it returns 
java.io.File, when instead it's java.nio.file.Path

* HttpResponseBodyProcessor: "write responses to String, byte[], File, 
Consumer<byte[]>"
change "File" to "file", since "File" suggests that it returns 
java.io.File, when instead it's java.nio.file.Path

* HttpResponseBodyProcessor: "If an exact content length was provided in 
onRequestStart(), then that number of bytes must be provided."
explicitly add "before returning null" at the end, so: "If an exact 
content length was provided in onRequestStart(), then that number of 
bytes must be provided before returning null."

* WebSocket: add a note that using a WebSocket in a try-with-resources 
statement will cause the close to be done by closing the underlying TCP 
connection & what the possible implications are.

=== spelling ===
* HttpHeaders: "read only"
should be "read-only"

* HttpRequest: "any type as body,"
either has missing text after the comma, or the comma should be a point

* HttpRequest: "client initiated"
should be "client-initiated"

* HttpRequest: "The response body can then be received (either 
synchronous or asynchronously)"
should be "The response body can then be received (either synchronously 
or asynchronously)."

* HttpRequest: "Path body = r2.body(asFile("/tmp/response.txt));"
should be "Path body = r2.body(asFile("/tmp/response.txt"));"

* HttpRequest: "All of above examples"
should be "All of the above examples"

* HttpRequest: "CompletableFuture<String>; last ="
should be "CompletableFuture<String> last ="

* HttpRequest: "Returns the HttpClient.requestHttp2() setting for this 
request."
should note that this setting may have been overridden using 
HttpRequest.Builder.requestHttp2(), and therefore is not always equal to 
HttpClient.requestHttp2()

* HttpRequest.Builder: "A builder of HttpRequests"
should be "A builder of HttpRequests." (if you look at the package 
overview, all classes except this one have a point at the end)

* HttpResponseBodyProcessor: "return null and the body"
has missing text after body

* MultiResponseProcessor: "provides the"
should be "Provides the"

* WebSocketMessage: "A WebSocketMessages (Binary or Text)"
should be "A WebSocketMessage (Binary or Text)"

On 9/03/2015 17:27, Michael McMahon wrote:
> Hi,
>
> JEP 110 HTTP 2 client
>
> in JDK 9, is defining and implementing a new API for HTTP which also 
> supports
> the new HTTP version 2 that has recently been working its way through 
> the IETF.
> The work also includes support for websockets (RFC 6455).
>
> In fact, the majority of the API is agnostic about the HTTP protocol 
> version, with only minor
> configuration settings, and support for multiple responses (Http 
> server push) having any direct impact.
>
> The HTTP API is defined around three main types (HttpClient, which is 
> the central
> point for configuration of SSL, executor service cookie management 
> etc), HttpRequest
> and HttpResponse (which should be self explanatory).
>
> Requests are sent/received either synchronously (blocking) or in a 
> non-blocking (asynchronous)
> mode using java.util.future.CompletableFuture which is a powerful new 
> framework for
> asynchronous execution introduced in JDK 8.
>
> The API docs can be seen at the link below:
>
> http://cr.openjdk.java.net/~michaelm/httpclient/01/
>
> All new classes and interfaces belong to the java.net package.
>
> A prototype implementation of this API supporting HTTP/1.1 only, is 
> available and will
> be uploaded into the JDK 9 sandbox forest in the coming day or two.
>
> Comments welcome!
>
> Thanks,
> Michael.
>