Hi again Simone :-)
> On 6 Apr 2016, at 20:31, Simone Bordet <simone.bor...@gmail.com> wrote:
>
> 1) Add a test for the decoder where you have the incoming bytes be
> passed 1 at the time to the decoder.
> Something like:
>
> ByteBuffer data = ByteBuffer.wrap(new byte[]{
> 0b00001111, // literal, index=15
> 0b00000000,
> 0b00001000, // huffman=false, length=8
> 0b00000000, // \
> 0b00000000, // but only 3 octets available...
> 0b00000000 // /
> });
>
> for (int i = 0; i < data.remaining(); ++i) {
> decoder.decode(ByteBuffer.wrap(new byte[] {data.get(i)}), ...);
I'm all for better testing!
> 2) Verify what happens when a request comes with a huge cookie (where
> huge == 8 MiB or so).
> Perhaps a BufferOverflowException is thrown, but you want to report
> this in a way that the HTTP implementation can return 431.
> Same for the response side (and the encoder).
Speaking from HPACK perspective nothing bad should happen as the implementation
works incrementally.
Decoder is fed with buffers until a header block is fully processed. Decoder
does not make any assumption on how the header block is spread across
ByteBuffer(s). A complete header block may be contained in a single ByteBuffer
or in many. As a bottom line, there's no Buffer to throw a
BufferOverflowException from.
Pretty much the same story with the Encoder. It is configured with a header.
Then it's given buffers to write to. One by one. Until it fully encodes a
header. Encoder incrementally writes more data to the given buffer. It might
require a single buffer to encode the header or many.
> 4) Where possible, try to avoid the use of the modulo operator % - it
> is known to be really slow.
> From what I saw, you can probably get by using power of twos and bit
> shifting and masking.
> It does make the code a little more unreadable though. Your call.
To be honest I don't know how slow it is compared to the typical case of the
loop above it, I haven't measured. But I don't see any problem with
substituting
return len / 8 + (len % 8 != 0 ? 1 : 0);
with
return (len + 7) / 8;
Given the divisor is relatively small, it doesn't increase a real-world
possibility of overflow :-) Any chance it reads better?
Thanks.
