Re: END_STREAM flag and trailing headers

On 9 May 2014 00:29, Martin Thomson <martin.thomson@gmail.com> wrote:

> On 8 May 2014 15:14, Greg Wilkins <gregw@intalio.com> wrote:
> > However, if I have this right, this now means that a PUSH_PROMISE, must
> have
> > data?
>
> PUSH_PROMISE has no data associated with it directly.
>

Sorry my bad interpretation of the spec.   It could definitely use an
example like yours.

> Actually I think that it is poor protocol layering to have to interpret
the type of a
> frame in order to workout stream boundaries.

I used to think the same, and if you care to do some git spelunking,
> there's good evidence of that.  I think that it was at the point that
> I went through the process of working through the state machine that I
> changed my mind.  What we've actually done here is conflate some of
> the stream control functions with the application semantics functions
> in the interests of efficiency.
>

I find that scary!  in the battle of compromises of efficiency vs
complexity, I have rarely seen efficiency achieved though complexity.  In
this case I see little efficiency to be gained by have a few frame types
not support END_STREAM when all frame types so far defined have many spare
flag bits anyway.  But the cost in complexity is significant, not least in
poor CPU branch prediction, but also in likely bugs and future constraints
on protocol development.

More over, the spec does not contain a formalisation of what is a legal
stream. Section 5.1 looks to be a formalisation of stream states, but it is
not because you are still required to read the detailed text to interpret
the state machine and i do not think the text descriptions of the states
adequately describe what can/can't be done in each state.

Problems with 5.1 include:

   - The text description of PUSH_PROMISE does not adequately describe the
   fact that two streams are involved and that it is the transmission of a
   PUSH_PROMISE on one stream that puts a different stream into the reserved
   states.
   - The half closed states are insufficient as they do not capture the
   sending of CONTINUATION frames after a frame with the ES bit.  The text
   "cannot be used for sending frames" is wrong as CONTINUATION, RST_STREAM
   and WINDOW_UPDATE frames at least can be sent from this state.   A proper
   formalisation probably needs to split this state into trailer and half
   closed states to indicate if the close was invoked via a DATA or HEADER
   frame.
   - The closed state suffers similar problems, as it currently can be
   entered by a half closed stream sending or receiving a frame with ES set,
   even though it has not yet received all the CONTINUATION frames that it
   might receive, or sent all the CONTINUATION frames that it may send.
   - A stream in closed state may receive frames (eg PUSH_PROMISE) that it
   must act on and reserve the stream.  There appears no end to this state as
   RST_STREAM has no response, so an implementation that sends a RST_STREAM
   will have a closed stream that it will have to monitor forever for
   potential PUSH_PROMISES!  (why doesn't RST_STREAM just push the stream into
   half closed, so the other end can acknowledge it?)

Some of these issues might be able to be solved with a better formalisation
of the stream state,  I started to give that a go as follows:

                                         +--------+
                                   PP    |        |    PP
                                ,--------|  idle  |--------.
                               /         |        |         \
                              v          +--------+          v
                       +----------+          |           +----------+
                       |          |          | H         |          |
                   ,---| reserved |          |           | reserved |---.
                   |   | (local)  |          |           | (remote) |   |
                   |   +----------+          |           +----------+   |
                   |      |                  v                   |      |
  +---------+      |      |              +--------+              |
 |    +---------+   |         |      |      |        H(ES) |        |
H(ES)       |      |    |         |  | trailer |<---- | ---- |
-------------|        |------------- | ---- | -->| trailer |
  | (remote)|      |      |              |        |              |
 |    | (local) |
  |         |      |      |     H(ES,EH) |  open  |  H(ES,EH)    |
 |    |         |
  +---------+      |      |       D(ES)  |        |  D(ES)       |
 |    +---------+
          \        |      | H    ,-------|        |-------.      | H    |      /
     C(EH) \       |      |     /        |        |        \     |
 |     /C(EH)
            \      |      v    v         +--------+         v    v      |    /
             \     |   +----------+          |           +----------+   |   /
              `--- | ->|   half   |          |           |   half   |<- | -'
                   |   |  closed  |          | R         |  closed  |   |
                   |   | (remote) |          |           | (local)  |   |
                   |   +----------+          |           +----------+   |
                   |        |                v                 |        |
                   |        |  ES / R    +--------+  ES / R    |        |
                   |        `----------->|        |<-----------'        |
                   |  R                  | closed |                  R  |
                   `-------------------->|        |<--------------------'
                                         +--------+

      H:  HEADERS frame (with implied CONTINUATIONs)
      PP: PUSH_PROMISE frame (with implied CONTINUATIONs)
      D(ES): DATA frame with END_STREAM flag
      H(ES,EH): HEADER frame with END_STREAM and END_HEADER flags
      H(ES) : HEADER frame with END_STREAM flag
      C(EH) : CONTINUATION frame with END_HEADER flag
      R:  RST_STREAM frame

But that is still insufficient as it needs to handle the case of H(ES) from
the other end while you are in one of the trailer states.  This get's way
too complex for one state machine.   So perhaps we should formalise the
half state of a stream in isolation from the other half (as this is probaly
how it will be implemented), but the two halves are not truly independent.

I think the fundamental problem is simply that the stream state is too
complex.  If it cannot be formalised without resorting to complex textual
descriptions, then that is a big worry for the ability of the protocol to
understood, implemented, tested, debug, hacked, exploited, etc!

I think that we should really reconsider reinstating the ES bit on all
frames and then we can have a much simpler formalisation of the stream
state and we can have some definitive statements like:

   - After sending an ES flag, the stream is half closed local, which means:
      - it shall not send any DATA, HEADER or CONTINUATION frames
      - it shall ignore any WINDOW_UPDATE or RST_STREAM frames sent
   - After receiving an ES flag, the stream is half closed remote, which
   means:
      - any frames other than WINDOW_UPDATE or RST_STREAM is a stream error
   - After sending and receiving an ES flag, the stream is closed, which
   means:
      - no frames should be sent or received on the stream.

Further:

   - The RST_FRAME must have the ES flag and on receipt of a RST_FRAME in
   any state other that closed or half closed, then a REST_FRAME frame should
   be immediately sent in reply.









-- 
Greg Wilkins <gregw@intalio.com>
http://eclipse.org/jetty HTTP, SPDY, Websocket server and client that scales
http://www.webtide.com  advice and support for jetty and cometd.

Received on Monday, 19 May 2014 13:59:09 UTC