fix(transaction): non-INVITE client transaction waits Timer K, not Timer D

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()
 	}
 

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))
+		})
+	}
+}