Conversation
|
Thanks, the ACK/PRACK fix makes sense. One issue remains: this regresses ACKs for re-INVITEs when the caller does buildReq initializes a missing CSeq from the initial INVITE, and ACK now skips the dialog-counter assignment. After Previously this inherited the current dialog CSeq, and WriteAck does not document that callers must set it manually. |
189f966 to
b9e6faf
Compare
DialogClientSession.buildReq derives the CSeq of every request within the dialog from the dialog counter, ACK and CANCEL included. RFC 3261 requires the opposite: an ACK carries the sequence number of the INVITE it acknowledges (13.2.2.4) and a CANCEL the one of the request being cancelled (9.1), and neither consumes a number of its own. The two values differ as soon as anything is sent between the INVITE and its final response. With reliable provisional responses that is the normal case: every PRACK moves the counter on, so the ACK for the 2xx leaves with the sequence number of the last PRACK. The UAS does not match it to the INVITE, keeps retransmitting the 2xx until the timer expires and then releases the call, which makes every call using 100rel fail to establish. The number an ACK or a CANCEL carries is therefore, in order: the one the caller set on the request, or the one of the last INVITE sent within the dialog. The dialog keeps that number in lastInviteCSeqNo, next to the counter it already had. Deriving it from the counter is what breaks with 100rel, and taking it from Dialog.InviteRequest alone would break the ACK for a re-INVITE built without a CSeq header, which WriteAck accepts and does not ask callers to fill in. The counter itself is left alone by ACK and CANCEL, so that the next request stays above the PRACK instead of colliding with it. The added tests fail on the code before this change and pass after it: the ACK after two PRACKs, an ACK whose CSeq the caller set, and an ACK for a re-INVITE built without one.
b9e6faf to
1b77f01
Compare
|
Good catch, thanks — the re-INVITE ACK without an explicit CSeq did regress, and I've reproduced it in a test. Restoring the old behaviour would bring back the original bug, though: the dialog counter is wrong whenever a PRACK is sent between the INVITE and its final response, and that applies to a re-INVITE using 100rel just as much as to the initial one — there both the counter and Dialog.InviteRequest give the wrong number. So the number an ACK or CANCEL carries is now, in order: the one the caller set on the request, otherwise the one of the last INVITE sent within the dialog, which the dialog keeps in lastInviteCSeqNo next to the counter it already had. WriteAck still accepts a bare ACK; the default is simply correct now. Added TestDialogClientAckCSeqReInviteWithoutHeader for exactly the case you described — it fails without lastInviteCSeqNo, with the sequence number of the initial INVITE. The branch is rebased onto current main (dfd6dc5) and go test ./... passes. |
2 participants
DialogClientSession.buildReq derives the CSeq of every request within the dialog from the dialog counter, ACK and CANCEL included. RFC 3261 requires the opposite: an ACK carries the sequence number of the INVITE it acknowledges (13.2.2.4) and a CANCEL the one of the request being cancelled (9.1), and neither consumes a number of its own.
The two values differ as soon as anything is sent between the INVITE and its final response. With reliable provisional responses that is the normal case: every PRACK moves the counter on, so the ACK for the 2xx leaves with the sequence number of the last PRACK. The UAS does not match it to the INVITE, keeps retransmitting the 2xx until the timer expires and then releases the call, which makes every call using 100rel fail to establish.
Take the number from the CSeq the caller set, or from the initial INVITE filled in above, and leave the dialog counter alone so that the next request stays above the PRACK instead of colliding with it. An ACK for a re-INVITE keeps working: the caller sets the CSeq of that re-INVITE and it is no longer overwritten.
The added tests fail on the code before this change and pass after it.