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

Activity

1. emiago commented on Dec 3, 2024

   

   emiago

   on Dec 3, 2024

   Owner

   More actions

   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
2. p-akshay commented on Dec 4, 2024

   

   p-akshay

   on Dec 4, 2024

   Last edited by p-akshay

   Author

   More actions

   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
3. emiago commented on Dec 4, 2024

   

   emiago

   on Dec 4, 2024

   Last edited by emiago

   Owner

   More actions

   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.
4. yangjuncode commented on Dec 20, 2024

   

   yangjuncode

   on Dec 20, 2024

   More actions

   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?
5. emiago commented on Dec 22, 2024

   

   emiago

   on Dec 22, 2024

   Owner

   More actions

   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
6. emiago

   added

   criticalCritical issueCritical issue

   breaking change

   on Dec 28, 2024
7. emiago commented on Jun 16, 2025

   

   emiago

   on Jun 16, 2025

   Owner

   More actions

   CSEQ increase is now removed, but also added as option. We handle this in many other wrappers, so no need to be implicit.
8. emiago

   closed this as completedon Jun 16, 2025