RFR JDK-8153353: HPACK implementation

Roger Riggs Roger.Riggs at Oracle.com
Mon Apr 11 15:18:11 UTC 2016


Hi Pavel,

Though this is  an implementation only package, it could use a few more 
comments
to support maintainers.  In the package.html, a couple of hints about 
what classes are
the external interface would be useful and an example of use in Encode 
and/or Decode
would help a lot.

The tests use randomness and should have @key randomness in the tests.

To allow reproducibility, the tests should print the seeds used and be 
able to be restarted
with a seed.  The testlibrary has a convenient factory for Random that 
logs the seed.

See jdk.testlibrary.RandomFactory

* @library /lib/testlibrary/
* @build jdk.testlibrary.RandomFactory

and use it as:
     private static final Random random = RandomFactory.getRandom();

IntegerReader:
   - It would be useful to have a reference to the spec for variable 
length integers: i.e.  rfc 7541 5.1 Integer Representation

  - Line 129, 53: inconsistent IAE's in some the valid condition is 
included in the exception message,
    but in others the *in*valid condition is included but without any 
indication of which is which.

- line 109/110:  style put 'while' on same line as '}' from do so it 
does not look like a separate statement.

IntegerWriter:
- line 40:  perhaps 'value' or 'v' would be more readable than 'i' which 
is often thought of as a local value.

Roger

p.s. RandomFactory needs to be moved/copied to the top level test 
library and the overlap
with test/lib/Utils.getRandomInstance resolved.



On 4/6/2016 2:00 PM, Pavel Rappo wrote:
> Hi,
>
> Could you please review my change for JDK-8153353?
>
> http://cr.openjdk.java.net/~prappo/8153353/webrev.00/
>
> This is an implementation of HPACK (Header Compression for HTTP/2) [1]
> Internal API classes are (package sun.net.httpclient.hpack):
>
>      Encoder, Decoder and DecodingCallback
>
> Example of their usage can be seen in the RFR for HTTP/2 implementation
> published previously today [2, 3].
>
> Thanks,
> -Pavel
>
> --------------------------------------------------------------------------------
> [1] https://tools.ietf.org/html/rfc7541
> [2] http://cr.openjdk.java.net/~michaelm/8087124/webrev.1/src/java.httpclient/share/classes/java/net/http/Http2Connection.java.html 304:306
> [3] http://cr.openjdk.java.net/~michaelm/8087124/webrev.1/src/java.httpclient/share/classes/java/net/http/Http2Connection.java.html 682:695



More information about the net-dev mailing list