-
Notifications
You must be signed in to change notification settings - Fork 815
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
Handle 2nd INVITE when in early state #3458
Conversation
As a reference, here's RFC 3261 section 14.2:
|
Please use enum instead of magic numbers, e.g. 500 or 491. |
9c2031f
to
9389560
Compare
Fixed the comments |
Please find the attached patch below. Mostly, just to fix the indentation, but also one build failure of missing semicolon. I have tested it here and it seems to work as expected. |
9389560
to
60df91c
Compare
pjsip/src/pjsip-ua/sip_inv.c
Outdated
* between 0-10. | ||
*/ | ||
pjsip_retry_after_hdr *ra_hdr; | ||
int val = (pj_rand() % 9) + 1; |
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.
There is a similar handling in inv_on_state_confirmed()
, and it use:
int val = (pj_rand() % 10);
Yes it is unclear whether between 0-10
should be inclusive, but I guess we better apply the same formula in early and confirmed?
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.
So changing back to int val = (pj_rand() % 10);
?
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.
It looks like between
in the RFC means inclusive such as any response with a status code between 100 and 199 is referred to as a "1xx response"
.
The correct one should be pj_rand() % 11
. But I think % 10
is safer in case that there's any UA that rejects a value of 10.
So yes, please revert it to pj_rand() % 10
. Thank you.
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.
Done
Ensures 2nd INVITE is handled if in state early.
60df91c
to
9902c87
Compare
Ensures 2nd INVITE is handled if in state early.