Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uac_replace_xxx - Remove extra spaces and strip display name on "auto" restore #1742

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

uac_replace_xxx - Remove extra spaces and strip display name on "auto" restore #1742

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions modules/uac/replace.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ int replace_uri( struct sip_msg *msg, str *display, str *uri,
int i;
struct dlg_cell *dlg = NULL;
pv_value_t val;
int ret;
int ret, start_offset, display_len;

/* consistency check! in AUTO mode, do NOT allow URI changing
* in sequential request */
Expand All @@ -270,8 +270,19 @@ int replace_uri( struct sip_msg *msg, str *display, str *uri,
if ( body->display.len) {
LM_DBG("removing display [%.*s]\n",
body->display.len,body->display.s);

start_offset = body->display.s-msg->buf;

//If replacing leave trailing spaces
if (display->len) {
display_len = body->display.len;
} else {
//Removing, strip all characters up to the URI
display_len = ((body->uri.s-msg->buf) - start_offset) - 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the -1 is for leaving a single space between the URI and the display, right ? If so, I'm not sure that the case of an enclosed URI is properly handled. If I'm not wrong, the < and > are not part of the uri field.
Also, the patch does not cover the situation of multiple spaces before the display, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are correct that <> are not part of the uri field.

The -1 actually stops us from trimming the < before the uri. I believe a header with a display name, but a non-enclosed uri is against spec (https://www.ietf.org/rfc/rfc2822.txt), so it shouldn't be a problem?

We remove everything from the start of the display name up to, but not including the <. What is left are the spaces from before the display name and the <> enclosed uri.

The patch does not cover multiple spaces before the display, but it could be modified to include those as well.

Example behaviour (verified against a test):

Original:
From: "Name" <sip:user@domain>;tag=as2ed502bc
Stripped:
From: <sip:user@domain>;tag=as2ed502bc

}

/* build del lump */
l = del_lump( msg, body->display.s-msg->buf, body->display.len, 0);
l = del_lump( msg, start_offset, display_len, 0);
if (l==0) {
LM_ERR("display del lump failed\n");
goto error;
Expand Down