[asterisk-dev] [Code Review] 3125: http: support chunked Transfer-Encoding

[Scott Griepentrog]

> On Jan. 13, 2014, 4:24 p.m., Matt Jordan wrote:
> > /branches/12/main/http.c, lines 804-805
> > <https://reviewboard.asterisk.org/r/3125/diff/1/?file=52887#file52887line804>
> >
> >     You don't need the while(1) here, as you aren't breaking out of the block scope that it provides.
> 
> rmudgett wrote:
>     The curly is in the wrong place per guidelines.
>     Also I know some static checkers can complain of an always true while loop where they won't if you use for (;;)

It's a loop - there are multiple header/chunk sequences (minimum 2) that have to be read.


> On Jan. 13, 2014, 4:24 p.m., Matt Jordan wrote:
> > /branches/12/main/http.c, lines 797-801
> > <https://reviewboard.asterisk.org/r/3125/diff/1/?file=52887#file52887line797>
> >
> >     You could actually defer allocating buffer here until you know content_length + chunk_length, then allocate the buffer. That would have the benefit of only allocating as much memory as you actually need.

The number of chunks isn't known ahead of time, and in most cases there will be a high quantity of small chunks, thus pre-allocating a reasonably sized buffer will most of the time avoid the cpu cost of reallocating.


> On Jan. 13, 2014, 4:24 p.m., Matt Jordan wrote:
> > /branches/12/main/http.c, lines 831-838
> > <https://reviewboard.asterisk.org/r/3125/diff/1/?file=52887#file52887line831>
> >
> >     Well, if chunk_length is *really* big, even bufsize *=2 might not be sufficient.
> >     
> >     In fact, MAX_POST_CONTENT is 1025, so we should never actually hit this code, since we explicitly disallow a chunk_length + content_length greater than that.

This will allow it to continue to expand the buffer as needed to meet the incoming stream.  However, in light of MAX_POST_CONTENT being that small, I should lower the initial allocation.

This algorithm is robust to handle future expansion should we ever need the ability to handle larger content.


- Scott


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


On Jan. 13, 2014, 4:09 p.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3125/
> -----------------------------------------------------------
> 
> (Updated Jan. 13, 2014, 4:09 p.m.)
> 
> 
> Review request for Asterisk Developers and Matt Jordan.
> 
> 
> Bugs: ASTERISK-23068
>     https://issues.asterisk.org/jira/browse/ASTERISK-23068
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Implements support for HTTP chunked mode Transfer-Encoding in JSON and Form (post vars) body content.  Relocated code from ast_http_get_json() and ast_http_get_post_vars() that reads content from stream into new function ast_http_get_contents(), and added support for reading and decoding chunked mode transfers.
> 
> 
> Diffs
> -----
> 
>   /branches/12/main/http.c 405282 
>   /branches/12/include/asterisk/http.h 405282 
> 
> Diff: https://reviewboard.asterisk.org/r/3125/diff/
> 
> 
> Testing
> -------
> 
> Testsuite test rest_api/chunked_transfer added (see https://reviewboard.asterisk.org/r/3126/) to insure correct operation of chunked and regular mode.
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

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