[asterisk-dev] [Code Review] rfc compliant sip option parsing + new unit test

[David Vossel]

> On 2010-06-09 03:47:19, Nick Lewis wrote:
> > Looks good to go. Perhaps the CLI could output the name of a test when it passes too. There is some ambiguity on the CLI as to what constitutes a test. After running though all of sip_parse_options_test the CLI reports "1 test passed" so perhaps the tests within sip_parse_options_test are merely subtests e.g. "subtest \"%s\" passed, bit profile: %x\n"

I will make this change, thanks for your reviews Nick!


- David


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


On 2010-06-08 17:12:28, David Vossel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/680/
> -----------------------------------------------------------
> 
> (Updated 2010-06-08 17:12:28)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> RFC 3261 section 8.2.2.3 states that if any unsupported options are found in the Require header field, a "420 (Bad Extension)" response should be sent with an Unsupported header field containing only the unsupported options.
> 
> This is not currently being done correctly.  Right now, if Asterisk detects any unsupported sip options in a Require header the entire list of options are returned in the Unsupported header even if some of those options are in fact supported.  This patch fixes that by building an unsupported options character buffer when parsing the options that can be sent with the 420 response.  A unit test verifying this functionality has been created.  Some code refactoring was required.
> 
> RFC 3261 section 8.2.2.3
> "  If a UAS does not understand an option-tag listed in a
>    Require header field, it MUST respond by generating a response with
>    status code 420 (Bad Extension).  The UAS MUST add an Unsupported
>    header field, and list in it those options it does not understand
>    amongst those in the Require header field of the request."
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_sip.c 269118 
>   /trunk/channels/sip/include/reqresp_parser.h 269118 
>   /trunk/channels/sip/include/sip.h 269118 
>   /trunk/channels/sip/reqresp_parser.c 269118 
> 
> Diff: https://reviewboard.asterisk.org/r/680/diff
> 
> 
> Testing
> -------
> 
> unit test passes and verifies expected behavior... test calls were made as well just as a sanity check.
> 
> 
> Thanks,
> 
> David
> 
>