sipsess/connect: set sess->established immediately on 200 receival

[maximilianfridrich]

Otherwise, if something in invite_resp_handler fails and sipsess_terminate is called, no BYE will be sent.

[cspiel1]

not sure about this change

[cspiel1]

Without a dialog sending a BYE is not possible

[maximilianfridrich]

The dialog is created in sipsess_connect. Even if this fails, a BYE can still be sent.

[cspiel1]

Should we send ACK if sip_dialog_create fails?

[cspiel1]

If yes, then I would prefer:

if (sdp && !err) {
/*call answer/offer handler*/
}

instead of a second sipsess_ack()

[maximilianfridrich]

There are two cases for why sip_dialog_create might fail:

1. We received a malformed response (Contact or Record-Route header are not a valid SIP URI or To tag is not set).
2. Something internally failed on our side (e.g. ENOMEM).

Regarding point 1: I looked through RFC 3261 thoroughly and I could not find anything that states what should be done when a malformed response is received to an INVITE. At this point in the code a lot of validation has happened before (sip_msg_decode and the Via branch is matched in ctrans.c.).

So I believe sending an ACK is okay even if the Contact or Record-Route headers are malformed - a BYE is sent immediately afterwards anyways to terminate the session.

Same for the second case, I think we should still try to send an ACK even if something failed internally and then we try to terminate the session.

[cspiel1]

Looks good now.

[cspiel1]

This is ready for the merge from our side.

[juha-h]

I somehow missed this PR. When invite_resp_handler is called on 2xx response, it is sure that it matches the dialog of the INVITE, i.e., To, From, Call-ID and CSeq? If not, the dialog should not be transitioned to the "confirmed" state and thus the session should not be established (https://www.rfc-editor.org/rfc/rfc3261#section-13.2.2.4).

[maximilianfridrich]

Thank you @juha-h.

In transp.c:sip_recv sip_msg_decode() is called and have_essential_fields() is checked. Then in ctrans.c:response_handler() the Via branch and CSeq method are matched to find the correct client transaction. That's all the checking that happens.

Do you think in invite_resp_handler we should check To, Call-ID and CSeq?

e.g.:

	if (!sip_dialog_cmp_half(sess->dlg, msg)
			|| sip_dialog_lseq(sess->dlg) != msg->cseq.num) {
		goto out;
	}

[juha-h]

My understanding is that for transaction match it is enough that Via and CSeq of response matches to those of the request. But for creation of a session, also dialog identifiers must match. So To/From tag and Call-ID check would be needed.

[maximilianfridrich]

@juha-h Please check #1129 and comment on the PR if that looks good to you.

[juha-h]

#1129 looks OK, since the added sip_dialog_cmp_half test compares To/From tags and Call-ID.