fix(sip_client): keep the proxy path in responses and ACKs

components/sip_client/sip_client.cpp

@@ -1,4 +1,5 @@
 #include "sip_client.h"
+#include <algorithm>
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
@@ -19,26 +20,6 @@ namespace sip_client {
 static const char *const TAG = "sip_client";
 static const char *const USER_AGENT = "ESPHome-sip_client";
 
-static std::string trim(const std::string &s) {
-  size_t b = s.find_first_not_of(" \t\r\n");
-  if (b == std::string::npos) return "";
-  size_t e = s.find_last_not_of(" \t\r\n");
-  return s.substr(b, e - b + 1);
-}
-
-static std::string extract_angle_uri(const std::string &value) {
-  size_t lt = value.find('<');
-  size_t gt = value.find('>');
-  if (lt != std::string::npos && gt != std::string::npos && gt > lt) {
-    return value.substr(lt + 1, gt - lt - 1);
-  }
-  std::string stripped = trim(value);
-  if (stripped.rfind("sip", 0) == 0) {
-    return stripped;
-  }
-  return "";
-}
-
 // Render an IPv4 sockaddr to dotted-quad without depending on inet_ntop.
 static std::string sockaddr_ip(const struct sockaddr_storage &ss, uint16_t *port) {
   if (ss.ss_family != AF_INET) return "";
@@ -307,6 +288,8 @@ void SipClient::call(const std::string &number) {
                    ">;tag=" + this->d_local_tag_;
   this->d_remote_ = "<sip:" + number + "@" + this->domain_ + ">";
   this->d_remote_target_ = "sip:" + number + "@" + this->domain_;
+  this->d_invite_uri_ = this->d_remote_target_;
+  this->dialog_routes_.clear();
   this->send_raw_(this->build_invite_());
   this->set_state_(SIP_INVITING);
   ESP_LOGI(TAG, "Calling %s", number.c_str());
@@ -351,7 +334,7 @@ std::string SipClient::local_sdp_(bool answer) {
 std::string SipClient::build_invite_() {
   std::string sdp = this->local_sdp_();
   std::string msg;
-  msg += "INVITE " + this->d_remote_target_ + " SIP/2.0\r\n";
+  msg += "INVITE " + this->d_invite_uri_ + " SIP/2.0\r\n";
   msg += "Via: SIP/2.0/UDP " + this->local_ip_ + ":" + std::to_string(this->local_port_) +
          ";branch=" + this->d_branch_ + ";rport\r\n";
   msg += "Max-Forwards: 70\r\n";
@@ -369,10 +352,31 @@ std::string SipClient::build_invite_() {
 
 std::string SipClient::build_ack_(const SipMessage &resp) {
   std::string to = resp.header("To");
-  std::string contact = resp.header("Contact");
-  std::string target = extract_angle_uri(contact);
-  if (target.empty()) {
-    target = this->d_remote_target_;
+  bool success = resp.status_code >= 200 && resp.status_code < 300;
+
+  // RFC 3261 §17.1.1.3: the ACK for a 3xx-6xx belongs to the INVITE
+  // transaction — same Request-URI and same top Via branch — or the server
+  // keeps retransmitting the failure (seen with 3CX after a 407). The ACK
+  // for a 2xx is its own transaction: new branch, sent to that response's
+  // Contact through that response's route set — not the stored dialog's,
+  // so a forked 2xx from another leg is acknowledged along its own path.
+  std::string target;
+  std::string route_block;
+  std::string branch;
+  if (success) {
+    target = extract_angle_uri(resp.header("Contact"));
+    if (target.empty()) target = this->d_remote_target_;
+    std::vector<std::string> routes = split_header_values(resp.header("Record-Route"));
+    std::reverse(routes.begin(), routes.end());
+    apply_route_set(routes, target, route_block);
+    branch = gen_branch();
+  } else {
+    target = this->d_invite_uri_.empty() ? this->d_remote_target_ : this->d_invite_uri_;
+    // Take the branch from the response itself so a late retransmission of
+    // an old transaction (e.g. the 407 while the authenticated INVITE is
+    // already in flight) is acknowledged with its own branch.
+    branch = via_branch(resp.header("Via"));
+    if (branch.empty()) branch = this->d_branch_;
   }
 
   uint32_t cseq = (uint32_t) std::atoi(resp.header("CSeq").c_str());
@@ -383,8 +387,9 @@ std::string SipClient::build_ack_(const SipMessage &resp) {
   std::string msg;
   msg += "ACK " + target + " SIP/2.0\r\n";
   msg += "Via: SIP/2.0/UDP " + this->local_ip_ + ":" + std::to_string(this->local_port_) +
-         ";branch=" + gen_branch() + ";rport\r\n";
+         ";branch=" + branch + ";rport\r\n";
   msg += "Max-Forwards: 70\r\n";
+  msg += route_block;
   msg += "From: " + this->d_local_ + "\r\n";
   msg += "To: " + (to.empty() ? this->d_remote_ : to) + "\r\n";
   msg += "Call-ID: " + this->d_call_id_ + "\r\n";
@@ -451,13 +456,17 @@ void SipClient::handle_invite_response_(const SipMessage &m, const std::string &
       return;
     }
 
-    // Capture remote tag and target, parse SDP, ACK, start media.
+    // Capture remote tag, target and route set, parse SDP, ACK, start media.
     std::string to = m.header("To");
     if (!to.empty()) this->d_remote_ = to;
     std::string contact = m.header("Contact");
     std::string target = extract_angle_uri(contact);
     if (!target.empty())
       this->d_remote_target_ = target;
+    // RFC 3261 §12.1.2: the UAC's route set is the Record-Route list of the
+    // 2xx in reverse order.
+    this->dialog_routes_ = split_header_values(m.header("Record-Route"));
+    std::reverse(this->dialog_routes_.begin(), this->dialog_routes_.end());
 
     SdpInfo sdp = parse_sdp(m.body);
     this->remote_rtp_ip_ = sdp.connection_ip.empty() ? this->remote_rtp_ip_ : sdp.connection_ip;
@@ -516,13 +525,23 @@ std::string SipClient::build_response_(const SipMessage &req, int code, const st
   std::string sdp = with_sdp ? this->local_sdp_(/*answer=*/true) : "";
   std::string msg;
   msg += "SIP/2.0 " + std::to_string(code) + " " + reason + "\r\n";
-  msg += "Via: " + req.header("Via") + "\r\n";
+  // Echo every Via, topmost first: a request that crossed a proxy/SBC has
+  // one per hop and the response must retrace them all (RFC 3261 §8.2.6.2).
+  for (const auto &via : split_header_values(req.header("Via")))
+    msg += "Via: " + via + "\r\n";
   msg += "From: " + req.header("From") + "\r\n";
   msg += "To: " + to + "\r\n";
   msg += "Call-ID: " + req.header("Call-ID") + "\r\n";
   msg += "CSeq: " + req.header("CSeq") + "\r\n";
-  if (code >= 200 && code < 300 && req.method == "INVITE")
+  // RFC 3261 §12.1.1: every dialog-establishing response — the early dialog
+  // of a 18x included — must echo the request's Record-Route and carry a
+  // Contact the peer can route in-dialog requests to. Dropping either
+  // strands the proxy outside the dialog and its ACK never reaches us.
+  if (req.method == "INVITE" && code > 100 && code < 300) {
+    for (const auto &route : split_header_values(req.header("Record-Route")))
+      msg += "Record-Route: " + route + "\r\n";
     msg += "Contact: " + this->contact_uri_() + "\r\n";
+  }
   msg += "User-Agent: " + std::string(USER_AGENT) + "\r\n";
   if (with_sdp) {
     msg += "Content-Type: application/sdp\r\n";
@@ -567,6 +586,9 @@ void SipClient::handle_request_(const SipMessage &m, const std::string &raw) {
     this->d_remote_ = m.header("From");
     std::string contact = m.header("Contact");
     this->d_remote_target_ = extract_angle_uri(contact);
+    this->d_invite_uri_.clear();
+    // RFC 3261 §12.1.1: the UAS's route set is the Record-Route list as-is.
+    this->dialog_routes_ = split_header_values(m.header("Record-Route"));
     this->d_cseq_ = std::atoi(m.header("CSeq").c_str());
     this->remote_rtp_ip_ = sdp.connection_ip;
     this->remote_rtp_port_ = sdp.audio_port;
@@ -658,7 +680,7 @@ void SipClient::hangup() {
     case SIP_RINGING_OUT: {
       // CANCEL the pending INVITE (same branch/cseq).
       std::string msg;
-      msg += "CANCEL " + this->d_remote_target_ + " SIP/2.0\r\n";
+      msg += "CANCEL " + this->d_invite_uri_ + " SIP/2.0\r\n";
       msg += "Via: SIP/2.0/UDP " + this->local_ip_ + ":" + std::to_string(this->local_port_) +
              ";branch=" + this->d_branch_ + ";rport\r\n";
       msg += "Max-Forwards: 70\r\n";
@@ -685,11 +707,15 @@ void SipClient::hangup() {
 }
 
 std::string SipClient::build_request_in_dialog_(const std::string &method) {
+  std::string target = this->d_remote_target_;
+  std::string route_block;
+  apply_route_set(this->dialog_routes_, target, route_block);
   std::string msg;
-  msg += method + " " + this->d_remote_target_ + " SIP/2.0\r\n";
+  msg += method + " " + target + " SIP/2.0\r\n";
   msg += "Via: SIP/2.0/UDP " + this->local_ip_ + ":" + std::to_string(this->local_port_) +
          ";branch=" + gen_branch() + ";rport\r\n";
   msg += "Max-Forwards: 70\r\n";
+  msg += route_block;
   // For BYE the From/To orientation follows who originates: we are always local.
   msg += "From: " + this->d_local_ + "\r\n";
   msg += "To: " + this->d_remote_ + "\r\n";

components/sip_client/sip_client.h

@@ -193,6 +193,8 @@ class SipClient : public Component {
   std::string d_local_;        // our From-style header incl. tag
   std::string d_remote_;       // peer header incl. tag
   std::string d_remote_target_;  // request-URI for in-dialog requests
+  std::string d_invite_uri_;     // Request-URI of our INVITE (non-2xx ACK / CANCEL target)
+  std::vector<std::string> dialog_routes_;  // route set (RFC 3261 §12.1), first hop first
   std::string d_local_tag_;
   std::string d_branch_;       // branch of the INVITE transaction
   uint32_t d_cseq_{0};

components/sip_client/sip_message.cpp

@@ -37,6 +37,11 @@ static std::string resolve_header_name(const std::string &name) {
   return name;
 }
 
+// Headers that may legally repeat and whose values form one ordered list.
+static bool is_comma_list_header(const std::string &name) {
+  return name == "via" || name == "record-route" || name == "route" || name == "service-route";
+}
+
 static std::vector<std::string> split_ws(const std::string &s) {
   std::vector<std::string> out;
   size_t i = 0;
@@ -81,6 +86,7 @@ SipMessage parse_sip_message(const std::string &raw) {
 
   size_t pos = 0;
   bool first = true;
+  std::string last_name;
   while (pos < head.size()) {
     size_t eol = head.find("\r\n", pos);
     std::string line = head.substr(pos, eol == std::string::npos ? std::string::npos : eol - pos);
@@ -106,16 +112,135 @@ SipMessage parse_sip_message(const std::string &raw) {
       continue;
     }
 
+    if (!line.empty() && (line[0] == ' ' || line[0] == '\t')) {
+      // Header folding: continuation of the previous header line.
+      if (!last_name.empty()) msg.headers[last_name] += " " + trim(line);
+      continue;
+    }
+
     size_t colon = line.find(':');
     if (colon == std::string::npos) continue;
     std::string name = resolve_header_name(to_lower(trim(line.substr(0, colon))));
     std::string value = trim(line.substr(colon + 1));
-    // Keep the first occurrence (topmost Via, etc.).
-    if (msg.headers.find(name) == msg.headers.end()) msg.headers[name] = value;
+    last_name = name;
+    auto it = msg.headers.find(name);
+    if (it == msg.headers.end()) {
+      msg.headers[name] = value;
+    } else if (is_comma_list_header(name)) {
+      // Repeated rows and one comma-separated row are equivalent on the wire;
+      // keep the proxy path in order (topmost Via first).
+      it->second += ", " + value;
+    }
+    // Any other repeated header keeps its first occurrence.
   }
   return msg;
 }
 
+std::vector<std::string> split_header_values(const std::string &value) {
+  std::vector<std::string> values;
+  size_t start = 0;
+  int angle_depth = 0;
+  bool quoted = false;
+  bool escaped = false;
+  for (size_t i = 0; i < value.size(); i++) {
+    char c = value[i];
+    if (escaped) {
+      escaped = false;
+    } else if (quoted && c == '\\') {
+      escaped = true;
+    } else if (c == '"') {
+      quoted = !quoted;
+    } else if (!quoted && c == '<') {
+      angle_depth++;
+    } else if (!quoted && c == '>' && angle_depth > 0) {
+      angle_depth--;
+    } else if (!quoted && angle_depth == 0 && c == ',') {
+      std::string item = trim(value.substr(start, i - start));
+      if (!item.empty()) values.push_back(item);
+      start = i + 1;
+    }
+  }
+  std::string item = trim(value.substr(start));
+  if (!item.empty()) values.push_back(item);
+  return values;
+}
+
+std::string extract_angle_uri(const std::string &value) {
+  size_t lt = value.find('<');
+  size_t gt = value.find('>');
+  if (lt != std::string::npos && gt != std::string::npos && gt > lt) {
+    return value.substr(lt + 1, gt - lt - 1);
+  }
+  std::string stripped = trim(value);
+  if (stripped.rfind("sip", 0) == 0) {
+    return stripped;
+  }
+  return "";
+}
+
+std::string via_branch(const std::string &via) {
+  // Only the top Via names our transaction; later ones belong to proxies.
+  std::string top = via.substr(0, via.find(','));
+  std::string lower = to_lower(top);
+  size_t p = lower.find(";branch=");
+  if (p == std::string::npos) return "";
+  p += 8;
+  size_t e = top.find_first_of(";, \t\r\n", p);
+  return top.substr(p, e == std::string::npos ? std::string::npos : e - p);
+}
+
+bool is_loose_route(const std::string &route) {
+  std::string uri = extract_angle_uri(route);
+  if (uri.empty()) uri = trim(route);
+  std::string lower = to_lower(uri);
+  // Only URI parameters count (RFC 3261 §16.4). They sit between the
+  // hostport and the "?" that starts the headers part; cut there first so an
+  // '@' inside a header value is not mistaken for the userinfo separator,
+  // then skip the userinfo (";lr" before '@' is not a parameter).
+  size_t end = lower.find('?');
+  if (end == std::string::npos) end = lower.size();
+  size_t p = lower.rfind('@', end);
+  p = (p == std::string::npos) ? 0 : p + 1;
+  // ";lr" must be a whole parameter: ";lr", ";lr=...", ";lr;..." or ";lr?"
+  // count, but ";lrx" must not.
+  while ((p = lower.find(";lr", p)) != std::string::npos && p < end) {
+    size_t after = p + 3;
+    if (after >= end) return true;
+    char c = lower[after];
+    if (c == ';' || c == '=' || c == '?') return true;
+    p = after;
+  }
+  return false;
+}
+
+void apply_route_set(const std::vector<std::string> &routes, std::string &target,
+                     std::string &route_block) {
+  route_block.clear();
+  std::vector<std::string> active;
+  for (const auto &r : routes) {
+    if (!trim(r).empty()) active.push_back(r);
+  }
+  if (active.empty()) return;
+  // One Route header per hop (like the Record-Route echo): some proxies only
+  // read the first value of a comma-joined list.
+  auto emit = [&route_block](const std::vector<std::string> &v) {
+    for (const auto &r : v) route_block += "Route: " + r + "\r\n";
+  };
+  if (is_loose_route(active[0])) {
+    // Loose router: remote target stays in the Request-URI, whole route set
+    // travels as Route headers.
+    emit(active);
+    return;
+  }
+  // Strict router: it takes the Request-URI; the remote target moves to the
+  // tail of the route set so it is not lost.
+  std::vector<std::string> remaining(active.begin() + 1, active.end());
+  if (!target.empty()) remaining.push_back("<" + target + ">");
+  emit(remaining);
+  std::string first = extract_angle_uri(active[0]);
+  target = first.empty() ? active[0] : first;
+}
+
 static void derive_codec_pts_(SdpInfo &info) {
   for (int pt : info.payload_types) {
     std::string name;

components/sip_client/sip_message.h

@@ -16,8 +16,9 @@ struct SipMessage {
   std::string reason;       // response only
 
   // Common headers (raw values, leading/trailing space trimmed). Names are
-  // stored lowercase in `headers`; the convenience fields below mirror the most
-  // used ones.
+  // stored lowercase in `headers`. Repeated list headers (Via, Record-Route,
+  // Route, Service-Route) are joined with ", " in wire order so the full
+  // proxy path survives; every other repeated header keeps its first value.
   std::map<std::string, std::string> headers;
   std::string body;
 
@@ -45,6 +46,27 @@ struct SdpInfo {
 SipMessage parse_sip_message(const std::string &raw);
 SdpInfo parse_sdp(const std::string &body);
 
+// Split a comma-list header value (Via, Record-Route, Route, ...) into its
+// field-values without splitting commas nested in <...> or quoted strings.
+std::vector<std::string> split_header_values(const std::string &value);
+
+// URI inside <...>, or the trimmed value itself when it is a bare sip: URI.
+std::string extract_angle_uri(const std::string &value);
+
+// Branch parameter of the top Via (the transaction the message belongs to).
+std::string via_branch(const std::string &via);
+
+// Whether a Record-Route/Route field-value points at a loose router (;lr).
+bool is_loose_route(const std::string &route);
+
+// Resolve the Request-URI and "Route: ...\r\n" lines for an in-dialog request
+// (RFC 3261 §12.2.1.1). `target` is the remote target on entry; on return it
+// is the Request-URI to use (a strict first hop takes it over and the remote
+// target moves to the end of the route set). `route_block` is one Route
+// header per hop, or empty when there is no route set.
+void apply_route_set(const std::vector<std::string> &routes, std::string &target,
+                     std::string &route_block);
+
 // Extract a quoted-or-token parameter from an auth header value, e.g.
 // auth_param("Digest realm=\"asterisk\", nonce=\"abc\"", "nonce") -> "abc".
 std::string auth_param(const std::string &header_value, const std::string &key);