[asterisk-dev] [Code Review] 3074: Fix old regression where config.c unescapes semicolons in embedded values in extensions.conf.

George Joseph reviewboard at asterisk.org
Tue Dec 17 16:19:43 CST 2013



> On Dec. 16, 2013, 7:28 p.m., Matt Jordan wrote:
> > Well, we can't just undo r354657, which is what this patch does. However we choose to fix the regression, we shouldn't do so in a way that just causes another regression by re-opening ASTERISK-17121.
> > 
> > To quote Kinsey from https://reviewboard.asterisk.org/r/1724:
> > 
> > The change here causes that backslash to be removed, but does not create a real escape system in the config parser.  The biggest complication with a real escape system would be breaking existing configs everywhere (parsing \\ as \ and breaking on escaped non-semicolon characters) even though it would be the "right" way to do things.  It also does not confer any benefits because there are no other escaped characters that have special meaning in config files.
> > 
> > I'm not sure we want to go crazy here, but ideally we would have a way to re-write out a '\' when we write out the config.
> 
> George Joseph wrote:
>     Hmmm.  So the client would write a correctly escaped '\;', the 1724 patch would strip the backslash, then something later on would write it back again?  How would 'something' tell the difference between a ';' that was escaped and needs to be re-escaped and a ';' meant to start a comment?
> 
> Tilghman Lesher wrote:
>     Because a semicolon meant to start a comment pushes the comment into a completely different field in the config line structure.

I'm going to discard this patch until I have time to look into it deeper.  


- George


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3074/#review10429
-----------------------------------------------------------


On Dec. 16, 2013, 6:18 p.m., George Joseph wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3074/
> -----------------------------------------------------------
> 
> (Updated Dec. 16, 2013, 6:18 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: 20127
>     https://issues.asterisk.org/jira/browse/20127
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This is one I've been patching locally since the middle of last year.   If you retrieve extensions.conf via ami and write it back again (as the Asterisk GUI does), config.c will UNescape semicolons in values causing the rest of the line to be considered a comment.
> 
> Lines such as 
> PAGING_HEADER = Call-Info: \;answer-after=0
> are being rewritten as 
> PAGING_HEADER = Call-Info: ;answer-after=0
> 
> 'answer-after=0' is now considered a comment and PAGING_HEADER is now just 'Call-Info:'.  Since the change to extensions.conf is permanent, this is really a data corruption.
> 
> This is a regression casued by v354657 of config.c.
> 
> 
> Diffs
> -----
> 
>   branches/12/main/config.c 403992 
> 
> Diff: https://reviewboard.asterisk.org/r/3074/diff/
> 
> 
> Testing
> -------
> 
> This patch has been in my production systems since July 2012.  
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131217/d1c7cf98/attachment.html>


More information about the asterisk-dev mailing list