1151122 - Missing mid's are not detected by SDP parser

Status: NEW

(Core :: WebRTC: Signaling, defect, P4)

Description

[Nils Ohlmeier [:drno]]

If the group bundle attribute contains an ID, which does not appear in any mid attribute our parser does not complain.
It results in different views on what to bundle and ultimately in ICE transport errors, instead of exiting early.

Comment 1

[Nils Ohlmeier [:drno]]

Attachment: bug_1151122_check_mids.patch

Does this look reasonable?
Should I a test for this?

Comment 2

[Byron Campen [:bwc]]

Comment on attachment 8588223 [details] [diff] [review]
bug_1151122_check_mids.patch

Review of attachment 8588223 [details] [diff] [review]:
-----------------------------------------------------------------

Just nits. I suppose a test couldn't hurt (writing one in jsep_session_unittest should be easy if you just stomp the mid attributes using Replace).

::: media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ +1911,5 @@
>      return NS_ERROR_INVALID_ARG;
>    }
>  
>    std::set<std::string> trackIds;
> +  std::set<std::string> mIds;

mIds looks like a member variable, mids should be fine.

@@ +1993,5 @@
> +  if (groups && !groups->tags.empty() && mIds.size()) {
> +    for (auto id=groups->tags.begin(); id!=groups->tags.end(); id++) {
> +      if (!mIds.count(*id)) {
> +        JSEP_SET_ERROR("group bundle id: " << *id
> +                       << " is missing in m-section mId's");

Maybe phrase as "Unknown mid " << *id << " in group attribute."

Comment 3

[Bulk Bug Changes for mreavy's org]

Mass change P3->P4 to align with new Mozilla triage process.

Comment 4

[Nils Ohlmeier [:drno]]

I don't have the capacity any more to work on this any time soon.