-
Notifications
You must be signed in to change notification settings - Fork 41
Use a control message identifier to correlate requests and responses #898
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
Conversation
This is more wire efficient and less ambiguous Fixes: #804
ianswett
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR doesn't add any limits on the control message ID, so there's no protection against rapid reset type attacks as you noted.
I think I'd be simpler to rename Subscribe ID to Control Message ID, and since Subscribe ID already has a MAX mechanism, we get limits on outstanding control messages for free.
| responses. There are independent control message IDs for each endpoint. The | ||
| client's control message ID starts at 0 and the server's control message ID | ||
| starts at 1. The control message ID increments by 2 with ANNOUNCE, | ||
| SUBSCRIBE_ANNOUNCES or TRACK_STATUS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we define a rule for computing the control message ID ?
I may be wrong, do we support server sending its own control message today ? It does respond to one, but will send its own though right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, a relay sends ANNOUNCE and SUBSCRIBE today.
|
#905 is my preferred alternate to this PR |
Co-authored-by: Zafer Gürel <[email protected]>
|
Based on feedback, let's proceed with #905 |
This is more wire efficient and less ambiguous
Fixes: #804