kazu-yamamoto / http2

HTTP/2.0 library including HPACK
BSD 3-Clause "New" or "Revised" License
86 stars 22 forks source link

http2-1.5.1/test.out couple of failures #8

Closed juhp closed 8 years ago

juhp commented 8 years ago

With http2-1.5.1, seeing a few test failures in Stackage Nightly:

Failures:

  test/HPACK/EncodeSpec.hs:34: 
  1) HPACK.Encode, encodeHeader and decodeHeader, works for NaiveH
       expected: True
        but got: False

  test/HPACK/EncodeSpec.hs:34: 
  2) HPACK.Encode, encodeHeader and decodeHeader, works for StaticH
       expected: True
        but got: False

  test/HPACK/EncodeSpec.hs:26: 
  3) HPACK.Encode, encodeHeader and decodeHeader, works for LinearH
       uncaught exception: DecodeError (TooLongEos)

Randomized with seed 593477218

Finished in 5.6542 seconds
27 examples, 3 failures
juhp commented 8 years ago

Just for completeness here is the preceding spec test log:

> /tmp/stackage-build8/http2-1.5.1$ dist/build/spec/spec

HPACK.Decode
  fromHeaderBlock
    decodes HeaderList in request
    decodes HeaderList in response
    decodes HeaderList even if an entry is larger than DynamicTable
HPACK.Encode
  encodeHeader and decodeHeader
    works for Naive
    works for NaiveH FAILED [1]
    works for Static
    works for StaticH FAILED [2]
    works for Linear
[(":status","200"),("cache-control","no-cache"),("content-type","application/x-javascript"),("date","Sat, 03 Nov 2012 13:39:29 GMT"),("expires","Thu, 01 Jan 1970 00:00:01 GMT"),("pragma","no-cache"),("server","n
ginx/1.0.0"),("set-cookie","nyt-m=8F26281382200A491A2301632AB3A6B8&e=i.1354338000&t=i.10&v=i.1&l=l.25.2802408197.-1.-1.-1.-1.-1.-1.-1.-1.-1.-1.-1.-1.-1.-1.-1.-1.-1.-1.-1.-1.-1.-1.-1.-1&n=i.2&g=i.0&er=i.135194995
6&vr=l.4.1.0.0.0&pr=l.4.2.0.0.0&vp=i.0&gf=l.10.2802408197.-1.-1.-1.-1.-1.-1.-1.-1.-1; expires=Thu, 02-Nov-2017 13:39:29 GMT; path=/; domain=.nytimes.com"),("content-length","113"),("connection","keep-alive")]
    works for LinearH FAILED [3]
HPACK.Huffman
  encode and decode
    duality
  encode
    encodes
  decode
    decodes
HPACK.Integer
  encode and decode
    duality
    duality
    duality
    duality
    duality
    duality
    duality
    duality
HTTP2.Frame
  encodeFrameHeader & decodeFrameHeader
    encode/decodes frames properly
  encodeFrame & decodeFrame
    encode/decodes frames properly
    encode/decodes padded frames properly
HTTP2.Priority
  priority tree
    enqueue and dequeue frames from Firefox properly
  base priority queue
    queues entries based on weight
    deletes properly
    deletes entries properly
kazu-yamamoto commented 8 years ago

Strange. I cannot reproduce this on 64bit Linux and 64bit Mac. What kind of OS are you using? 32bit?

juhp commented 8 years ago

The Stackage builder is using 64bit Ubuntu 14.04 LTS with ghc-7.10.3 from ppa:hvr/ghc.

kazu-yamamoto commented 8 years ago

I have no idea on how to reproduce this.

juhp commented 8 years ago

Hmm, I can't reproduce locally with stack --resolver nightly-2016-02-28 test http2 either (nor with lts-5.5). Then maybe it relates to the Stackage docker image/environment somehow.

cc @snoyberg: any ideas? Is there any easy way to reproduce the Stackage build?

snoyberg commented 8 years ago

I haven't figured anything out yet, sorry.

clinty commented 8 years ago

We also experience this here: https://buildd.debian.org/status/package.php?p=haskell-http2

clinty commented 8 years ago

If we ignore armel (which is a different problem), all the failing architectures (amd64, arm64, x32) are little-endian and have 64-bit kernels. However, ppc64el is also little-endian with a 64-bit kernel and passes the Huffman encoding tests.

clinty commented 8 years ago

This issue is now https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=818143 in the Debian BTS.

kazu-yamamoto commented 8 years ago

Thank you for your report. Unfortunately, I have no idea on how to reproduce this in my environment. All tests are passed in my CentOS 7, my Mac and Travis CI.

kazu-yamamoto commented 8 years ago

Having

prop_dual cs = monadicIO $ do
    b <- run $ do
        let bs = BS.pack cs
        es <- encodeHuffman bs
        ds <- decodeHuffman es
        return $ ds == bs
    assert b

the following test succeeded.

> quickCheckWith stdArgs { maxSuccess = 100000 } prop_xxx
+++ OK, passed 100000 tests.

Uhhm.

kazu-yamamoto commented 8 years ago

I have released http2 1.5.3 which prints more debug information. Please tell me what is printed when the tests fail.

clinty commented 8 years ago

build log attached:

haskell-http2_1.5.3-1_amd64.build.txt

kazu-yamamoto commented 8 years ago

Thank you! Now I know what happened. HPACK encoder generates too long EOS.

However, I have not understood why this happened yet. It seems to me that my code does not generate such too long EOS.

kazu-yamamoto commented 8 years ago

OK. I understand. System specific part is memcpy only. http2 lib tries to copy an area to an overlapped place. The specification says that the behavior of memcpy is undefined in the case. Many implementations treats the case well but others does not. That's why.

clinty commented 8 years ago

Strange that Data.ByteString.Internal has memmove commented out.

kazu-yamamoto commented 8 years ago

Please try the current master branch which includes the patch above.

kazu-yamamoto commented 8 years ago

@clinty In ByteString implementation, memmove is not necessary. For instance, createAndTrim allocates ByteString twice. They are never overlapped.

clinty commented 8 years ago

With the patch, the tests pass on my machine.

kazu-yamamoto commented 8 years ago

Thank you, all guys, for this patient bug reports. I really appreciate.

I have released v1.5.4. Let's close.

juhp commented 8 years ago

Thank you! - not tested yet in Stackage Nightly