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.

Sign up for GitHub

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

Jump to bottom

incorrect error code used for invalid RETIRE_CONNECTION_ID frames #3860

Closed
marten-seemann opened this issue Jul 8, 2020 · 6 comments · Fixed by #3861
Closed

incorrect error code used for invalid RETIRE_CONNECTION_ID frames #3860

marten-seemann opened this issue Jul 8, 2020 · 6 comments · Fixed by #3861
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.
Projects
Late Stage Processing

Comments

@marten-seemann
Copy link
Contributor

The sequence number specified in a RETIRE_CONNECTION_ID frame MUST NOT refer to the Destination Connection ID field of the packet in which the frame is contained. The peer MAY treat this as a connection error of type FRAME_ENCODING_ERROR.

Our principle regarding error codes states that only those errors that are detectable without accessing connection state should result in a FRAME_ENCODING_ERROR. Since the RETIRE_CONNECTION_ID frame contains only the sequence number of the connection ID, but not the actual connection ID itself, this should be a different error.
Looking through the list of error codes, it seems like we don't have any more specific error code here, so this would be a PROTOCOL_VIOLATION.
This was referenced Jul 8, 2020
Merged
Merged
@martinthomson
Copy link
Member

Seems OK.

@larseggert larseggert added this to Triage in Late Stage Processing via automation Jul 8, 2020
@LPardue
Copy link
Member

LPardue commented Jul 9, 2020

As the proposed PR indicates, this would be a design change.

@ianswett
Copy link
Contributor

Agreed, this seems OK.

@janaiyengar
Copy link
Contributor

As I noted in the PR, I agree that PROTOCOL_VIOLATION seems like a more appropriate error. In addition to @marten-seemann's point, this is a semantic error, not one of encoding (which suggests syntactic).

@LPardue LPardue added the design An issue that affects the design of the protocol; resolution requires consensus. label Jul 14, 2020
@project-bot project-bot bot moved this from Triage to Design Issues in Late Stage Processing Jul 14, 2020
@larseggert
Copy link
Member

It seems we have a proposed resolution, marking as such.

@larseggert larseggert added the proposal-ready An issue which has a proposal that is believed to be ready for a consensus call. label Jul 21, 2020
@project-bot project-bot bot moved this from Design Issues to Consensus Emerging in Late Stage Processing Jul 21, 2020
@larseggert larseggert moved this from Consensus Emerging to Consensus Call issued in Late Stage Processing Jul 22, 2020
@LPardue LPardue added call-issued An issue that the Chairs have issued a Consensus call for. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list. and removed proposal-ready An issue which has a proposal that is believed to be ready for a consensus call. call-issued An issue that the Chairs have issued a Consensus call for. labels Jul 24, 2020
@project-bot project-bot bot moved this from Consensus Call issued to Consensus Declared in Late Stage Processing Jul 29, 2020
Late Stage Processing automation moved this from Consensus Declared to Issue Handled Jul 29, 2020
@dtikhonov
Copy link
Member

PR #3042 was less than a year ago, that's when this error code went from PROTOCOL_VIOLATION to FRAME_ENCODING_ERROR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
-transport design An issue that affects the design of the protocol; resolution requires consensus. has-consensus An issue that the Chairs have determined has consensus, by canvassing the mailing list.
Projects
Late Stage Processing
  
Issue Handled
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

change error to PROTOCOL_VIOLATION when receiving an invalid RCID frame marten-seemann/base-drafts
7 participants
@martinthomson @larseggert @marten-seemann @dtikhonov @LPardue @janaiyengar @ianswett