[Home]

Summary:ASTERISK-16535: [regression] After 1.6.2.8 the #include directive does not work if missing closing double quote
Reporter:Michael Cramer (micc)Labels:
Date Opened:2010-08-10 00:19:09Date Closed:2015-03-27 15:56:10
Priority:MinorRegression?Yes
Status:Closed/CompleteComponents:Core/Configuration
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:As soon as I started 1.6.2.10 I noticed my dialplan reload was not a valid command and a fraction of my dialplan existed. Dialplan show ? only showed some of the contexts in my extensions.conf. I use a lot of includes and I have a lot of lines that are commented out spacers of dashes and underscores like
;--------------------------------------------------------------------
#include "mydir/macros.conf"
;____________________________________________________________________

I also have a lot of nested include files. I had to down grade to 1.6.2.8 which worked fine with the same files. I tried a second time with a fresh 1.6.2.10 source tree with the same results.
Comments:By: Michael Cramer (micc) 2010-08-10 01:03:08

Ok, so the problem was a malformed #include "without/an/endquote
So in previous versions of asterisk it was forgiving of not having the end quote, but the new code requires it or it thinks the whole thing is the file name. This could really screw over some people if they upgrade and have the same problem. So if you want to make it forgiving like previous versions you can do this on line 1066
-                         }
+                         } else {
+                                cur++; // be forgiving as previous versions.
+                         }


Its right after this code:
                       if (*(c + strlen(c) - 1) == quote_char) {
                               cur++;
                               *(c + strlen(c) - 1) = '\0';
                       }

Sorry for it not being an official patch. I haven't done one of those before.
By: Michael Cramer (micc) 2010-08-10 20:11:37

I'd like some feedback on whether it would be better to be strict here or let id behave the same as previous versions so people's dialplans don't go to hell when they upgrade. I'm glad it was strict in one sense, that I was able to fix it, but if a call had came in during that time I would have lost a call, and lucky I had a 1.6.2.8 already built and ready for a make install to roll back to.

I think the best of both worlds is to allow it to load the file, but log an error saying that the include line is missing the ending quote. This log message would go in the new else block.
By: Leif Madsen (lmadsen) 2010-08-11 12:36:09

For a proper submission just create the patch as a text file and attach it to this issue.
By: Leif Madsen (lmadsen) 2010-08-11 12:36:39

Additionally, your question appears to be a policy decision, thus you need to ask it on the asterisk-dev mailing list so the community can chime in about how best to handle this situation. Thanks!
By: Leif Madsen (lmadsen) 2010-08-12 14:54:25

I think the way to really fix this is to add a WARNING message that says there is a missing closing quote, but jtodd does not think "fixing" this by having the behaviour that you don't need a closing quote is correct.
By: Michael Cramer (micc) 2010-08-12 17:01:30

I agree with that. As long as its going to be obvious enough as soon as someone starts asterisk. If its starting in the background though they might not see it until its reloaded, which doesn't seem to be available in this case for some reason, maybe theres another bug here as well. Someone would have to look in the asterisk log file messages to see the warning. Which still kind of makes me think it might be nice to have a fall back behavior. It doesn't seem to cause any other problems, but I'm ok with just having a warning and also a big note somewhere in a readme or release notes about this change so people don't get screwed.
By: Matt Jordan (mjordan) 2015-03-27 15:56:01.933-0500

This is actually already fixed (issuing an ERROR message, at least).

If my {{extensions.conf}} contains the following:

{noformat}
# include "foo.conf
{noformat}

Asterisk will spit out the following ERROR:

{noformat}
*CLI> [Mar 27 15:51:03] ERROR[29059]: config.c:1549 process_text_line: The file '"foo.conf' was listed as a #include but it does not exist.
{noformat}

This was added (or has a last known revision of) in r135717.