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.

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

Snom patch: Last_ keywords extension #260

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
3 changes: 2 additions & 1 deletion include/call.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,8 @@ class call : virtual public task, virtual public listener, public virtual socket

char * get_header_field_code(const char * msg, const char * code);
char * get_last_header(const char * name);
char * get_last_request_uri();
char * get_last_tag (const char *name);
char * get_last_request_uri(const char *name);
Copy link
Member

Choose a reason for hiding this comment

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

This should not be called get_last_request_uri but get_last_uri_from or something.

unsigned long hash(const char * msg);

typedef std::map <std::string, int> file_line_map;
Expand Down
35 changes: 35 additions & 0 deletions regress/github-#0260/run
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#!/bin/sh
# This regression test is a part of SIPp.
# Author: Pietro Bertera, Snom Technology AG, 2016

. "`dirname "$0"`/../functions"; init

sippbg -m 1 -sf uas.xml -p 5070 -i 127.0.0.1 \
-timeout 4 -timeout_error -trace_logs \
-log_file log.log >/dev/null 2>&1

job=$!

sippfg -m 1 -sf uac.xml 127.0.0.1:5070 \
-timeout 4 -timeout_error >/dev/null 2>&1

status=$?
wait $job || status=1

if test $status -eq 0; then
if grep -e ^LAST\ From:\ \"Tom\ Jones\"\ \<sip:[email protected]\>\;tag=SIPpTag001$ log.log > /dev/null; then
if grep -e ^LAST\ To:uri:\ sip:[email protected]$ log.log > /dev/null; then
if grep -e ^LAST\ From-tag:\ SIPpTag001$ log.log > /dev/null; then
ok
else
fail "From:param.tag not found"
fi
else
fail "To:uri not found"
fi
else
fail "From header not found"
fi
else
fail "process failure"
fi
Copy link
Member

@wdoekes wdoekes Oct 6, 2016

Choose a reason for hiding this comment

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

This could be done without so much nesting with negative tests instead:

if test $status -ne 0; then
    fail "process failure"
elif ! grep -q '^LAST From: "Tom Jones" <sip:tom\[email protected]>;tag=SIPpTag001$' log.log; then
    fail "From header not found"
...

54 changes: 54 additions & 0 deletions regress/github-#0260/uac.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?xml version="1.0" encoding="ISO-8859-1"?>
<!DOCTYPE scenario SYSTEM "sipp.dtd">
<scenario>
<send retrans="500" start_txn="invite">
<![CDATA[

INVITE sip:[service]@[remote_ip]:[remote_port] SIP/2.0
Via: SIP/2.0/[transport] [local_ip]:[local_port];branch=justafake
From: "Tom Jones" <sip:[email protected]>;tag=SIPpTag00[call_number]
To: "Fromage" <sip:[email protected]>
Call-ID: [call_id]
CSeq: 1 INVITE
Contact: <sip:sipp@[local_ip]:[local_port]>
Content-Length: 0

]]>
</send>

<recv response="200" response_txn="invite" rrs="true"/>

<send retrans="500" ack_txn="invite">
<![CDATA[

ACK [next_url] SIP/2.0
Via: SIP/2.0/[transport] [local_ip]:[local_port];branch=justafake
From: "Tom Jones" <sip:[email protected]>;tag=SIPpTag00[call_number]
To: "Fromage" <sip:[email protected]>[peer_tag_param]
Call-ID: [call_id]
CSeq: 1 ACK
Contact: sip:sipp@[local_ip]:[local_port]
Content-Length: 0

]]>
</send>

<recv request="BYE"/>

<send>
<![CDATA[

SIP/2.0 200 OK
[last_Via:]
[last_From:]
[last_To:]
[last_Call-ID:]
[last_CSeq:]
Contact: <sip:[local_ip]:[local_port];transport=[transport]>
Content-Length: 0

]]>
</send>

<timewait milliseconds="500"/>
</scenario>
51 changes: 51 additions & 0 deletions regress/github-#0260/uas.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?xml version="1.0" encoding="ISO-8859-1"?>
<!DOCTYPE scenario SYSTEM "sipp.dtd">
<scenario>
<recv request="INVITE"/>

<send retrans="500">
<![CDATA[

SIP/2.0 200 OK
Via: [last_Via:value]
From: [last_From:value]
To: [last_To:value];tag=[pid]SIPpTag00[call_number]
Call-ID: [last_Call-ID:value]
CSeq: [last_CSeq:value]
Contact: sip:sipp@[local_ip]:[local_port]
Content-Length: 0

]]>
</send>

<recv request="ACK" rrs="true">
<action>
<log message="LAST Contact: [last_Contact]" />
<log message="LAST Via: [last_Via:value]"/>
<log message="LAST From: [last_From:value]"/>
<log message="LAST To: [last_To:value]"/>
<log message="LAST To:uri: [last_To:uri]"/>
<log message="LAST Contact:uri: [last_Contact:uri]"/>
<log message="LAST From-tag: [last_From:param.tag]"/>
Copy link
Member

Choose a reason for hiding this comment

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

Your "key names" don't correspond to the lookups:

Suggest:
LAST Contact|[last_Contact]
LAST Via:value|[last_Via:value]
etc..

</action>
</recv>

<send retrans="500">
<![CDATA[

BYE [next_url] SIP/2.0
Via: SIP/2.0/[transport] [local_ip]:[local_port];branch=[branch]
From: [last_To:value]
To: [last_From:value]
Call-ID: [call_id]
CSeq: 2 BYE
Contact: sip:sipp@[local_ip]:[local_port]
Content-Length: 0

]]>
</send>

<recv response="200"/>

<timewait milliseconds="500"/>
</scenario>
99 changes: 90 additions & 9 deletions src/call.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -923,33 +923,72 @@ char * call::get_last_header(const char * name)
if (name[len - 1] == ':') {
return get_header(last_recv_msg, name, false);
} else {

char with_colon[MAX_HEADER_LEN];
sprintf(with_colon, "%s:", name);
return get_header(last_recv_msg, with_colon, false);
const char *sep = strrchr(name, ':');

if (!sep){
sprintf(with_colon, "%s:", name);
return get_header(last_recv_msg, with_colon, false);
} else {
if (!strcmp(sep, ":value")) {
snprintf(with_colon, len - 4, "%s", name);
return get_header(last_recv_msg, with_colon, true);
}
if (!strcmp(sep, ":uri")) {
snprintf(with_colon, len - 2, "%s", name);
char * last_header_uri = get_last_request_uri(with_colon);
char * ret;
if (!(ret = (char *)malloc(strlen(last_header_uri + 1)))){
Copy link
Member

Choose a reason for hiding this comment

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

get_header() returns a static buffer (yuck, I know).

You can't start returning malloc'ed memory all of a sudden: memory leaks.

ERROR("call::get_last_header: Cannot allocate uri !\n");
}
sprintf(ret, "%s", last_header_uri);
free(last_header_uri);
return ret;
}
if (!strcmp(sep, ":param.tag")) {
snprintf(with_colon, len - 8, "%s", name);
char * last_tag = get_last_tag(with_colon);
char * ret;
if (!(ret = (char *)malloc(strlen(last_tag + 1)))){
ERROR("call::get_last_header: Cannot allocate param.tag !\n");
}
sprintf(ret, "%s", last_tag);
free(last_tag);
return ret;
}
ERROR("Token %s not valid in %s", sep, name);
}
Copy link
Member

@wdoekes wdoekes Oct 6, 2016

Choose a reason for hiding this comment

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

This function is too big. The code in the if(strcmp..){...} blocks should be delegated to helper methods and duplicate code should be avoided.

For example:

        if (!sep) { /* [last_Header] */
            sprintf(with_colon, "%s:", name);
            return get_header(last_recv_msg, with_colon, false);
        } 

        /* [last_Header:...] */
        sprintf(with_colon, "%.*s", (sep - name + 1), name);
        if (!strcmp(sep, ":")) {
            return get_header(last_recv_msg, with_colon, false);
        }

        char *value = get_header(last_recv_msg, with_colon, true);
        if (!strcmp(sep, ":value")) {
            ; /* done */
        } else if (!strcmp(sep, ":uri")) {
            extract_header_uri(value); /* reuse mem in value (memmove?) */
        } else if (....) {
            ...
        } else {
            ERROR("Token %s not valid in %s", sep, name);
        }

        return value;

}
}

/* Return the last request URI from the To header. On any error returns the
/* Return the last request URI from the header. On any error returns the
* empty string. The caller must free the result. */
char * call::get_last_request_uri()
char * call::get_last_request_uri(const char *name)
{
char * tmp;
char * tmp2;
char * last_request_uri;
char * last_header;
int tmp_len;

char * last_To = get_last_header("To:");
if (!last_To) {
if (!name || !strlen(name)) {
return strdup("");
}

tmp = strchr(last_To, '<');
last_header = get_last_header(name);

if (!last_header) {
return strdup("");
}

tmp = strchr(last_header, '<');
if (!tmp) {
return strdup("");
}
tmp++;

tmp2 = strchr(last_To, '>');
tmp2 = strchr(last_header, '>');
if (!tmp2) {
return strdup("");
}
Expand All @@ -972,6 +1011,48 @@ char * call::get_last_request_uri()
return last_request_uri;
}

/* Return the last tag from the header line. On any error returns the
* empty string. The caller must free the result. */
char * call::get_last_tag(const char *name)
Copy link
Member

Choose a reason for hiding this comment

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

This should be implemented in sip_parser.cpp instead. See get_peer_tag.

{
char * tmp;
char * tmp2;
char * result;
char * last_header;
int tmp_len;

if (!name || !strlen(name)) {
return strdup("");
}

last_header = get_last_header(name);
if (!last_header) {
return strdup("");
}

tmp = strcasestr(last_header, "tag=");
if (!tmp) {
return strdup("");
}

tmp += strlen("tag=");
tmp2 = tmp;

while (*tmp2 && *tmp2 != ';' && *tmp2!='\r' && *tmp2!='\n') tmp2++;

tmp_len = strlen(tmp) - strlen(tmp2);
if (tmp_len < 0) {
return strdup("");
}
if(!(result = (char *) malloc(tmp_len+1))) ERROR("call::get_last_tag: Cannot allocate !\n");
memset(result, 0, sizeof(&result));
if(tmp && (tmp_len > 0)){
strncpy(result, tmp, tmp_len);
}
result[tmp_len] = '\0';
return result;
}

char * call::send_scene(int index, int *send_status, int *len)
{
#define MAX_MSG_NAME_SIZE 30
Expand Down Expand Up @@ -2201,7 +2282,7 @@ char* call::createSendingMessage(SendingMessage *src, int P_index, char *msg_buf
}
break;
case E_Message_Last_Request_URI: {
char * last_request_uri = get_last_request_uri();
char * last_request_uri = get_last_request_uri("To:");
dest += sprintf(dest, "%s", last_request_uri);
free(last_request_uri);
Copy link
Member

@wdoekes wdoekes Oct 8, 2016

Choose a reason for hiding this comment

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

Why not this?

dest += sprintf(dest, "%s", get_last_header("To:uri"));

And drop the entire get_last_header_uri() method.

The get_uri-code could be implemented in sip_parser.cpp instead as get_first_uri(const char *msg, const char *name, const char *shortname) or something, with the same semantics as the other get_* functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why you wrote get_first_uri ? maybe you are thinking about multiple URI in the header ?

break;
Expand Down