cloudwebrtc / go-sip-ua

Go SIP UA library for client/b2bua
Apache License 2.0
215 stars 84 forks source link

ua.go incorrectly assumes that a received invite is a reinvite. #72

Closed danieldonoghue closed 2 years ago

danieldonoghue commented 2 years ago

in ua.go, handleInvite(), it is assumed that if a transaction is found, the incoming request must be a reinvite. this is not necessarily correct..

if an INVITE is received and it has a "t"o tag, it is a reinivte. if there is no transaction found for that request, a 481 "transaction does not exist" response should be sent. However, if the transaction is found, this should be considered a reinvite.

if an INVITE is received that has no to tag, it is not part of a sip dialog and, as such, cannot be a reinvite. it should be considered a new invite (or a retransmission due to lack of timely response) unless a matching transaction, for the branch id and call-id combination already exists. In which case, a 482 loop detected response (or similar) should be sent.

in all other cases, the INVITE should be considered a new request.

danieldonoghue commented 2 years ago

I propose a small change to handleInvite() to check the "to" tag before setting the state to reinvite. You could leave everything else treated as new new invite with further processing in the caller's inviteHandler but I don't know whether that would have knock on implications anywhere in your package..

callID, ok := request.CallID()
if ok {
    var transaction sip.Transaction = tx.(sip.Transaction)
    v, found := ua.iss.Load(*callID)
    if request.To().Params().Has("tag") {
        if found {
            is := v.(*session.Session)
            is.SetState(session.ReInviteReceived)
            ua.handleInviteState(is, &request, nil, session.ReInviteReceived, &transaction)
        } else {
            // some equivalent to: is.Reject(sip.StatusCode(481), "Call/Transaction does not exist")
        }
    } else {
        contact, _ := request.Contact()
        is := session.NewInviteSession(ua.RequestWithContext, "UAS", contact, request, *callID, transaction, session.Incoming, ua.Log())
        ua.iss.Store(*callID, is)
        is.SetState(session.InviteReceived)
        ua.handleInviteState(is, &request, nil, session.InviteReceived, &transaction)
        is.SetState(session.WaitingForAnswer)
    }
}

note this still doesn't take into account multiple branches, which might require a change to the key for the session store to include a branch id

danieldonoghue commented 2 years ago

I created a pull request addressing these issues... https://github.com/cloudwebrtc/go-sip-ua/pull/73

cloudwebrtc commented 2 years ago

Hey @danieldonoghue, cool, thanks for your contribution, I will test and merge in this week

danieldonoghue commented 2 years ago

PR merged