[Asterisk-code-review] chan sip: Fix From header truncation for extremely long CALL... (asterisk[11])

[Walter Doekes]

Walter Doekes has posted comments on this change.

Change subject: chan_sip: Fix From header truncation for extremely long CALLERID(name).
......................................................................


Patch Set 1:

(5 comments)

Thanks for the quick review :)

https://gerrit.asterisk.org/#/c/1269/1/channels/chan_sip.c
File channels/chan_sip.c:

Line 13921: 		ast_log(LOG_ERROR, "The From header was truncated. This call setup will fail.\n");
> Put the p->callid in the message so you have some tracing of which call wil
fixed

    -               ast_log(LOG_ERROR, "The From header was truncated. This call setup will fail.\n");
    +               ast_log(LOG_ERROR, "The From header was truncated in call '%s'. This call setup will fail.\n", p->callid);


Line 13928: 		if (left >= 0) {
> Should just be if (left > 0)
A matter of preference. Some devices treat the empty string differently than void. So if the user wanted to supply something, he still does, even though it is truncated to 0 bytes.

But I don't care enough to not change it.

fixed

    -               if (left >= 0) {
    +               if (left > 0) {


Line 13932: 				left = strlen(n);
> Remove this line.  It is in the wrong place.
Drat. You're right. For non-pedantic mode, this would've broken everything.

Thanks for catching that!


Line 13934: 			memmove(from + left + 3, from, written + 1);
> if (strlen(n) < left) {
fixed

    -				left = strlen(n);
     			}
    -			memmove(from + left + 3, from, written + 1);
    +			name_len = strlen(n);
    +			if (left < name_len) {
    +				name_len = left;
    +			}
    +			memmove(from + name_len + 3, from, written + 1);
     			from[0] = '"';
    -			memcpy(from + 1, n, left);
    -			from[left + 1] = '"';
    -			from[left + 2] = ' ';
    +			memcpy(from + 1, n, name_len);
    +			from[name_len + 1] = '"';
    +			from[name_len + 2] = ' ';


Line 13998: 		ast_log(LOG_ERROR, "The To header was truncated. This call setup will fail.\n");
> Put the p->callid in the message so you have some tracing of which call wil
fixed

    -               ast_log(LOG_ERROR, "The To header was truncated. This call setup will fail.\n");
    +               ast_log(LOG_ERROR, "The To header was truncated in call '%s'. This call setup will fail.\n", p->callid);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I33632baf024f01b6a00f8c7f35c91e5f68c40260
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Walter Doekes <walter+asterisk at wjd.nu>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Walter Doekes <walter+asterisk at wjd.nu>
Gerrit-HasComments: Yes