[reSIProcate] resiprocate parsing masks SDP errors

[John Gregg]

I have some test cases in which a device sends my resiprocate code some 
malformed SDP (in particular, I've been focusing on dynamic payload 
types in the 'm' line that don't match up with any "a=rtpmap" lines, but 
the exact nature of the protocol violation is not important here.) I see 
that in SdpContents.cxx, SdpContents::Session::Medium::codecs(), we wrap 
the actual call to parse() in a try/catch, and in the event of an 
exception, we just silently don't include the offending payload type in 
the rtpmap. Likewise, if we see a payload in the 'm' line that isn't 
also in an 'a=rtpmap" line and also isn't a static codec, we silently 
ignore that as well.

I understand about "Be conservative in what you send, liberal in what 
you accept," but sometimes silently letting the other guy get away with 
what are definitely violations of protocol can make things worse for 
everyone. You might limp along doing something suboptimal for a long 
time with no one ever realizing it, when it might have been better to 
explicitly fail the call with a 4XX.

Either way, it seems that the application developer ought to have 
discretion to make this call, but as the code is written, there is no 
way for the stack to communicate up to the app that this has happened. 
All my code knows about are the codecs that survived and were valid. It 
never knows about the ones that failed the parse.

I'm not even sure what I'd like to happen ideally here. You certainly 
don't want to unconditionally abort parsing upon finding an error (so 
throwing an exception would be the wrong idea). Ideally, you'd want 
somehow to set a checkable status for the app to see if he wanted to, 
saying that there was a parsing error, and with a text field for more 
detail. Or something like that.

At the very least, in my own code I have added the following InfoLog 
near the bottom of SdpContents::Session::Medium::codecs():

if (ri != staticCodecs.end())
{
...
}
else
{
     InfoLog(<< "SdpContents::Session::Medium::codecs invalid SDP: 
payload type " << *i << " is neither in an rtpmap nor is it a static 
codec");
}

Any ideas? Am I missing something?

Thanks,

-John Gregg

FYI: here is a snippet to illustrate what I'm talking about (the 96 
should be a 99, or the 99s should be 96s):

m=video 7002 RTP/AVP 97 96
c=IN IP4 10.0.23.14
a=rtpmap:97 H264/90000
a=fmtp:97 profile-level-id=42e028; packetization-mode=1; max-br=452; 
max-mbps=11880
a=rtpmap:99 H263-2000/90000
a=fmtp:99 profile=0;level=45



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://list.resiprocate.org/pipermail/resiprocate-devel/attachments/20140926/b3d3f91e/attachment.htm>