Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change illegal PRIORITY frame to a conn error #2508

Merged
merged 1 commit into from
Mar 9, 2019
Merged

Change illegal PRIORITY frame to a conn error #2508

merged 1 commit into from
Mar 9, 2019

Conversation

martinduke
Copy link
Contributor

Fixes #2507.

@MikeBishop MikeBishop added design An issue that affects the design of the protocol; resolution requires consensus. -http labels Mar 9, 2019
@MikeBishop MikeBishop merged commit 3b3a20f into quicwg:master Mar 9, 2019
@kazuho
Copy link
Member

kazuho commented Mar 9, 2019

Has this been the only case where a protocol violation resulted in a stream error? Or should we check if there are similar cases and file PRs as necessary?

I ask this, because IIUC at least some of us considered the proposal as a change of design principle, to stop localizing such errors, and I wonder if the change made in this PR is a tip of an iceberg.

@martinduke
Copy link
Contributor Author

I haven't comprehensively reviewed the draft, but having just implemented the HTTP/3 frame parser, I can say that other UNEXPECTED_FRAME instances are connection errors.

Furthermore, PRIORITY frames can arrive on the control stream. If there were a stream error there it would have to take down the connection anyway.

@MikeBishop
Copy link
Contributor

@kazuho, that's a good question. Previously, the philosophy has been that connection errors are ones which screw up the state so badly you can't trust the connection to proceed correctly. It's a reasonable, but not bulletproof, argument that a peer which doesn't obey the protocol has the same implication -- you can't trust the connection.

If you see other protocol violations which are stream-local errors, please file issues and we'll discuss.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-http design An issue that affects the design of the protocol; resolution requires consensus.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants