Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not allow update max dynamic table size to 0 #16

Closed
swamp-agr opened this issue Feb 13, 2019 · 5 comments
Closed

Do not allow update max dynamic table size to 0 #16

swamp-agr opened this issue Feb 13, 2019 · 5 comments

Comments

@swamp-agr
Copy link
Contributor

swamp-agr commented Feb 13, 2019

Hi @kazu-yamamoto,

Problem statement

During invocation of http2-client that uses http2 under the hood I encountered TooSmallTableSize exception. I create a new issue here to bring some light on this topic. Initial discussion with http2-client is there: haskell-grpc-native/http2-client#65

Details

  • server side: nginx 1.14.0 with http2 and https enabled by default.
  • client: http2-client with the latest version customized to use http2 from master branch of this repo.
  • :method: GET
  • Sequence of frames (trace from http2-client):
(2019-02-13 14:57:11.820714 UTC,[(SettingsMaxFrameSize,16384),(SettingsMaxConcurrentStreams,100),(SettingsMaxHeaderBlockSize,1048576),(SettingsInitialWindowSize,2147418112),(SettingsEnablePush,1),(SettingsHeaderTableSize,65536)])
(">>> ",0,[(0,SettingsFrame [(SettingsMaxFrameSize,16384),(SettingsMaxConcurrentStreams,100),(SettingsMaxHeaderBlockSize,1048576),(SettingsInitialWindowSize,2147418112),(SettingsEnablePush,1),(SettingsHeaderTableSize,65536)])])
      Table size: 0/655360
("<<< ",0,(FrameHeader {payloadLength = 18, flags = 0, streamId = 0},Right (SettingsFrame [(SettingsMaxConcurrentStreams,128),(SettingsInitialWindowSize,65536),(SettingsMaxFrameSize,16777215)])))
(">>> ",0,[(1,SettingsFrame [])])
      Table size: 0/655360
("<<< ",0,(FrameHeader {payloadLength = 4, flags = 0, streamId = 0},Right (WindowUpdateFrame 2147418112)))
      Table size: 0/655360
(">>> ",1,[(5,HeadersFrame Nothing "\130\EOT\255\r`r!\142\227BX\152\169c\252=\169\129\159\131\214H\ESCv1\150W\219\141\182H\212m\200\225\ESCvF\202\247\193\r$\SOH\NUL>\tR`\SI\196\150\217\NUL>\a!\164\131\224u\214i \248A\146X\141$\r6\239\199\179\212X\141$\SI8\NUL\252Vi q\231]o\226\146\200a\196i y\151\196\211\223\b4\144$\168O\194{\196I\199UE\162#I\STXJ\132\196I\199UE\163\226b\158\EOT\149\t\138b\159\226\146\200a\192j\211\223\135A\138\FS\136^\164\DC1j\a\SUB\230?S\131\249c\231\144@\133M\131\&5\ENQ\179\128")])
(2019-02-13 14:57:11.908085 UTC,"stream started (1,1)")
("<<< ",0,(FrameHeader {payloadLength = 0, flags = 1, streamId = 0},Right (SettingsFrame [])))
      Table size: 0/655360
("<<< ",1,(FrameHeader {payloadLength = 55, flags = 4, streamId = 1},Right (HeadersFrame Nothing " H\ETX403v\137\170cU\229\128\174\SYN\151\aa\150\228Y>\148\v*a,j\b\SOH}@\181q\183n\EOT*b\209\191_\135I|\165\137\211M\US\\\ETX169")))
http2-client-exe: ExceptionInLinkedThread ThreadId 13 TooSmallTableSize
  • 1st frame from server is a SettingsFrame.

  • 2nd frame from server is a HeadersFrame with 0x4 END_HEADERS flag (and the last one).

  • For this frame http2-client creates default newDynamicalTable.

  • After that decodeHeader invoked accordingly.

  • And its execution fails.

  • Here is minimal reproducible case that received in http2 master branch with cabal new-repl (GHC version 8.4.1):

*Network.HPACK> :m Network.HPACK Data.ByteString Data.Word
Prelude Network.HPACK Data.ByteString Data.Word> let bytelist = [32,72,3,52,48,51,118,137,170,99,85,229,128,174,22,151,7,97,150,228,89,62,148,11,42,97,44,106,8,1,125,65,10,227,45,220,19,202,98,209,191,95,135,73,124,165,137,211,77,31,92,3,49,54,57] :: [Word8]
Prelude Network.HPACK Data.ByteString Data.Word> dt <- newDynamicTableForDecoding 4096 4096
Prelude Network.HPACK Data.ByteString Data.Word> decodeHeader dt (pack bytelist)
*** Exception: TooSmallTableSize
  • The same test case without prompt for convenience (i.e. copy-paste as is):
:m Network.HPACK Data.ByteString Data.Word
let bytelist = [32,72,3,52,48,51,118,137,170,99,85,229,128,174,22,151,7,97,150,228,89,62,148,11,42,97,44,106,8,1,125,65,10,227,45,220,19,202,98,209,191,95,135,73,124,165,137,211,77,31,92,3,49,54,57] :: [Word8]
dt <- newDynamicTableForDecoding 4096 4096
decodeHeader dt (pack bytelist)
  • It was also determined that decodeHPACK function erases maxNumOfEntries for empty DynamicTable (initial value of buffer size was 4096, thus, maxNumOfEntries was equal to 128).
  • I also tried to invoke decodeSimple on ByteString from test case and it works just fine (There is only Entry 42 (Token {ix = 4, shouldBeIndexed = True, isPseudo = True, tokenKey = ":status"}) "403")!

Questions

  • Could you please suggest the next steps in order to identify whether it is a bug or an expected behaviour?
  • In case of expected behaviour could you please advise me what is a supposed behaviour of client http2-client (so I can follow your guidance and implement it) in order to mitigate the exception?

Thanks and Regards,
@swamp-agr

@swamp-agr
Copy link
Contributor Author

After some research it was determined that 1st bit of bytestring is a priority flag E (0x2) of PRIORITY Frame Payload that currently not supported yet by http2-client.

 +-+-------------------------------------------------------------+
 |E|                  Stream Dependency (31)                     |
 +-+-------------+-----------------------------------------------+
 |   Weight (8)  |
 +-+-------------+

Closing issue.

@kazu-yamamoto
Copy link
Owner

Currently support for the exclusive is weak in http2.
Please feel free to describe how we can improve the support.

@lucasdicioccio
Copy link

lucasdicioccio commented Feb 17, 2019

Hi @kazu-yamamoto , hope you're doing well.

I've helped @swamp-agr with the investigations. I think the exclusive has nothing to do with the bug we observed.

I suspect strongly an error due to sides-effect of receiving an Dynamic Size Update with mazSize = 0. I know about nothing on HPACK so I don't know what it means to have maxSize=0, but nginx and nghttp2 seem fine with it.

I put down a summary at
haskell-grpc-native/http2-client#65 (comment)

But the gist is:

bstr = " H\ETX403v\137\170cU\229\128\174\SYN\151\aa\150\228Y>\148\v_a,j\b\SOH}@\181q\183n\EOT_b\209\191_\135I|\165\137\211M\US\\ETX169"
withDynamicTableForDecoding 4096 4096 (\dtbl1 -> decodeHeader dtbl1 bstr)

throws TooSmallTableSize because it sets the maxSize to 0.

Should decode something along [(":status","403"),("server","nginx/1.14.0"),("date","Wed, 13 Feb 2019 14:57:11 GMT"),("content-type","text/html"),("content-length","169")]

Feel free to open a new issue.

@swamp-agr swamp-agr reopened this Feb 17, 2019
@swamp-agr swamp-agr changed the title Single HEADER Frame with 403 status failed with DecodeError Exception in decodeHeader Do not allow update max dynamic table size to 0 Feb 17, 2019
@kazu-yamamoto
Copy link
Owner

@lucasdicioccio Thank you for pointing out!
Unfortunately, I'm very busy now.
It would be appreciated if a PR to fix this was sent.

@swamp-agr
Copy link
Contributor Author

Hi @kazu-yamamoto,

Thank you for response! Let me prepare PR for the issue and move discussion to PR, because we are not pretty sure about whether it fit RFC 7541 or not.

Thanks,
@swamp-agr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants