baresip / re

Generic library for real-time communications with async IO support
BSD 3-Clause "New" or "Revised" License
129 stars 80 forks source link

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

Closed maximilianfridrich closed 3 months ago

maximilianfridrich commented 3 months ago

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

cspiel1 commented 3 months ago

Looks good now.

cspiel1 commented 3 months ago

This is ready for the merge from our side.

juha-h commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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

juha-h commented 3 months ago

https://github.com/baresip/re/pull/1129 looks OK, since the added sip_dialog_cmp_half test compares To/From tags and Call-ID.