From 1fe52ea1806b1b76a5c14ff8452c2ae726d714dd Mon Sep 17 00:00:00 2001 From: laconicman Date: Sun, 4 Oct 2026 12:53:10 +0300 Subject: [PATCH] refactor(pjsua): commit the new Contact in update_regc_contact() Pass the candidate Contact into update_regc_contact() and commit it to acc->contact only after the regc has accepted it, so the rollback lives in one place instead of in acc_check_nat_addr() and auto_rereg_timer_cb(). This also drops acc_check_nat_addr()'s prev_contact, which in UNREGISTER mode was captured after destroy_regc() had already cleared acc->contact. Remove the else branch that aliased reg_contact to contact: len always exceeds acc->contact.slen, so it never ran. The OUTBOUND_WANTED store it left dead goes too. Log the new Contact only once the regc has accepted it, and treat &acc->contact as new_contact like NULL. No change on the wire. --- pjsip/src/pjsua-lib/pjsua_acc.c | 229 +++++++++++++++----------------- 1 file changed, 109 insertions(+), 120 deletions(-) diff --git a/pjsip/src/pjsua-lib/pjsua_acc.c b/pjsip/src/pjsua-lib/pjsua_acc.c index 6d6e8f4552..c36e89cac6 100644 --- a/pjsip/src/pjsua-lib/pjsua_acc.c +++ b/pjsip/src/pjsua-lib/pjsua_acc.c @@ -2147,17 +2147,23 @@ PJ_DEF(pj_status_t) pjsua_acc_set_online_status2( pjsua_acc_id acc_id, return PJ_SUCCESS; } -/* Create reg_contact, adding SIP outbound params and other REGISTER specific - * Contact params, i.e: reg_contact_params, reg_contact_uri_params. +/* Create reg_contact from new_contact (NULL: acc->contact), adding SIP + * outbound params and other REGISTER specific Contact params, i.e: + * reg_contact_params, reg_contact_uri_params. * With push, also load it into the regc; pass PJ_FALSE where the regc takes * it from pjsip_regc_init() or regc_tsx_cb()'s param->contact instead. - * Returns PJ_FALSE if the regc refused it, leaving reg_contact as it was. + * Returns PJ_FALSE if the push was refused, leaving the account as it was. + * Otherwise new_contact becomes acc->contact; without push, before the regc + * has seen it. */ -static pj_bool_t update_regc_contact(pjsua_acc *acc, pj_bool_t push) +static pj_bool_t update_regc_contact(pjsua_acc *acc, + const pj_str_t *new_contact, + pj_bool_t push) { pjsua_acc_config *acc_cfg = &acc->cfg; + const pj_str_t *contact = new_contact ? new_contact : &acc->contact; pj_bool_t need_outbound = PJ_FALSE; - pj_str_t prev_contact = acc->reg_contact; + pj_str_t prev_reg_contact = acc->reg_contact; unsigned prev_status = acc->rfc5626_status; const pj_str_t tcp_param = pj_str(";transport=tcp"); const pj_str_t tls_param = pj_str(";transport=tls"); @@ -2173,8 +2179,8 @@ static pj_bool_t update_regc_contact(pjsua_acc *acc, pj_bool_t push) if (acc->rfc5626_status == OUTBOUND_NA) goto done; - if (pj_stristr(&acc->contact, &tcp_param)==NULL && - pj_stristr(&acc->contact, &tls_param)==NULL) + if (pj_stristr(contact, &tcp_param)==NULL && + pj_stristr(contact, &tls_param)==NULL) { /* Currently we can only do SIP outbound for TCP * and TLS. @@ -2190,119 +2196,102 @@ static pj_bool_t update_regc_contact(pjsua_acc *acc, pj_bool_t push) pj_ssize_t len; pj_str_t reg_contact; - acc->rfc5626_status = OUTBOUND_WANTED; - len = acc->contact.slen + + len = contact->slen + acc->cfg.contact_params.slen + acc->cfg.reg_contact_params.slen + acc->cfg.reg_contact_uri_params.slen + (need_outbound? (acc->rfc5626_instprm.slen + acc->rfc5626_regprm.slen): 0) + 5; /* allowance */ - if (len > acc->contact.slen) { - reg_contact.ptr = (char*) pj_pool_alloc(acc->pool, len); + reg_contact.ptr = (char*) pj_pool_alloc(acc->pool, len); - /* Must be NULL terminated: the buffer is parsed below and by - * pjsip_regc, and the parser requires a NULL terminated input. + /* Must be NULL terminated: the buffer is parsed below and by + * pjsip_regc, and the parser requires a NULL terminated input. + */ + pj_strncpy_with_null(®_contact, contact, len); + + /* Contact URI params */ + if (acc->cfg.reg_contact_uri_params.slen) { + pj_pool_t *pool; + pjsip_contact_hdr *contact_hdr; + /* Only ever handed to pjsip_uri_print(), which dispatches + * on the URI's own vptr, so this must not be narrowed to + * pjsip_sip_uri*: a Contact may legitimately hold another + * scheme and no SIP specific field is read here. */ - pj_strncpy_with_null(®_contact, &acc->contact, len); - - /* Contact URI params */ - if (acc->cfg.reg_contact_uri_params.slen) { - pj_pool_t *pool; - pjsip_contact_hdr *contact_hdr; - /* Only ever handed to pjsip_uri_print(), which dispatches - * on the URI's own vptr, so this must not be narrowed to - * pjsip_sip_uri*: a Contact may legitimately hold another - * scheme and no SIP specific field is read here. - */ - pjsip_uri *uri = NULL; - pj_str_t uri_param = acc->cfg.reg_contact_uri_params; - const pj_str_t STR_CONTACT = { "Contact", 7 }; - char tmp_uri[PJSIP_MAX_URL_SIZE]; - pj_ssize_t tmp_len = -1; - - /* Get the URI string */ - pool = pjsua_pool_create("tmp", 512, 512); - contact_hdr = (pjsip_contact_hdr*) - pjsip_parse_hdr(pool, &STR_CONTACT, - reg_contact.ptr, - reg_contact.slen, NULL); - if (contact_hdr && contact_hdr->uri) - uri = (pjsip_uri*)pjsip_uri_get_uri(contact_hdr->uri); - if (uri) { - tmp_len = pjsip_uri_print(PJSIP_URI_IN_CONTACT_HDR, - uri, tmp_uri, - sizeof(tmp_uri)); - } - pj_pool_release(pool); - - if (tmp_len > 0) { - pj_ssize_t new_len; - - /* Regenerate Contact */ - new_len = pj_ansi_snprintf( - reg_contact.ptr, len, - "<%.*s%.*s>%.*s", - (int)tmp_len, tmp_uri, - (int)uri_param.slen, uri_param.ptr, - (int)acc->cfg.contact_params.slen, - acc->cfg.contact_params.ptr); - if (new_len > 0 && new_len < len) - reg_contact.slen = new_len; - else - tmp_len = -1; - } - - if (tmp_len <= 0) { - /* Leave reg_contact as the plain copy of acc->contact */ - PJ_LOG(1,(THIS_FILE, - "Unable to apply registration Contact URI " - "params of acc %d to Contact '%.*s'", - acc->index, (int)acc->contact.slen, - acc->contact.ptr)); - pj_strncpy_with_null(®_contact, &acc->contact, len); - } + pjsip_uri *uri = NULL; + pj_str_t uri_param = acc->cfg.reg_contact_uri_params; + const pj_str_t STR_CONTACT = { "Contact", 7 }; + char tmp_uri[PJSIP_MAX_URL_SIZE]; + pj_ssize_t tmp_len = -1; + + /* Get the URI string */ + pool = pjsua_pool_create("tmp", 512, 512); + contact_hdr = (pjsip_contact_hdr*) + pjsip_parse_hdr(pool, &STR_CONTACT, + reg_contact.ptr, + reg_contact.slen, NULL); + if (contact_hdr && contact_hdr->uri) + uri = (pjsip_uri*)pjsip_uri_get_uri(contact_hdr->uri); + if (uri) { + tmp_len = pjsip_uri_print(PJSIP_URI_IN_CONTACT_HDR, + uri, tmp_uri, + sizeof(tmp_uri)); } + pj_pool_release(pool); - /* Outbound */ - if (need_outbound) { - acc->rfc5626_status = OUTBOUND_WANTED; + if (tmp_len > 0) { + pj_ssize_t new_len; + + /* Regenerate Contact */ + new_len = pj_ansi_snprintf( + reg_contact.ptr, len, + "<%.*s%.*s>%.*s", + (int)tmp_len, tmp_uri, + (int)uri_param.slen, uri_param.ptr, + (int)acc->cfg.contact_params.slen, + acc->cfg.contact_params.ptr); + if (new_len > 0 && new_len < len) + reg_contact.slen = new_len; + else + tmp_len = -1; + } - /* Need to use outbound, append the contact with - * +sip.instance and reg-id parameters. - */ - pj_strcat(®_contact, &acc->rfc5626_regprm); - pj_strcat(®_contact, &acc->rfc5626_instprm); - } else { - acc->rfc5626_status = OUTBOUND_NA; + if (tmp_len <= 0) { + /* Leave reg_contact as the plain copy of the Contact */ + PJ_LOG(1,(THIS_FILE, + "Unable to apply registration Contact URI " + "params of acc %d to Contact '%.*s'", + acc->index, (int)contact->slen, + contact->ptr)); + pj_strncpy_with_null(®_contact, contact, len); } + } - /* Contact params */ - pj_strcat(®_contact, &acc->cfg.reg_contact_params); + /* Outbound */ + if (need_outbound) { + acc->rfc5626_status = OUTBOUND_WANTED; - /* pj_strcat() does not NULL terminate, and the buffer is later - * parsed by pjsip_regc. The 'len' allowance guarantees room. + /* Need to use outbound, append the contact with + * +sip.instance and reg-id parameters. */ - pj_assert(reg_contact.slen < len); - if (reg_contact.slen < len) - reg_contact.ptr[reg_contact.slen] = '\0'; + pj_strcat(®_contact, &acc->rfc5626_regprm); + pj_strcat(®_contact, &acc->rfc5626_instprm); + } else { + acc->rfc5626_status = OUTBOUND_NA; + } - acc->reg_contact = reg_contact; + /* Contact params */ + pj_strcat(®_contact, &acc->cfg.reg_contact_params); - PJ_LOG(4,(THIS_FILE, - "Contact for acc %d updated: %.*s", - acc->index, - (int)acc->reg_contact.slen, - acc->reg_contact.ptr)); + /* pj_strcat() does not NULL terminate, and the buffer is later + * parsed by pjsip_regc. The 'len' allowance guarantees room. + */ + pj_assert(reg_contact.slen < len); + if (reg_contact.slen < len) + reg_contact.ptr[reg_contact.slen] = '\0'; - } else { - /* Outbound is not needed/wanted for the account and there's - * no custom registration Contact params. acc->reg_contact - * is set to the same as acc->contact. - */ - acc->reg_contact = acc->contact; - acc->rfc5626_status = OUTBOUND_NA; - } + acc->reg_contact = reg_contact; } if (push && acc->regc) { @@ -2310,14 +2299,22 @@ static pj_bool_t update_regc_contact(pjsua_acc *acc, pj_bool_t push) status = pjsip_regc_update_contact(acc->regc, 1, &acc->reg_contact); if (status != PJ_SUCCESS) { - /* set_contact() validates first: the regc kept prev_contact */ + /* set_contact() validates first: the regc kept prev_reg_contact */ pjsua_perror(THIS_FILE, "Failed updating registration Contact", status); - acc->reg_contact = prev_contact; + acc->reg_contact = prev_reg_contact; acc->rfc5626_status = prev_status; return PJ_FALSE; } } + if (new_contact && new_contact != &acc->contact) + pj_strdup_with_null(acc->pool, &acc->contact, new_contact); + + PJ_LOG(4,(THIS_FILE, + "Contact for acc %d updated: %.*s", + acc->index, + (int)acc->reg_contact.slen, + acc->reg_contact.ptr)); return PJ_TRUE; } @@ -2577,7 +2574,7 @@ static pj_bool_t acc_check_nat_addr(pjsua_acc *acc, pj_bool_t secure; pjsip_contact_hdr *new_hdr; pj_in6_addr v6_addr; - pj_str_t prev_contact; + pj_str_t new_contact; secure = pjsip_transport_get_flag_from_type(tp->key.type) & PJSIP_TRANSPORT_SECURE; @@ -2663,13 +2660,10 @@ static pj_bool_t acc_check_nat_addr(pjsua_acc *acc, destroy_regc(acc, PJ_TRUE); } - prev_contact = acc->contact; - pj_strdup2_with_null(acc->pool, &acc->contact, tmp); - - if (!update_regc_contact(acc, contact_rewrite_method == - PJSUA_CONTACT_REWRITE_NO_UNREG)) + if (!update_regc_contact(acc, pj_cstr(&new_contact, tmp), + contact_rewrite_method == + PJSUA_CONTACT_REWRITE_NO_UNREG)) { - acc->contact = prev_contact; pj_pool_release(pool); return PJ_FALSE; } @@ -3024,7 +3018,7 @@ static void update_rfc5626_status(pjsua_acc *acc, pjsip_rx_data *rdata) on_return: /* Unconfirmed outbound is not in use (RFC 5626 section 6): drop reg-id */ if (was_outbound && acc->rfc5626_status == OUTBOUND_NA) - update_regc_contact(acc, PJ_TRUE); + update_regc_contact(acc, NULL, PJ_TRUE); PJ_LOG(4,(THIS_FILE, "SIP outbound status for acc %d is %s", acc->index, (acc->rfc5626_status==OUTBOUND_ACTIVE? "active": "not active"))); @@ -3494,8 +3488,7 @@ static pj_status_t pjsua_regc_init(int acc_id) goto on_return; } - pj_strdup_with_null(acc->pool, &acc->contact, &tmp_contact); - update_regc_contact(acc, PJ_FALSE); + update_regc_contact(acc, &tmp_contact, PJ_FALSE); } status = pjsip_regc_init( acc->regc, @@ -5435,12 +5428,8 @@ static void auto_rereg_timer_cb(pj_timer_heap_t *th, pj_timer_entry *te) goto on_return; } } else { - /* A new buffer, so the old Contact survives a refusal */ - pj_str_t prev_contact = acc->contact; - - pj_strdup_with_null(acc->pool, &acc->contact, &tmp_contact); - if (!update_regc_contact(acc, PJ_TRUE)) - acc->contact = prev_contact; + /* On refusal, re-register with the Contact the regc kept */ + update_regc_contact(acc, &tmp_contact, PJ_TRUE); } } pj_pool_release(pool);