ietf
[Top] [All Lists]

Re: Gen-art last call review of draft-ietf-httpbis-http2-16

2015-01-26 15:30:23
I've updated the pull request.  You can see the changes there.

Note that the reason I'm pushing back hard on text additions is that
it's hard to ensure that I've correctly maintained the intent of text
that has already been extensively reviewed.  Deletions are easier, but
only slightly.  If you feel strongly about anything I've not acted on,
feel free to raise it.

On 21 January 2015 at 09:36, Elwyn Davies 
<elwynd(_at_)dial(_dot_)pipex(_dot_)com> wrote:
It's the WG's choice to limit transports to TCP : so be it.  I'd have liked
to allow alternatives such as SCTP but that is a personal view.

The only reason we did so is that no one offered to do the work.

How about adding "subject to limited constraints" to the Section 5 bullet.

The statement is still correct without it.

I have done a bit of homework on this including having a look at
draft-pironti-tls-length-hiding-01,
so I hope I understand this a little better now.  Practically, I think it
would be helpful to move the last paragraph of s6.1 and combine it with the
second para that introduces the idea of padding, making the security
implications more upfront.  I think I am correct in saying that padding
would be pointless if not using TLS: I think it would be worth saying this
in s6.1.

Sounds reasonable.

I think an (informative) reference to draft-pironti-tls-length-hiding-01 in
s10.7 would be useful - I think it covers the 'redundant padding could be
counterproductive' story in the 2nd sentence and also the statements in the
last para of s10.7.

I think that I would, if it weren't for the relative levels involved
here.  The idea here is to include a clear signal that padding isn't
necessarily easy and then leave it to implementers to learn that this
warning was perhaps a little strong.  Better that than a false sense
of security.

I don't think this is a problem.  Most proposals that have been
floated use a reciprocal SETTINGS frame to signal that the value was
understood.  The only failure mode there occurs when there is a
collision between two extensions that results in two conflicting views
of the setting semantics.  Of course, that is equally possible with a
rejection-based scheme.

Sorry, but I don't understand your logic here.  Let me try again:

If (say) the client asks for a SETTINGS value from some future extension
that the server doesn't understand, my interpretation is that the server
will ignore the value it doesn't understand while doing the ones that it
does understand and say 'ACK' to the client. I assume that the client is
then entitled to believe that the server has interpreted its requested
SETTINGS as expected and the client can do whatever the extension was
intended to allow.  Depending on what the ignored SETTINGS are, this may
have bad consequences during later operations.

Accordingly, I think that the SETTINGS extension needs some way to tell the
requesting endpoint that the responding endpoint couldn't action (part of)
its requests.  The requester can then act accordingly, terminating the
session or avoiding actions that exploit whatever setting couldn't be
actioned.

Let me try again.  The ACK only provides an indication that the
settings that were understood were acted upon.  It doesn't provide
proof-positive that an extension was understood.  Another mechanism is
required for that.  One potential solution is to include a value for
the same setting in a separate SETTINGS frame.

s3.5, paras 2-5: It would be worth explaining that what is being done
here
is to send what would be a method request for the PRI method which will
not
be recognized by well-implemented HTTP/1.x servers.  The PRI method is
then
reserved for HTTP/1.1 so that it can't be accidentally implemented later
(see Section 11.6).

We've discussed providing explanatory text of this nature in the
working group on a few occasions, but avoided it.  In this case, the
main beneficiaries of that information are people who aren't reading
and implementing this specification, so we decided against more
verbiage to that effect.

Well.. with my gen-art hat firmly in place, part of the object of RFCs is
that they shouldn't be totally opaque to those who are reading them, even
those who aren't actually experts implementing the protocol.  My suggestion
for an explanation would help those.. I didn't immediately twig why the
string was structured as it is.

The right place for this discussion is the working group list.  If you
feel strongly enough.  I'm trying very hard to avoid (re)opening
issues.

<<snip>>

s5.1, next to last para; s5.5, para 3, s6.2, END_HEADERS, para 2:
s5.1 says:
OLD:
Frame of unknown types are ignored.
[...]
I have little concern that the feature
will be accidentally missed.

Perhaps not but I spent some time worrying about the inconsistency so other
may..
can we compromise on:
NEW:
Frames of unknown types are ignored except when they occur in CONTINUATION
frame sequences (see Sections 5.5, 6.2 and 6.6).

My inclination is to remove the sentence entirely rather than fix it.

s5.1.1, last para/s3.4, last para, s9.1, para 3:  Having to open a new
connection when stream ids run out interacts with the point made in s3.4
that there is no guarantee that the new connection will support HTTP/2.
It
might be worth pointing this out in s5.1.1 and s9.1 - it means the client
has to be able to switch back to HTTP/1.1 in the very rare case that this
happens. Thought: Would it be useful for a server to have a SETTINGS flag
that guarantees that it would do HTTP/2 on all subsequent connections?

Yes, and the fallback isn't necessarily pretty.  At least not for
performance.  I think that the view is that this is a low risk
scenario and not worthy of special attention.

Hmm.. On the other hand, in the rare case that it does happen it might bite
the client nastily if not catered for.  I think it is worth a short sentence
in s5.1.1 at least.

If I were to do anything, I'd have thought that Sec3 was more
appropriate, but I couldn't see anywhere there.

One virtue of HTTP is that implementations are generally stateless.
That extends to the transport/protocol mappings.  This error doesn't
arise because maintaining state about whether a server previously
supported X is actually harder than just running the same logic every
time.  With ALPN that's free.

s6.2, END_STREAM flag, 2nd para: s/A HEADERS frame carries/A HEADERS
frame
can carry/

It always carries the flag, it is whether it is set that matters.

In fact, isn't the first sentence of s6.2, para 2 redundant?  The previous
para just said it signals stream end.  I think you can delete it completely.

I think that I got the text in question.  Yeah.