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

sipcapture: table name corrupted #566

Closed
seudin opened this issue Apr 14, 2016 · 13 comments
Closed

sipcapture: table name corrupted #566

seudin opened this issue Apr 14, 2016 · 13 comments
Assignees

Comments

@seudin
Copy link
Contributor

seudin commented Apr 14, 2016

In commit c1863d7 added homer5 functionality and table name is built based on timestamp, but corrupted. The added functionality isn't documented too.

Q: why it is mandatory to build table name based on time stamp?

Kind regards,
Seudin

@seudin
Copy link
Contributor Author

seudin commented Apr 14, 2016

correct commit where it's added is 37afdd1

Kind regards,
Seudin

@adubovikov
Copy link
Member

sorry, but I don't really understand why its corrupted. The timestamp as part of table name is a new design that works well, without any issues.
Did you install kamailio 4.4 and check the last config ?

https://github.com/sipcapture/homer-api/blob/master/examples/sipcapture/sipcapture.kamailio

@seudin
Copy link
Contributor Author

seudin commented Apr 15, 2016

Hi,

I'm testing on 4.4.

The 'new design' isn't documented, so I can't figure out how the name will be generated.
When use simple config:

modparam("sipcapture", "db_url", DBURL)
modparam("sipcapture", "capture_on", 1)
modparam("sipcapture", "hep_capture_on", 1)
modparam("sipcapture", "raw_ipip_capture_on", 0)
modparam("sipcapture", "table_name", "sip_capture")

sipcapture uses given table name as format string, but the final result was
'sip_capture.....A'. So no any timestamp in the name. The reason for this could be next two lines:

ntab.len = strftime(strftime_buf, sizeof(strftime_buf), table->s,  &capt_ts);
ntab.s = strftime_buf;

The table->s not necessary ends with '\0', because it's part of Kamailio internal str structure.
Avoid non zero terminated string will work in this case.

Parsing loop in parse_table_names function doesn't add '\0' at the end of table name.
Suggested patch is here.

diff --git a/modules/sipcapture/sipcapture.c b/modules/sipcapture/sipcapture.c
index 41d153e..2bdb4b4 100644
--- a/modules/sipcapture/sipcapture.c
+++ b/modules/sipcapture/sipcapture.c
@@ -436,8 +436,9 @@ int parse_table_names (str table_name, str ** table_names){
        {
                LM_INFO ("INFO: table name:%s\n",p);
                names[i].len = strlen (p);
-               names[i].s =  (char *)pkg_malloc(sizeof(char) *names[i].len);
+               names[i].s =  (char *)pkg_malloc(sizeof(char) *(names[i].len + 1));
                memcpy(names[i].s, p, names[i].len);
+               names[i].s[names[i].len] = '\0';
                i++;
                p = strtok (NULL, "| \t");
        }

BTW, can you explain what 'table->s' will contain (format string required by strftime) using provided example config ?

Thanks,
Seudin

@adubovikov
Copy link
Member

the docs are here: https://github.com/sipcapture/homer/wiki

since H5 we generate table name depends on message type and datestamp:

I have pointed you already to the new kamailio's config:

https://github.com/sipcapture/homer-api/blob/master/examples/sipcapture/sipcapture.kamailio#L814

https://github.com/sipcapture/homer-api/blob/master/examples/sipcapture/sipcapture.kamailio#L849

https://github.com/sipcapture/homer-api/blob/master/examples/sipcapture/sipcapture.kamailio#L855

at the end (L855) you push the generated table name to sip_capture function
and a message will be inserted to the table.

modparam "table_name" is only for H3

Wbr,
Alexandr

On 15 April 2016 at 09:52, Seudin Kasumovic notifications@github.com
wrote:

Hi,

I'm testing on 4.4.

The 'new design' isn't documented, so I can't figure out how the name will
be generated.
When use simple config:

modparam("sipcapture", "db_url", DBURL)
modparam("sipcapture", "capture_on", 1)
modparam("sipcapture", "hep_capture_on", 1)
modparam("sipcapture", "raw_ipip_capture_on", 0)
modparam("sipcapture", "table_name", "sip_capture")

sipcapture uses given table name as format string, but the final result was
'sip_capture.....A'. So no any timestamp in the name. The reason for this
could be next two lines:

ntab.len = strftime(strftime_buf, sizeof(strftime_buf), table->s, &capt_ts);
ntab.s = strftime_buf;

The table->s not necessary ends with '\0', because it's part of Kamailio
internal str structure.
Avoid non zero terminated string will work in this case.

Parsing loop in parse_table_names function doesn't add '\0' at the end of
table name.
Suggested patch is here.

diff --git a/modules/sipcapture/sipcapture.c b/modules/sipcapture/sipcapture.c
index 41d153e..2bdb4b4 100644
--- a/modules/sipcapture/sipcapture.c
+++ b/modules/sipcapture/sipcapture.c
@@ -436,8 +436,9 @@ int parse_table_names (str table_name, str ** table_names){
{
LM_INFO ("INFO: table name:%s\n",p);
names[i].len = strlen (p);

  •           names[i].s =  (char *)pkg_malloc(sizeof(char) *names[i].len);
    
  •           names[i].s =  (char *)pkg_malloc(sizeof(char) *(names[i].len + 1));
            memcpy(names[i].s, p, names[i].len);
    
  •           names[i].s[names[i].len] = '\0';
            i++;
            p = strtok (NULL, "| \t");
    }
    

BTW, can you explain what 'table->s' will contain (format string required
by strftime) using provided example config ?

Thanks,
Seudin


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#566 (comment)

@miconda
Copy link
Member

miconda commented Apr 15, 2016

Is the modparam table_name still needed? Like, is it making possible to run kamailio 4.4 with homer 3?

Or, if it is not longer needed, then it should be removed -- it can be confusing and messing up things.

@seudin
Copy link
Contributor Author

seudin commented Apr 15, 2016

I thought on Kamailio docs. Thanks for links.

So, workaround in my setup is to always call
sip_capture("$var(table_format)") with table format as argument, even don't
need to shard data, to get correct table name.

Kind regards and thanks,
Seudin

On Fri, Apr 15, 2016 at 10:03 AM, Alexandr Dubovikov <
notifications@github.com> wrote:

the docs are here: https://github.com/sipcapture/homer/wiki

since H5 we generate table name depends on message type and datestamp:

I have pointed you already to the new kamailio's config:

https://github.com/sipcapture/homer-api/blob/master/examples/sipcapture/sipcapture.kamailio#L814

https://github.com/sipcapture/homer-api/blob/master/examples/sipcapture/sipcapture.kamailio#L849

https://github.com/sipcapture/homer-api/blob/master/examples/sipcapture/sipcapture.kamailio#L855

at the end (L855) you push the generated table name to sip_capture function
and a message will be inserted to the table.

modparam "table_name" is only for H3

Wbr,
Alexandr

On 15 April 2016 at 09:52, Seudin Kasumovic notifications@github.com
wrote:

Hi,

I'm testing on 4.4.

The 'new design' isn't documented, so I can't figure out how the name
will
be generated.
When use simple config:

modparam("sipcapture", "db_url", DBURL)
modparam("sipcapture", "capture_on", 1)
modparam("sipcapture", "hep_capture_on", 1)
modparam("sipcapture", "raw_ipip_capture_on", 0)
modparam("sipcapture", "table_name", "sip_capture")

sipcapture uses given table name as format string, but the final result
was
'sip_capture.....A'. So no any timestamp in the name. The reason for this
could be next two lines:

ntab.len = strftime(strftime_buf, sizeof(strftime_buf), table->s,
&capt_ts);
ntab.s = strftime_buf;

The table->s not necessary ends with '\0', because it's part of Kamailio
internal str structure.
Avoid non zero terminated string will work in this case.

Parsing loop in parse_table_names function doesn't add '\0' at the end of
table name.
Suggested patch is here.

diff --git a/modules/sipcapture/sipcapture.c
b/modules/sipcapture/sipcapture.c
index 41d153e..2bdb4b4 100644
--- a/modules/sipcapture/sipcapture.c
+++ b/modules/sipcapture/sipcapture.c
@@ -436,8 +436,9 @@ int parse_table_names (str table_name, str **
table_names){
{
LM_INFO ("INFO: table name:%s\n",p);
names[i].len = strlen (p);

  • names[i].s = (char *)pkg_malloc(sizeof(char) *names[i].len);
  • names[i].s = (char *)pkg_malloc(sizeof(char) *(names[i].len + 1));
    memcpy(names[i].s, p, names[i].len);
  • names[i].s[names[i].len] = '\0';
    i++;
    p = strtok (NULL, "| \t");
    }

BTW, can you explain what 'table->s' will contain (format string required
by strftime) using provided example config ?

Thanks,
Seudin


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#566 (comment)


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#566 (comment)

MSC Seudin Kasumovic
Tuzla, Bosnia

@adubovikov
Copy link
Member

@miconda

this is only for back compatibility to H3. We have a lot users who still use it in production. So better keep it until
kamailio 5.0 release

Wbr,
Alexandr

@adubovikov
Copy link
Member

@seudin
yes, please always call the table name param. Since H5 it will be mandatory.

thank you

Wbr,
Alexandr

@miconda
Copy link
Member

miconda commented Apr 20, 2016

Is this sorted out? Does it need a further action (e.g., adding some notes to the docs) or can be closed?

@adubovikov
Copy link
Member

@miconda I will update the docs during this week.

@miconda
Copy link
Member

miconda commented May 6, 2016

Hoping @adubovikov will get back to this soon to close this issue.

@adubovikov
Copy link
Member

sorry guys, a lot stuff... yes, I will close it on this WE...

@adubovikov adubovikov self-assigned this May 9, 2016
@adubovikov
Copy link
Member

ok, the docs are in. I close the ticket, please re-open if anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants