Transaction key malformed when response contains multiple Via headers combined on the same line.

[joeloptera]

Endpoints drop responses with multiple entries in a single Via header field. This appears to be because the multiple valued Via header is parsed incorrectly by TransactionKey::from_response so the transaction is unmatched.

Example:

#[cfg(test)]
mod tests {
	#[test]
	fn test_generate_transaction_key() {
		let raw_message = format!(
			"{}\r\n",
			&[
				"SIP/2.0 401 Unauthorized",
				"Via: SIP/2.0/UDP 172.22.22.80:5062;received=172.22.22.80;rport=5062;branch=z9hG4bKkwpQ8Rq2RRoi,SIP/2.0/TCP 10.0.13.70:5060;branch=z9hG4bK20ede05124e2b786dbfbe11b",
				"From: <sip:001010000000001@ims.example.com>;tag=e80c1d8c",
				"To: <sip:001010000000001@ims.example.com>;tag=2e518",
				"Call-ID: 9e353fc94f78064f@10.0.13.70",
				"CSeq: 1 REGISTER",
				"Content-Length: 0",
				"",
			]
			.join("\r\n")
		);

		let resp = rsipstack::sip::Response::try_from(raw_message).unwrap();

		let key = rsipstack::transaction::key::TransactionKey::from_response(
			&resp,
			rsipstack::transaction::key::TransactionRole::Client,
		)
		.unwrap();

		assert_eq!(
			key.to_string(),
			"c.REGISTER_1_9e353fc94f78064f@10.0.13.70_e80c1d8c_z9hG4bKkwpQ8Rq2RRoi"
		);
	}
}

This test fails with the following output:

---- tests::test_generate_transaction_key stdout ----

thread 'tests::test_generate_transaction_key' (9238547) panicked at ***:
assertion `left == right` failed
  left: "c.REGISTER_1_9e353fc94f78064f@10.0.13.70_e80c1d8c_z9hG4bKkwpQ8Rq2RRoi,SIP/2.0/TCP 10.0.13.70:5060"
 right: "c.REGISTER_1_9e353fc94f78064f@10.0.13.70_e80c1d8c_z9hG4bKkwpQ8Rq2RRoi"
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Furthermore, it appears that the order of header parameters matters. If rport immediately precedes comma, then typed::Via::parse() fails with an error, but (as in the example above) there is no error if branch is the final header parameter before the comma.

[joeloptera]

FWIW here is the patch I am testing on my local clone of the source:

diff --git a/src/transaction/key.rs b/src/transaction/key.rs
index 4847b86..2fce055 100644
--- a/src/transaction/key.rs
+++ b/src/transaction/key.rs
@@ -1,4 +1,5 @@
 use crate::sip::{
+    headers,
     param::Tag,
     prelude::{HeadersExt, ToTypedHeader},
     typed::Via,
@@ -51,7 +52,15 @@ impl TransactionKey {
     }
 
     pub fn from_response(resp: &Response, role: TransactionRole) -> Result<Self> {
-        let via = resp.via_header()?.typed()?;
+        let raw_via_header_contents = resp.via_header()?.value();
+        // extract the first item in a potentially multi-value Via header
+        let via = headers::Via::new(
+            raw_via_header_contents
+                .split_once(',')
+                .map_or(raw_via_header_contents, |split| split.0),
+        )
+        .typed()?;
+
         let cseq = resp.cseq_header()?;
         let method = cseq.method()?;
         let from_tag = resp

I think this fixes the immediate problem, but it seems likely that it's better remove the assumption that resp.via_header() will return a single valued header.