[Home]

Summary:ASTERISK-21765: [patch] - FILE function's length argument counts from beginning of file rather than the offset
Reporter:John Zhong (johnz)Labels:
Date Opened:2013-05-07 16:38:36Date Closed:2015-03-16 12:01:19
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Functions/func_dialplan
Versions:11.2.1 11.3.0 Frequency of
Occurrence		Constant
Related
Issues:
is related toASTERISK-21797 FILE function reads inconsistently
Environment:AsteriskNow64 3.0.0 Asterisk 11.2.1 and Asterisk 11.3.0Attachments:( 0) file_read_390821.patch
Description:AsteriskNow64 3.0.0 running under VMWare workstation.

There are two issues have been found while I am testing our HiTPM with AsteriskNow64 3.0.0 integration.

Reproduce steps:
 1) Install netcat
 2) Create two test files as:
    echo 0123456789 >/tmp/0123456789.txt
    base64 -w 0 /var/lib/asterisk/sounds/en/vm-login.ulaw > /tmp/vm-login.ulaw.b64
 3) Run: ifconfig;nc -l 4573
 4) Add a peer for SIP client into sip.conf
 5) Create an extension for an AGI(agi://[the_ip_address_of_nc_listen_on])
 6) Call the AGI extension
 7) If setup is correct, the nc windows should have AGI headers
 8) Send following commands by nc:
{noformat}
    GET VARIABLE FILE("/tmp/0123456789.txt,2,5")

    and

    GET VARIABLE FILE("/tmp/vm-login.ulaw.b64,4080,5100")
{noformat}
 9) Verify the result:
{noformat}
    GET VARIABLE FILE("/tmp/0123456789.txt,2,5")
    200 result=1 (234)
{noformat}
    Base on previous version Asterisk and current document, it should
    return (23456), the first argument is offset and the second argument
    should be length, but it is the 1 before the end position now.

[Edit by Rusty Newton - removed secondary issue as you shouldn't file two bugs in one report. Filing a new issue report.]
Comments:By: Rusty Newton (rnewton) 2013-05-20 18:37:47.690-0500

Reproduced. Just used FILE on some test files via dialplan.

The documentation (core show function FILE) doesn't match the current behavior.

In general the examples of the length argument don't match actual behavior, but here is one example:

{noformat}
   ;reads from the 11th to 20th byte in the file (i.e. skip the first
   10, then read 10 bytes).
   Set(foo=${FILE(/tmp/test.txt,10,10)})
Read mode (line):
{noformat}

Lines like the above do not represent the behavior. In my test this would return ''

To vary on that example:

For a file with the contents "0123456789"

   using the FILE arguments ",9,9" would result in ''
   using the FILE arguments ",10,9" results in
{noformat}
   '
   3456789
   9
   '
{noformat}
The newline was included in the return value.

It's at least been this way since 1.8.4 ( a random older version I had on hand)


Confirmed. Though I'm guessing the documentation has just always been wrong and hasn't been fixed. The reading consistency issue should be a bug and I've created ASTERISK-21797 for it.

I also happen to have 1.8.15 on hand, the behavior is the same there.
{quote}
Base on previous version Asterisk
{quote}
What older version behaved differently for you?



By: Di-Shi Sun (homesick) 2013-06-07 01:37:43.517-0500

The doc should be fine. But the implementation has problems.

funcs/func_env.c
{code}
if (off_i + toappend > offset + length) {
   toappend = length - off_i;
}
{code}

should be

{code}
if (off_i + toappend > offset + length) {
   toappend = offset + length - off_i;
}
{code}

It should fix the offset and length issue.
By: Di-Shi Sun (homesick) 2013-06-07 02:28:47.658-0500

This patch should fix FILE function offset and length issue.
By: Corey Farrell (coreyfarrell) 2015-03-19 11:29:55.167-0500

[~mjordan]: Looks like this patch broke the unit test: {{/funcs/func_env/func_file}}.
By: Matt Jordan (mjordan) 2015-03-19 11:41:27.436-0500

Yeah, I'm going to dig into it today and figure out why.
By: Matt Jordan (mjordan) 2015-03-19 14:07:44.957-0500

So, as it turns out, this patch is not correct. The unit tests caught this with the "no length" tests:

{code}
       /* No length */
       { "123456789", "-5", "56789" },
       { "123456789", "4", "56789" },
{code}

When used with the patch, the length in the above scenarios is calculated as 9 - that is, {{4 + 9 - 4}}. That is because the length is the entire length of the buffer when unspecified - and as a result, we end up reading more than the 5 bytes available (which is what was previously calculated).

However, that doesn't mean the calculation is incorrect in the patch *when* it does not exceed the max length of the buffer. When doing mid-string searches - such as {{2,5}} - the calculation of {{length - off_i}} will incorrectly extract only 3 bytes, as opposed to starting at byte 2 and continuing for 5 bytes.

One easy was to fix this is to just watch to see if we have exceeded the total length of the file ({{flength}}) and, if so, only read up to that point. That can be done by using:

{{MIN(offset + length - off_i, flength - off_i)}}

I've added a few more additions to the unit test to cover the cases outlined by the issue.