On Wednesday, April 6, 2016, Pavel Rappo <pavel.ra...@oracle.com> wrote:
> Hi again Simone :-)
>
> > On 6 Apr 2016, at 20:31, Simone Bordet <simone.bor...@gmail.com
> <javascript:;>> 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?
Compilers (C2 included) have been strength reducing div and mod by
constants for a long time, so I wouldn't bother in this case if it makes
the code worse (pick your definition of worse).
>
> Thanks.
>
>
--
Sent from my phone
