Re: [hybi] I-D Action: draft-ietf-hybi-permessage-compression-13.txt

[Takeshi Yoshino <[e-mail]>]

Some addressed in -14. The rest will be addressed in -15.

On Wed, Oct 9, 2013 at 12:14 PM, Joakim Erdfelt <[e-mail]> wrote:

>    1.  Introduction . . . . . . . . . . . . . . . . . . . . . . . . .  3
>
>  > good
>  > +1 for the removal of the PPP text
>

Yes. It was broken after rendering HTML.


>    3.  Complementary Terminology  . . . . . . . . . . . . . . . . . .  5
>
>  > sufficient, would providing links to RFC6455 sections be worthwhile?
>  > example:
>
>  - Non-control message means a message consists of non-control frames.
>  + Non-control message means a message that consists of non-control
>    frames (e.g., data frames) as defined in [Section 5.6] of [RFC6455]
>
done


>
>  - Message payload (or payload of a message) means concatenation of the
>  - payload data portion of all frames consisting the message.
>  + Message payload (or payload of a message) means concatenation of the
>  + payload data portion of all data frames consisting of a single
>  + message, as defined in [Section 6] of [RFC6455]
>

s/consisting of/representing/ ?

done


>
>  > no commentary on section for the paragraphs about Extension behavior
>  > as the reference to [Section 9] of [RFC6455] is handled in the next
>  > section of this doc
>
>
>    5.  Extension Negotiation  . . . . . . . . . . . . . . . . . . . .  7
>

Used texts I showed in my last reply.


>
>    If a server gives an invalid response, such as accepting a PMCE that
>    the client did not offer, the client MUST _Fail the WebSocket
>    Connection_.
>
>  > should we mention close code 1010 here?
>

I'm not sure if 1010 is for this kind of failure. The text in RFC 6455
sounds like it's used to reject an opening handshake response that doesn't
include extension(s) the client considered to be mandatory.

There isn't any extension with concept of mandatory introduced yet.
permessage-deflate is not mandatory extension. We just want to tell the
server that it sent back an invalid extension response.


>
>
>      6.1.  Compression  . . . . . . . . . . . . . . . . . . . . . . . 10
>
>    2.  If this PMCE is the last extension to process outgoing messages,
>        build frame(s) by using the compressed data instead of the
>        original data for the message payload, and setting the
>
>  > The condition "if this PCME is the last extension to process
>  > outgoing messages" is meaningless, here and further down in this
>  > section.  The process is the same for all PMCEs regardless if it is
>  > the first, middle, or last extension.
>  >
>  > Also, with the current spec, we would argue that it is impossible
>  > to have multiple active PMCEs anyway (due to the RSV1 bit requirement)
>
>

[TBA]


>      6.2.  Decompression  . . . . . . . . . . . . . . . . . . . . . . 11
>
>  > Again, same commentary about the "PCME is the last extension"
>  > condition being irrelevant
>
>

[TBA]


>    7.  Intermediaries . . . . . . . . . . . . . . . . . . . . . . . . 13
>
>  > good, but could use some commentary that if the intermediaries are
>  > not aware of the opening handshake, then they should make no
>  > modifications to the frames.  See [Section 5.4] of [RFC6455]
>  > 3rd paragraph for similar text.
>
>

[TBA]


>    8.  permessage-deflate extension . . . . . . . . . . . . . . . . . 14
>
>  > OK, some suggestions on how to clean this section up.
>  >
>  > Introduce the 4 extension parameters first.
>  >
>  >   Use the language found elsewhere in the spec now about the asymmetric
>  >   nature of the extension parameter configuration.
>  >
>  >   The following 2 extension parameters control the behavior of the
>  >   deflate configuration for Client-to-Server messages:
>  >   (We took this notation from the title of [Section 5.3] of [RFC-6455],
>  >    including the capitalization)
>  >
>  >     o  "c2s_no_context_takeover"
>  >     o  "c2s_max_window_bits"
>  >
>  >   The following 2 extension parameters control the behavior of the
>  >   deflate configuration for Server-to-Client messages:
>  >
>  >     o  "s2c_no_context_takeover"
>  >     o  "s2c_max_window_bits"
>  >
>  > Then describe how the Offer works. (essentially the language you
>  > have now in the doc, with careful attention to not mix your intro
>  > and then bullets)
>  >    Request: desired client behavior + desired server behavior
>  >    Response: acknowledge client behavior + server chosen behavior
>  >
>  > Example of confusing language:
>
>    For an offer for this extension, the following 4 extension parameters
>    are defined.  If needed to distinguish from ones for a response,
>    these parameters are called with a prefix "client-to-server".
>
>    o  "s2c_no_context_takeover"
>
>    o  "c2s_no_context_takeover"
>
>    o  "s2c_max_window_bits"
>
>    o  "c2s_max_window_bits"
>
>  > This section introduces you to "client-to-server", then lists
>  > s2c_*, why?  Same for the next section, but in reverse.
>  > Don't mix the introduction of the extension parameters and
>  > the behavior of the request and response together.
>  >
>  > Finally, go into the MUST rules for Client and Server like you
>  > currently have in the spec documentation.
>   >
>  > Include wording for Client MUST pointing back to close code
>  > 1010 from [RC6455] as a recommended behavior for dealing with
>  > bad extension negotiation.
>  > Even for things like a server saying s2c_max_window_bits=1024
>
>

[TBA]


>      8.1.  Method Parameters  . . . . . . . . . . . . . . . . . . . . 15
>        8.1.1.  Context Takeover Control . . . . . . . . . . . . . . . 15
>
>  > Include explicit language indicating default behavior for
>  > context takeover.
>
>  > When documenting one side, for example "s2c_", do not make references
>  > to the opposite direction.  Its confusing, noisy, and adds no value.
>  >
>  > For example, Section 8.1.2.2. c2s_max_window_bits, start out talking
>  > about the c2s_max_window_bits, then has suddenly talks about
>  > "server-to-client parameter", mentioning behavior and server limits.
>  > This is confusing.
>  > Fix this, clarify it, eliminate the noise, or simplify it.
>  >
>  > Eg of simplification:
>  >
>  >  See [Section 8.1.2.1.] "s2c_max_window_bits" for server side
>  >  equivalent
>  >
>  > We suspect that the use of "c2s" and "client-to-server" to
>  > interchangably refer to either the "extension parameter token" itself
>  > or the phase of the extension negotiation (request or response) is
>  > the cause of the confusion.
>  >
>  > Either choose new terminology for the extension parameter token, or
>  > choose new terminology for the phase of the extension negotiation
>  > (request side or response side)
>  >
>

[TBA]


>
>        8.1.2.  Limiting the LZ77 sliding window size  . . . . . . . . 16
>
>  > Include explicit language indicating default size for
>  > LZ77 sliding window size.
>

Added texts.


>  > RFC1979 seems to indicate 32KB as the maximum value, but no obvious
>  > word on what is the default behavior, assuming no window size is
>  > specifically declared.
>  >
>  > The ABNF for the c2s_max_window_bits indicates that a digit is required,
>  > but the text indicates that no value is possible.
>  > What does no-value mean?
>

It means that only capability is announced with no hint.

Added texts.


>
>        8.1.3.  Example  . . . . . . . . . . . . . . . . . . . . . . . 18
>
>  > See earlier commentary about no value c2s_max_window_bits extension
>  > parameter token.
>  > See earlier commentary about extension negotiation for example #3
>  > (fallback behavior written into this spec)
>
>      8.2.  Message Payload Transformation . . . . . . . . . . . . . . 19
>
>  > good
>  > (one note, the BFINAL logic needs to be validated in current
>  > implementations.  as BFINAL does not behave the same in pywebsocket,
>  > autobahn, chrome, and safari).  We had to hack BFINAL to always be
>  > 0 for outgoing frames from our server in order for existing
>  > client implementations to work.
>

Sorry. Could you please elaborate this?


>
>        8.2.3.  Examples . . . . . . . . . . . . . . . . . . . . . . . 21
>
>  > Include example of multiple message behavior with no_context_takeover
>  > configured.
>

In "Sharing LZ77 Sliding Window" section, the second example is identical
to the first one. This is the example of multiple message behavior with
no_context_takeover.

>   This is the payload of the first message the client has sent.  If the
>   "agreed parameters" contain the "c2s_no_context_takeover" extension
>   parameter, the client compresses the payload of the next message into
>   the same bytes (if the client uses the same "BTYPE" value and
>   "BFINAL" value).  So, the payload of the second message will be:
>
>       0xf2 0x48 0xcd 0xc9 0xc9 0x07 0x00

Added concatenated examples.


>
>      8.3.  Implementation Notes . . . . . . . . . . . . . . . . . . . 24
>
>  > This section's updates are good.
>  > The LZ77 sliding window size recommendation is ok, but where's the
>  > default LZ77 sliding window size?
>

This sentence in "Compression" section

> An endpoint MUST NOT use an LZ77 sliding window longer than 32,768 bytes
to compress messages to send.

and these sentences in "Decompression" section are intended to define
default LZ77 sliding window size.

> Otherwise, the client MUST use a 32,768 byte LZ77 sliding window to
decompress received messages.

> Otherwise, the server MUST use a 32,768 byte LZ77 sliding window to
decompress received messages.


>
>      8.4.  Intermediaries . . . . . . . . . . . . . . . . . . . . . . 24
>
>  > Isn't this section identical to [Section 7] in this same spec?
>

Yes. Section 7 is talking about interaction with intermediaries in general,
and this section is saying almost the same thing but using
permessage-deflate specific language for example.