[asterisk-dev] [Code Review] Clean up http.conf parsing

[mjordan]

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



trunk/main/http.c
<https://reviewboard.asterisk.org/r/1460/#comment8704>

    I'm not sure I like this portion moved below the resetting of the http_tls_cfg.  Imagine the following scenario:
    
    1. An existing http session exists where TLS is enabled
    2. The admin edits http.conf, and puts a typo somewhere in there
    3. A reload occurs over the CLI.  On one thread, we perform the http_load, while at the same time, the existing http session performs a request which calls httpstatus_callback
    
    Since the config file is invalid, we would reset the TLS information (potentially at the same time as an existing http session is attempting to use it), and then fail to load the rest of the http configuration information.
    
    In general, I would think we would want something like the following for configuration loading that supports reloading:
    1. Load all information into a temp object
    2. Verify that all information is correct and valid
    3. Lock the configuration objects
    4. Update configuration objects from the temp
    5. Unlock
    



trunk/main/http.c
<https://reviewboard.asterisk.org/r/1460/#comment8705>

    None of this is thread safe.  This is a general problem throughout this load if the server is started - we blow out the previous configuration at the the same time httpstatus_callback (and other things in tcptls) could be accessing it.
    
    We could get around some of those problems using a ref counted object.  We'd still have to lock the portion where we switch the previous ref counted object with a new one, but it'd prevent a lot of weird scenarios that could occur if someone did a reload in the middle of an http request.


- mjordan


On Sept. 23, 2011, 8:41 p.m., Paul Belanger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1460/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2011, 8:41 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Another patch to clean up configuration file parsing.
> 
> 
> Diffs
> -----
> 
>   trunk/main/http.c 337894 
> 
> Diff: https://reviewboard.asterisk.org/r/1460/diff
> 
> 
> Testing
> -------
> 
> local development box
> 
> 
> Thanks,
> 
> Paul
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111020/01a22bf7/attachment.htm>