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

[Tilghman Lesher]

> On 2009-04-16 13:49:55, Russell Bryant wrote:
> > /trunk/main/http.c, line 283
> > <http://reviewboard.digium.com/r/223/diff/4/?file=4189#file4189line283>
> >
> >     This is an important fact that should be in the ast_http_send() API docs

It actually IS there, but since it is out of the ordinary from the way most functions do it, I felt that it was useful to note.


> On 2009-04-16 13:49:55, Russell Bryant wrote:
> > /trunk/main/astobj2.c, lines 460-461
> > <http://reviewboard.digium.com/r/223/diff/4/?file=4188#file4188line460>
> >
> >     I'm okay with this and the other changes to astobj2.c, but they should go in separately since they are not related to this functionality.

Probably.  I just noticed that if you don't specify the hashing function, then it just uses a single container anyway.


> On 2009-04-16 13:49:55, Russell Bryant wrote:
> > /trunk/include/asterisk/http.h, lines 141-142
> > <http://reviewboard.digium.com/r/223/diff/4/?file=4185#file4185line141>
> >
> >     This should probably return const.  Also, full doxygen documentation would be preferred.  (The full documentation comment applies to all new API calls in this patch.)

Changed.  Also added attribute_pure for the compiler.


> On 2009-04-16 13:49:55, Russell Bryant wrote:
> > /trunk/include/asterisk/http.h, line 182
> > <http://reviewboard.digium.com/r/223/diff/4/?file=4185#file4185line182>
> >
> >     This is not a valid name for a new API call.  In fact, it's not even possible that using this would have worked unless you also updated the appropriate .export file.

Changed.


> On 2009-04-16 13:49:55, Russell Bryant wrote:
> > /trunk/main/ast_expr2.h, lines 92-108
> > <http://reviewboard.digium.com/r/223/diff/4/?file=4187#file4187line92>
> >
> >     Unrelated changes

Oops, missed those.  Fixed.


> On 2009-04-16 13:49:55, Russell Bryant wrote:
> > /trunk/main/http.c, line 121
> > <http://reviewboard.digium.com/r/223/diff/4/?file=4189#file4189line121>
> >
> >     This should probably just be const char text[]

Changed.


> On 2009-04-16 13:49:55, Russell Bryant wrote:
> > /trunk/main/http.c, lines 131-133
> > <http://reviewboard.digium.com/r/223/diff/4/?file=4189#file4189line131>
> >
> >     coding guielines

Fixed.


> On 2009-04-16 13:49:55, Russell Bryant wrote:
> > /trunk/main/http.c, line 154
> > <http://reviewboard.digium.com/r/223/diff/4/?file=4189#file4189line154>
> >
> >     coding guielines ...

Fixed.  Also changed the name to ast_http_manid_from_vars().


- Tilghman


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


On 2009-04-15 12:26:54, Tilghman Lesher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/223/
> -----------------------------------------------------------
> 
> (Updated 2009-04-15 12:26:54)
> 
> 
> 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
> 
>