• Brad Fitzpatrick's avatar
    http2: move merging of HEADERS and CONTINUATION into Framer · 9e1fb3c1
    Brad Fitzpatrick authored
    HEADERS and CONTINUATION frames are special in that they must appear
    contiguous on the wire and there are lots of annoying details to
    verify while working through its state machine, including the handling
    of hpack header list size limits and DoS vectors.
    
    We now have three implementations of this merging (Server, Transport,
    and grpc), and grpc's is not complete. The Transport's was also
    partially incomplete.
    
    Move this up to the Framer (opt-in, for compatibility) and remove the
    support from the Server and Transport. I can fix grpc later to use
    this.
    
    Recommended reviewing order:
    
    * hpack.go exports the HeaderField.Size method and adds an IsPseudo
      method.
    
    * errors.go adds some new unexported error types, for testing.
    
    * frame.go adds the new type MetaHeadersFrame.
    
    * frame.go adds new fields on Framer for controlling how ReadFrame
      behaves
    
    * frame.go Framer.ReadFrame now calls the new Framer.readMetaFrame
      method
    
    * frame_test.go adds a bunch of tests. these are largely redundant
      with the existing tests which were in server and transport
      before. They really belong with frame_test.go, but I also don't want
      to delete tests in a CL like this. I probably won't remove them
      later either.
    
    * server.go and transport.go can be reviewed in either order at this
      point. Both are the fun part of this change: deleting lots of hairy
      state machine code (which was redundant in at least 6 ways: server
      headers, server trailers, client headers, client trailers, grpc
      headers, grpc trailers...). Both server and transport.go have the
      general following form:
    
      - set Framer.ReadMetaHeaders
      - stop handling *HeadersFrame and *ContinuationFrame; handle
        *MetaHeadersFrame instead.
      - delete all the state machine + hpack parsing callback hell
    
    The diffstat numbers look like a wash once you exclude the new tests,
    but this pays for itself by far when you consider the grpc savings as
    well, and the increased simplicity.
    
    Change-Id: If348cf585165b528b7d3ab2e5f86b49a03fbb0d2
    Reviewed-on: https://go-review.googlesource.com/19726Reviewed-by: 's avatarBlake Mizerany <blake.mizerany@gmail.com>
    Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
    9e1fb3c1
errors.go 3.7 KB