[asterisk-dev] [Code Review] 2811: New pjsip.conf.sample with 100% more pjsip

Mark Michelson reviewboard at asterisk.org
Fri Aug 30 09:47:20 CDT 2013


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


I'd like to apologize if I repeat something brought up by Matt or Paul in my review. I'm trying not to, but I may accidentally.


/trunk/configs/pjsip.conf.sample
<https://reviewboard.asterisk.org/r/2811/#comment18714>

    either:
    
    s/res_pjsip" Then/res_pjsip". Then/
    
    or
    
    s/res_pjsip" Then/res_pjsip", then/
    
    (Note: I put the comma and period outside the quotes, which I know is not standard with American English, but I don't care. I like it better that way)



/trunk/configs/pjsip.conf.sample
<https://reviewboard.asterisk.org/r/2811/#comment18715>

    The sentence here is a bit odd.
    
    First, "allow compromise" should be "allow compromises".
    
    Second, I think the "and possibly worse" should be removed because a compromise of Asterisk's security is the only thing that bad security practices can cause. The compromises themselves can consist of varying degrees of severity (such as crashing the system or running arbitrary code). So either just outright remove the "and possibly worse" or reword the entire sentence to reflect the varying severity of compromises to security.



/trunk/configs/pjsip.conf.sample
<https://reviewboard.asterisk.org/r/2811/#comment18716>

    I'd avoid phrases like "hop on the wiki" in favor of simple (a.k.a. boring) English since Asterisk is used by people with varying fluency in English.



/trunk/configs/pjsip.conf.sample
<https://reviewboard.asterisk.org/r/2811/#comment18717>

    Rather than pointing to acl.conf, point out this wiki page: https://wiki.asterisk.org/wiki/x/uA80AQ
    
    On a separate note, that page needs to be updated to show that pjsip.conf is a consumer of named ACLs.



/trunk/configs/pjsip.conf.sample
<https://reviewboard.asterisk.org/r/2811/#comment18718>

    s/dialplan/extensions/



/trunk/configs/pjsip.conf.sample
<https://reviewboard.asterisk.org/r/2811/#comment18719>

    Good summary.
    
    One thing that might be nice to point out, since there will be many people looking at this who are coming from sip.conf, is that you are not restricted to a single UDP, a single TCP, and a single TLS transport in pjsip.conf. As long as they don't use the same resources, you can have many UDP, TCP, and TLS transports.



/trunk/configs/pjsip.conf.sample
<https://reviewboard.asterisk.org/r/2811/#comment18720>

    s/setup/set up/
    
    Yes, I'm being this pedantic. The same error is made several other places in the file. The word "setup" is a noun (e.g. "My gaming setup r0xx0rz"), where as "set up" is used in other cases.



/trunk/configs/pjsip.conf.sample
<https://reviewboard.asterisk.org/r/2811/#comment18721>

    Kudos for using multiple contacts in the aor and multiple matches in the identify. This will clear up a lot of potential confusion among people!



/trunk/configs/pjsip.conf.sample
<https://reviewboard.asterisk.org/r/2811/#comment18723>

    While it's easy to see based on the example you've given, it's probably worth explicitly stating that sections can have the same name as long as they are of different types.



/trunk/configs/pjsip.conf.sample
<https://reviewboard.asterisk.org/r/2811/#comment18724>

    Have you tested that this works as expected?
    
    The configuration is likely accepted and stasis subscriptions are set up for each of the specified mailboxes. However, I don't think any actual mailbox changes will ever get sent to the phone with this setup because no voicemail contexts are provided. Stasis mailbox uniqueids are composed of "mailbox at context". For this to work, I think you'd need to change this line to be:
    
    mailboxes=6001 at default,7001 at default,8001 at default,9001 at default
    
    or whatever other voicemail context you choose to use.


- Mark Michelson


On Aug. 29, 2013, 11:33 p.m., rnewton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2811/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2013, 11:33 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22145
>     https://issues.asterisk.org/jira/browse/ASTERISK-22145
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> pjsip.conf!
> 
> The file consists of two main bodies of text.
> 1. Manually written examples - fulfilling a variety of basic configuration scenarios. A few of which are detailed on the ASTERISK-22145 issue.
> 2. A full config option list - Output from a python script I wrote. It takes an xml config dump from Asterisk and parses the pjsip.conf config options out into the format you see in the file. (I may throw that script up here later after I improve it)
> 
> Before the examples there is a blurb talking about where the official documentation is and a brief security notice.
> 
> Notes on the thought process behind what we have so far:
> 
> * We want this to be easy to maintain
> 
> In writing this I found it's easy to turn it into exhaustive documentation and make it very redundant with our other documentation sources. For maintenance sake I trimmed it all back to be about as simple as possible and tried to aim it towards a "quick reference sheet", rather than "demonstrate every config option in action document".
> 
> The full option list is easy to update thanks to it being pulled from a script and not hand edited. 
> 
> * The sample files should start moving towards "sample" files and not be exhaustive documentation.
> * The sample file is not a tutorial.
> 
> The examples provided in the file are geared towards someone who has already read the basics on the wiki[1] or elsewhere. The examples also helpful to serve as context for when you are looking at specific options through the cli config help or referencing the full option list in the bottom of the file. 
> 
> * The examples in the sample file should be simple for a user to understand and modify
> 
> The examples don't have every option listed under them because it makes it a bit confusing to look at. I've tried to arrange them realistically for basic configurations so that it would be easy for you to copy paste options from the big list at the bottom to the examples at the top.
> 
> * Every example is not explained in detail
> 
> I tried to pick out key points or options used in the examples, rather than just explaining them line by line (since that would lead to a lot of redundancy)
> 
> *** Don't provide feedback here for all the individual option names, synopsis, descriptions etc. That is all pulled from the XML config documentation. If you see errors in those areas please file a JIRA issue ***
> 
> Please provide feedback on the example configurations and text that is not in the big list of options at the bottom of the file. That is any obviously wrong configuration (option that couldn't be used in a section), horrible spelling, grammar or logic and flow problems. Suggestions for additional configuration examples that are crucial or essential would be very helpful, especially if you can write the example. Feedback is also very welcome on the formatting of the options list at the bottom. :)
> 
> Moving around all this text, I'm sure I missed quite a few things! Thanks in advance for the help!
> 
> [1]: https://wiki.asterisk.org/wiki/display/AST/Asterisk+12+Installation+and+Configuration
> 
> 
> Diffs
> -----
> 
>   /trunk/configs/pjsip.conf.sample 397957 
> 
> Diff: https://reviewboard.asterisk.org/r/2811/diff/
> 
> 
> Testing
> -------
> 
> I tested pretty much all the configurations in this file or pulled them from other peoples working configurations, there are probably a few option combinations and configurations that I missed, but added in.
> 
> 
> Thanks,
> 
> rnewton
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130830/28a5f3f5/attachment-0001.htm>


More information about the asterisk-dev mailing list