Repository navigation
fix(sip_client): keep the proxy path in responses and ACKs - #324
Merged
Merged
Conversation
Calls through a proxy/SBC (3CX) could not complete: the 200 OK to an inbound INVITE echoed only the top Via and dropped Record-Route, so the proxy's ACK never reached us and the call stayed in ANSWERING; and the ACK for a 407 used a fresh Via branch, so the PBX kept retransmitting the 407 while the authenticated INVITE was already in flight. - parse_sip_message: Via / Record-Route / Route / Service-Route rows are joined in wire order instead of keeping the first; header folding. - build_response_: echo every Via; Record-Route + Contact on every dialog-establishing INVITE response (18x and 2xx), RFC 3261 s12.1.1. - build_ack_: non-2xx ACK reuses the response's top Via branch and the INVITE Request-URI (s17.1.1.3); 2xx ACK goes through the route set. - Learn the dialog route set from Record-Route (UAS as-is, UAC reversed) and add Route headers to in-dialog requests (BYE, 2xx ACK) with loose/strict router handling, s12.2.1.1. CANCEL keeps the INVITE URI. - Native tests for the header parsing and routing helpers. Ports the equivalent hass-sip fixes (#22, #38, #40). Co-Authored-By: Claude Opus 5 <[email protected]>
…safe ;lr Review follow-ups on #324: - is_loose_route: only ";lr" after the '@' (hostport + URI params) and before any '?' counts; "<sip:user;lr=on@proxy>" is strict. - 2xx ACK builds its route set from that response's Record-Route instead of the stored dialog's, so a forked 2xx is acknowledged along its own path; the accepted dialog and its retransmissions are unchanged. - Route headers are emitted one per hop, matching the Record-Route echo, for proxies that only read the first value of a comma list. - The route-set logic moves to apply_route_set() in sip_message so it is host-testable; loose/strict/empty and reversed-2xx cases added. Co-Authored-By: Claude Opus 5 <[email protected]>
An '@' inside the URI headers part ("<sip:host;lr?x=a@b>") was taken as
the userinfo separator, hiding the real ";lr" parameter. Find the end of
the parameters first, then the userinfo boundary within it.
Co-Authored-By: Claude Opus 5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #323
Summary
Calls through a proxy/SBC (reported with 3CX) could not complete because
sip_clientdid not preserve the proxy path in responses and ACKs:200 OKto an inbound INVITE echoed only the topViaand droppedRecord-Route, so the proxy's ACK never reached us and the call stayed inANSWERING.407used a freshViabranch, so the PBX kept retransmitting the407while the authenticated INVITE was already in flight.This ports the equivalent hass-sip fixes (eigger/hass-sip#22, eigger/hass-sip#38, eigger/hass-sip#40) rather than applying the patch attached to the issue verbatim, so the same routing rules are used in both projects.
Changes
parse_sip_message:Via/Record-Route/Route/Service-Routerows are joined in wire order instead of keeping only the first occurrence; header folding is supported. New helperssplit_header_values,via_branch,is_loose_route,extract_angle_uri,apply_route_set.build_response_: echoes everyVia; addsRecord-Route+Contactto every dialog-establishing INVITE response (18x and 2xx), RFC 3261 §12.1.1.build_ack_: a non-2xx ACK reuses the response's topViabranch and the INVITE Request-URI (§17.1.1.3), no Route; a 2xx ACK gets a new branch and goes through the route set of that response (so a forked 2xx is acknowledged along its own path).Record-Route(UAS as-is, UAC reversed) and applied to in-dialog requests (BYE) with loose/strict router handling, §12.2.1.1, oneRoute:header per hop. CANCEL keeps the INVITE Request-URI (d_invite_uri_).is_loose_route: only;lrafter@and before?counts —<sip:user;lr=on@proxy>is strict.;lroutside<>is a header parameter, not a URI parameter, and stays strict; misclassifying a loose router as strict is the safe direction (§16.4 strict-router compatibility).tests/native/sip_sdp/test_sip_message.cppcovers header parsing (multi-Via order, Record-Route rows vs comma list, nested commas, folding),via_branch,is_loose_route(incl. userinfo/headers-partlr), andapply_route_set(empty / loose / strict / reversed-2xx).Differences from the patch in #323
Record-Route/Contactare added only to 101–299 INVITE responses (the attached patch also putRecord-Routeon100 Trying, and 18x responses still had noContact).Routeheaders built from the route set; the attached patch did not cover this, which would still break hangup through a proxy that is not a B2BUA.Known limitation (unchanged)
send_raw_always sends on the socket connected to the configuredserver_. If the first Route hop is a different address, BYE / 2xx ACK still go toserver_— correct for a single SBC / outbound-proxy setup, which is what this component targets.Test plan
test_sip_message.cpp,test_parse_sdp.cpp) pass with/W4 /WX(MSVC);run.shruns in CIesphome compile tests/components/sip_client/test.esp32-idf.yamlbuilds with no warnings (xtensa-gcc 14.2)IN_CALL; outbound 407 no longer retransmitted; BYE from both sides🤖 Generated with Claude Code