[asterisk-dev] RFC: chan_sip SDP parsing changes in behavior

Paul Belanger paul.belanger at polybeacon.com
Wed May 30 08:18:59 CDT 2012


On 12-05-30 08:27 AM, Kevin P. Fleming wrote:
> In review 1811 (https://reviewboard.asterisk.org/r/1811/) I've got a set
> of changes designed to accomplish multiple things:
>
> * Improve chan_sip's warning/error messages related to SDP parsing to
> use proper RFC-defined terminology, and ensure that the messages are
> clearly stating exactly what happened.
>
> * Ensure that warning/error messages related to parsing SDP 'm' lines
> include the 'm' line content that was in error.
>
> * Ensure that Asterisk *rejects* SDP offers/answers that contain media
> stream specifications that Asterisk doesn't support.
>
> This latter change is what is of some concern here: if this patch is
> merged, Asterisk will start rejecting SDP offers/answers that it used to
> accept or ignore. Specifically, if:
>
> * An 'm' line for an audio/video/text stream includes an unknown RTP
> profile.
>
> * An 'm' line specifies a top-level media type other than 'audio',
> 'video', 'text' or 'image'.
>
> * An 'm' line specifies more than one port for an audio/video/text
> stream (the current patch allows this to pass, but I think it should be
> changed before it is committed).
>
> Personally I'd prefer that Asterisk behave this way, even in the
> existing release branches, as accepting such SDP offers/answers will
> only lead to broken calls anyway, and it would be better to break the
> call at the beginning as soon as the media offer problems can be
> identified.
>
> Does anyone out there object to these changes in behavior going into
> 1.8, 10, and trunk?
>
Regarding 1.8 and 10 branches, do we have any idea how many endpoints 
will be affected?  I tried looking for a JIRA issue but didn't find 
anything.  My concert would be the change in functionality for people 
upgrading a dot release.

Also, have we added any new tests for this patch?  Testing functionality 
before and after the change?  All I can see if 'Compile testing only', 
which I'm not a fan off when we are talking about 'changes in behavior'. 
  I would be more comfortable with 1.8 and 10 if we did have the tests.

-- 
Paul Belanger | PolyBeacon, Inc.
Jabber: paul.belanger at polybeacon.com | IRC: pabelanger (Freenode)
Github: https://github.com/pabelanger | Twitter: 
https://twitter.com/pabelanger



More information about the asterisk-dev mailing list