[asterisk-dev] [Code Review] HTTP Digest authentication

Mark Michelson mmichelson at digium.com
Wed Apr 15 11:01:08 CDT 2009


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/223/#review699
-----------------------------------------------------------


Most of what I'm pointing out here are very minor problems. Good job here, Tilghman!

There are quite a few places where review board is showing spacing inconsistencies (big red parts) in your new code. Rather than try to point all of them out individually, I'll just let you know that they're there and should be fixed.


/trunk/include/asterisk/http.h
<http://reviewboard.digium.com/r/223/#comment1804>

    This is an ABI compatibility change that should be avoided.



/trunk/include/asterisk/http.h
<http://reviewboard.digium.com/r/223/#comment1806>

    I don't want to start a dialect war over response vs. responce, but we should be consistent in our documentation.



/trunk/include/asterisk/http.h
<http://reviewboard.digium.com/r/223/#comment1810>

    This last section of documentation needs rewriting.



/trunk/include/asterisk/utils.h
<http://reviewboard.digium.com/r/223/#comment1808>

    This seems like a good use for string fields.



/trunk/main/http.c
<http://reviewboard.digium.com/r/223/#comment1809>

    minor spacing issue for *post_vars=NULL


- Mark


On 2009-04-14 18:48:27, Tilghman Lesher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/223/
> -----------------------------------------------------------
> 
> (Updated 2009-04-14 18:48:27)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Enable the use of HTTP Digest authentication when using the AMI HTTP interface.
> 
> 
> This addresses bug 10961.
>     http://bugs.digium.com/view.php?id=10961
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/http.h 188475 
>   /trunk/include/asterisk/utils.h 188475 
>   /trunk/main/ast_expr2.h 188475 
>   /trunk/main/astobj2.c 188475 
>   /trunk/main/http.c 188475 
>   /trunk/main/manager.c 188475 
>   /trunk/main/utils.c 188475 
>   /trunk/res/res_http_post.c 188475 
>   /trunk/res/res_phoneprov.c 188475 
> 
> Diff: http://reviewboard.digium.com/r/223/diff
> 
> 
> Testing
> -------
> 
> Lots of testing with fairly large files; all issues found were resolved.
> 
> 
> Thanks,
> 
> Tilghman
> 
>




More information about the asterisk-dev mailing list