[Asterisk-code-review] res pjsip caller id: Anonymize 'From' when caller id present... (asterisk[master])

[George Joseph]

George Joseph has posted comments on this change.

Change subject: res_pjsip_caller_id: Anonymize 'From' when caller id presentation is prohibited
......................................................................


Patch Set 3:

(6 comments)

> (7 comments)
 > 
 > I think that should do it.
 > 
 > Going by the graph only, I'll say this:
 > 
 > - the graph looks good,
 > - the From header is consistent (regardless of the
 > trust_id_outbound option),
 > - you'll be able to tell whether the callerid was probitited by
 > caller name even if there is no PAI/RPID sent,
 > - it's consistent with the RFCs (no sending of the header if there
 > is no trust) and with chan_sip (non-legacy mode),
 > 
 > 

Good because the table was created by observing the packets on the wire with the new code. :)

https://gerrit.asterisk.org/#/c/2294/3/res/res_pjsip.c
File res/res_pjsip.c:

Line 3914: 	id_uri = pjsip_uri_get_uri(id_name_addr->uri);
> You could move this line to before 3925.
I like keeping the assignments together especially when there's no savings by moving them into the conditionals.


https://gerrit.asterisk.org/#/c/2294/3/res/res_pjsip_caller_id.c
File res/res_pjsip_caller_id.c:

Line 497: 			add_privacy_header(tdata, id);
> The add_privacy_header looks buggy in that it would keep "Privacy: none" wh
I'll look at this separately.


Line 649:  * For an initial INVITE request, we may change the From header to appropriately
        :  * reflect the identity information. 
> Updating From-header is not done here anymore.
Done


https://gerrit.asterisk.org/#/c/2294/3/res/res_pjsip_session.c
File res/res_pjsip_session.c:

Line 1139: 	/* Now set up dlg->local.info so pjsip can correctly generate From */
         : 
         : 	id_name_addr = (pjsip_name_addr *) session->inv_session->dlg->local.info->uri;
         : 	id_uri = pjsip_uri_get_uri(id_name_addr);
> Adding an alias/temporary pointer to both "dlg->local.info" and "dlg->pool"
Done


Line 1171: 	}
         : 
         : 	if (restricted &&
> No need to exit the previous if{} here.
Done


Line 1179: 	 * the invite message.  All future message should also have the correct From.
> future message+s
Done


-- 
To view, visit https://gerrit.asterisk.org/2294
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c82a5ca1413c2c00fb62ea95b0ae8e97af54dc9
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Walter Doekes <walter+asterisk at wjd.nu>
Gerrit-HasComments: Yes