Skip to content
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

SIP Cancel request is not received by the intended client #160

Open
p-akshay opened this issue Dec 3, 2024 · 5 comments
Open

SIP Cancel request is not received by the intended client #160

p-akshay opened this issue Dec 3, 2024 · 5 comments
Labels

Comments

@p-akshay
Copy link

p-akshay commented Dec 3, 2024

I am working on a simple scenario where after call initiation Cacnel request is sent.

There are 2 client, UserA and UserB who are connected to the sip server. The SIP Server is based on sipgo whereas the clients are based on SIPjs.

Scenario:
UserA calls UserB via SIP Server. Then UserA initiates the CANCEL Request. A similar CANCEL Request is required to be generated by SIP Server towards UserB.

In this case, the Cancel request is sent back to UserA instead of UserB.

Code snippet to generate Cancel Request:
`cancelReq := sip.NewRequest(sip.CANCEL, outDlg.InviteRequest.Recipient)
cancelReq.AppendHeader(sip.HeaderClone(outDlg.InviteRequest.Via())) // Cancel request must match invite TOP via and only have that Via
cancelReq.AppendHeader(sip.HeaderClone(outDlg.InviteRequest.From()))
cancelReq.AppendHeader(sip.HeaderClone(outDlg.InviteRequest.To()))
cancelReq.AppendHeader(sip.HeaderClone(outDlg.InviteRequest.CallID()))

cancelReq.SetSource(outDlg.InviteRequest.Source())
cancelReq.SetDestination(outDlg.InviteRequest.Destination())`

Extra Observation:
If I remove the line in the above code that copies the Via header, then UserB will correctly receive the Request. However, the client rejects the request because, according to RFC 3261, the Cancel must have the same Via as the original INVITE.

Please advise about the same. If you need more detailed traces, let me know, and I can provide them.

@emiago
Copy link
Owner

emiago commented Dec 3, 2024

Hi @p-akshay
Thanks for reporting. Yes via must kept the same in order to match transaction.
Now I am having a feeling that you are buiding some B2BUA but maybe you are sending request like proxy.

In diago (https://github.com/emiago/diago) (B2BUA) this is handled for you by just canceling context on INVITE. You only need to make sure that you are passing context from received dialog

@p-akshay
Copy link
Author

p-akshay commented Dec 4, 2024

Hi @emiago ,

Ok...Thanks for the suggestion.

I am indeed working on a B2BUA kind of design. One question w.r.t. Diago library... will I be able to use the same and sipgo together? in my application dialog plays an important role but other sip events are also required. I was not sure via documentation if the diago allows for the handling of other sip events.

One more question...will the same fix be added in sipgo?

Thanks

@emiago
Copy link
Owner

emiago commented Dec 4, 2024

Hi @p-akshay, I am not sure what fix you are expecting, as diago already handles this.

Diago library... will I be able to use the same and sipgo together?

You can override behavior by passing your server and then registering handlers after

NewDiago(ua, WithServer(srv))

srv.OnOptions(....)

Diago registers what it needs and mostly it is arround dialog for now.

@yangjuncode
Copy link

i have the same problem, the cause is in func (c *Client) TransactionRequest in client.go

	if cseq := req.CSeq(); cseq != nil {
			// Increase cseq if this is existing transaction
			// WriteRequest for ex ACK will not increase and this is wanted behavior
			// This will be a problem if we allow ACK to be passed as transaction request
			cseq.SeqNo++
			cseq.MethodName = req.Method
		}

the cancel req should not call cseq.SeqNo++, orelse sip server can not found the transaction, in my test, the freeswitch will response 481.

and my hack now is seqno-- before call TransactionRequest.

func (d *TSipDialog) newCancelRequest() *sip.Request {
	cancelReq := sip.NewRequest(sip.CANCEL, d.inviteRequest.Recipient)
	// Cancel request must match invite TOP via and only have that Via
	cancelReq.AppendHeader(sip.HeaderClone(d.inviteRequest.Via()))
	cancelReq.AppendHeader(sip.HeaderClone(d.inviteRequest.From()))
	cancelReq.AppendHeader(sip.HeaderClone(d.inviteRequest.To()))
	cancelReq.AppendHeader(sip.HeaderClone(d.inviteRequest.CallID()))
	// TODO:sipgo add ++, so we need -- here
	cseqHeader := sip.HeaderClone(d.inviteRequest.CSeq()).(*sip.CSeqHeader)
	cseqHeader.SeqNo--
	cancelReq.AppendHeader(cseqHeader)
	cancelReq.AppendHeader(sip.HeaderClone(d.inviteRequest.GetHeader("User-Agent")))
	sip.CopyHeaders("Route", d.inviteRequest, cancelReq)
	cancelReq.SetSource(d.inviteRequest.Source())
	cancelReq.SetDestination(d.inviteRequest.Destination())
	return cancelReq
}

@emiago what's your opinion about this?

@emiago
Copy link
Owner

emiago commented Dec 22, 2024

HI @yangjuncode . It is not issue if you are using dialog. I am still happy to remove this implicit behavior, but I think it also solves some issues, like re REGISTER.

Generally I see it is bad. I will have a look
As for workarround I think it should be enough to pass some empty option func.

Thanks for brining this up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants