RFR JDK-8153353: HPACK implementation

[Chris Hegarty]

Pavel,

Overall this is very good. I have mostly minor comments.

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

+1 package-info.java is almost useless in its current form. Please a consider adding
an example of encoding/decoding, as well as the quasi-public API ( the interface
HTTP2 is expected to use ).

A general comment; Quite a number of classes, especially Encoder and Decoder,
would benefit from a few relatively lightweight comments, e.g. IntegerReader

  /**
   * A reader that consumes bytes that conform to the HPACK, unsigned, variable-length,
   * Integer Representation, see section 5.1 of RFC 7541.
   */
  public final class IntegerReader {
      …
      /**
       * Attempts to read, or continues to read, the bytes from the given
       * byte buffer until the Integer is reconstructed.
       *
       * @return true if, and only if, the Integer (@link #get() value} has been fully
       * reconstructed.
       */
      public boolean read(ByteBuffer input) { … }

  }

IntegerWriter.java

 IntegerWriter.configure(int, int, int)  could use Object.checkFromToIndex ??

StringReader.java

  reset() can probably just reset its readers, regardless of huffman, since the 
  readers are always guaranteed to be present ( and reset is minimal ).

  General comment; in many cases it would be useful to capture, at least, the ‘state’
  when throwing InternalError.

IntegerReader.java

  reset() assumes that there is no bugs in the code. Should is assign 0 to r,
  value, and maxValue as well.  Q: is it necessary to call configure after reset?

ISO_8859_1.java

  Sorry, I don’ get 'outstanding'. Are you catching RuntimeException somewhere,
  or do you expect a user of the Decoder to do some special exception handling ?

HpackException.java

  Do you need this ??? ( FIXME comment )

-Chris.

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