diff --git a/sip/transaction_client_tx.go b/sip/transaction_client_tx.go index bf9b61a..0c05d8c 100644 --- a/sip/transaction_client_tx.go +++ b/sip/transaction_client_tx.go @@ -59,8 +59,18 @@ func (tx *ClientTx) Init() error { tx.timer_a = time.AfterFunc(tx.timer_a_time, func() { tx.spinFsm(client_input_timer_a) }) - // Timer D is set to 32 seconds for unreliable transports - tx.timer_d_time = Timer_D + // How long the transaction stays in Completed absorbing retransmitted + // final responses. RFC 3261 17.1.1.2 gives an INVITE client transaction + // Timer D (32 seconds for unreliable transports); 17.1.2.2 gives a + // non-INVITE one Timer K (T4). Using Timer D for both keeps every + // OPTIONS, BYE and INFO transaction in the layer six times longer than + // the RFC asks for, which shows as transaction table growth on a proxy + // or B2BUA that sends keep-alive OPTIONS to many peers. + if tx.origin.IsInvite() { + tx.timer_d_time = Timer_D + } else { + tx.timer_d_time = Timer_K + } tx.mu.Unlock() } diff --git a/sip/transaction_client_tx_test.go b/sip/transaction_client_tx_test.go index 5ce31d4..eae0432 100644 --- a/sip/transaction_client_tx_test.go +++ b/sip/transaction_client_tx_test.go @@ -129,3 +129,56 @@ func TestClientTransactionFSM(t *testing.T) { require.NoError(t, compareFunctions(tx.currentFsmState(), tx.inviteStateAccepted)) }) } + +// A client transaction lingers in Completed to absorb retransmitted final +// responses, and RFC 3261 gives the two transaction kinds different timers for +// that wait: 17.1.1.2 gives an INVITE one Timer D (32 seconds on unreliable +// transports), 17.1.2.2 gives a non-INVITE one Timer K (T4). Using Timer D for +// both keeps every OPTIONS, BYE and INFO in the transaction layer six times +// longer than needed. +// +// On a reliable transport both waits are zero, and this checks that too: the +// timer is picked in the unreliable branch only, and a change there must not +// start arming a timer over TCP. +func TestClientTxCompletedWaitPerMethod(t *testing.T) { + SetTimers(1*time.Millisecond, 1*time.Millisecond, 1*time.Millisecond) + + completedWait := func(t *testing.T, req *Request) time.Duration { + t.Helper() + req.raddr = Addr{IP: net.ParseIP("127.0.0.99"), Port: 5060} + conn := &UDPConnection{ + PacketConn: &fakes.UDPConn{ + Reader: bytes.NewBuffer([]byte{}), + Writers: map[string]io.Writer{"127.0.0.99:5060": bytes.NewBuffer([]byte{})}, + }, + } + tx := NewClientTx("timer-k", req, conn, slog.Default()) + require.NoError(t, tx.Init()) + tx.mu.Lock() + defer tx.mu.Unlock() + return tx.timer_d_time + } + + invite := func(transport string) *Request { + req, _, _ := testCreateInvite(t, "sip:127.0.0.99:5060", transport, "127.0.0.2:5060") + return req + } + options := func(transport string) *Request { + return testCreateRequest(t, "OPTIONS", "sip:127.0.0.99:5060", transport, "127.0.0.2:5060") + } + + for _, tc := range []struct { + name string + req *Request + want time.Duration + }{ + {"INVITE over UDP waits Timer D", invite("udp"), Timer_D}, + {"OPTIONS over UDP waits Timer K", options("udp"), Timer_K}, + {"INVITE over TCP does not wait", invite("tcp"), 0}, + {"OPTIONS over TCP does not wait", options("tcp"), 0}, + } { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.want, completedWait(t, tc.req)) + }) + } +}