allow STOP_SENDING on implicitly opened streams

[marten-seemann]

As suggested in #2345 (comment).
Fixes #2345.

[mikkelfj]

I am not necessarily objecting to this, but before just merging this in, please review past discussions on this - it is not as trivial as it might seem.

#1050
#1797

[marten-seemann]

What concrete problem do you see, @mikkelfj?

[mikkelfj]

What concrete problem do you see, @mikkelfj?

I don't see a concrete problem, but I do remember a lot of pitfalls, so I just wanted to make a note of it before anything gets merged. I have not yet read through the issues in detail.

One concern is race conditions but it might be captured by the proposed text, I'm just not sure yet.

Also, it is difficult to grasp exactly what is the difference between not yet created, created, Ready, peer has not seen data from higher stream but ID is below MAX_STREAM_ID.

[mikkelfj]

propose:

Receiving a STOP_SENDING frame for a locally-initiated stream that has not transitioned
beyond the Ready state MUST be treated as a connection error of type STREAM_STATE_ERROR unless there is a higher numbered stream that has made this transition already (see {{stream-send-states}}). A peer MUST NOT send STOP_SENDING until it has received a frame related to the same stream, or to a higher numbered stream of the same type. An endpoint that receives a STOP_SENDING frame for a receive-only stream MUST terminate the connection with error STREAM_STATE_ERROR.

See also discussion on associated issue. Removing Ready is not safe as this can lead to speculative STOP_SENDING, for example based on assumed out of band knowledge of the senders local state.

Note: I am not exactly sure which frames a the peer must have received. Does flow control count as opening the stream as well? I suppose so.

[kazuho]

I think the other approach we might consider is to state that a stream enters a Ready state when the endpoints send a frame containing a higher stream ID.

Taking that approach might help resolve @mikkelfj's concern that the definition is becoming loose.

[Ralith]

Enters the Send state, I think you mean? Ready is the state that currently forbids STOP_SENDING.

That seems like an effective solution, although I find this use of "created" intuitive and consistent with the "Within each type, streams are created with numerically increasing stream IDs" language.

[marten-seemann]

I don't think we gain anything by worrying about out-of-band knowledge about stream states, and I don't find @mikkelfj's text suggestion very readable. The same would apply to MAX_STREAM_DATA frames for remote-opened streams as well, and we don't do anything to prevent that either.

[mikkelfj]

Note that I just edited my text. I think @kazuho has a point. But it still needs to be clear what the remote endpoint must observe before sending.

[mikkelfj]

The same would apply to MAX_STREAM_DATA

I think that needs to be fixed as well. The frame might get lost due to race conditions.

[marten-seemann]

The same would apply to MAX_STREAM_DATA

I think that needs to be fixed as well. The frame might get lost due to race conditions.

Can you elaborate where you see a race condition here?

[mikkelfj]

Can you elaborate where you see a race condition here?

The peer sends MAX_STREAM_DATA on stream x and stream x + 4 because they are "always" opened together. The sending endpoint has only set stream x in transmission mode and is about to do the same for stream x + 4 when it receives MAX_STREAM_DATA for stream x + 4. Since MAX_STREAM_DATA is sent to a non-existent stream it either faithfully terminates the connection, or just ignores the frame since it is not between min/max range of active streams.

[marten-seemann]

I don't see any race condition we need to fix here. If it was permissible to send MAX_STREAM_DATA x+4, no amount of reordering will make this a protocol violation. If it was not permissible, and the remote peer just happened to open x+4 while the MAX_STREAM_DATA frame was in flight, then the peer got lucky this one time and the protocol violation won't be detected. I don't think this is something we need to address though.

[mikkelfj]

If it was permissible to send MAX_STREAM_DATA x+4, no amount of reordering will make this a protocol violation

then you have to be able track infinite amount state because the peer controls MAX_STREAM_ID.

If it was not permissible, and the remote peer just happened to open x+4 while the MAX_STREAM_DATA frame was in flight, then the peer got lucky this one time and the protocol violation won't be detected.

You can always get lucky, by it is sufficient to risk enforcement, and more importantly, it protects against tracking infinite state.

[marten-seemann]

then you have to be able track infinite amount state because the peer controls MAX_STREAM_ID.

No, you don't, because you're allowed to silently drop MAX_STREAM_ID frames for closed streams. We have text about that already.

[mikkelfj]

No, you don't, because you're allowed to silently drop MAX_STREAM_ID frames for closed streams. We have text about that already.

I am talking about not yet opened streams.
But is you said in the #2353 there is text protecting against this - only I believe that the text is not very precise.
Wrt. STOP_SENDING I basically want the same protection, and for both frames, I'd like it to be clear what the sender of these reverse frames need to observe before concluding that it is safe to send them.

[Ralith]

I agree that MAX_STREAM_DATA and STOP_SENDING should have exactly the same rules for when they're accepted, and that MAX_STREAM_DATA's current language, while somewhat vague, permits the case that this issue enables for STOP_SENDING. When we settle on a way to phrase this, it might be nice to use exactly the same language for both frames to make that clear.

[martinthomson]

I think that we can do a little more (for instance, to the preceding sentence). See #2354.

[martinthomson]

#2354 supercedes this.