diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index d0bd3c88af..84a075630e 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -491,52 +491,6 @@ RTCError ValidateBundledPayloadTypes( return RTCError::OK(); } -RTCError FindDuplicateHeaderExtensionIds( - const RtpExtension extension, - std::map& id_to_extension) { - auto existing_extension = id_to_extension.find(extension.id); - if (existing_extension != id_to_extension.end() && - extension != existing_extension->second) { - return RTCError( - RTCErrorType::INVALID_PARAMETER, - "A BUNDLE group contains a codec collision for " - "header extension id='" + - rtc::ToString(extension.id) + - ". The id must be the same across all bundled media descriptions"); - } - id_to_extension.insert(std::make_pair(extension.id, extension)); - return RTCError::OK(); -} - -RTCError ValidateBundledRtpHeaderExtensions( - const cricket::SessionDescription& description) { - // https://www.rfc-editor.org/rfc/rfc8843#name-rtp-header-extensions-consi - // ... the identifier used for a given extension MUST identify the same - // extension across all the bundled media descriptions. - std::vector bundle_groups = - description.GetGroupsByName(cricket::GROUP_TYPE_BUNDLE); - for (const cricket::ContentGroup* bundle_group : bundle_groups) { - std::map id_to_extension; - for (const std::string& content_name : bundle_group->content_names()) { - const cricket::MediaContentDescription* media_description = - description.GetContentDescriptionByName(content_name); - if (!media_description) { - return RTCError(RTCErrorType::INVALID_PARAMETER, - "A BUNDLE group contains a MID='" + content_name + - "' matching no m= section."); - } - for (const auto& extension : media_description->rtp_header_extensions()) { - auto error = - FindDuplicateHeaderExtensionIds(extension, id_to_extension); - if (!error.ok()) { - return error; - } - } - } - } - return RTCError::OK(); -} - bool IsValidOfferToReceiveMedia(int value) { typedef PeerConnectionInterface::RTCOfferAnswerOptions Options; return (value >= Options::kUndefined) && @@ -3417,18 +3371,12 @@ RTCError SdpOfferAnswerHandler::ValidateSessionDescription( return RTCError(RTCErrorType::INVALID_PARAMETER, kSdpWithoutIceUfragPwd); } - // Validate that there are no collisions of bundled payload types. + // Validate bundle, payload types and that there are no collisions. error = ValidateBundledPayloadTypes(*sdesc->description()); // TODO(bugs.webrtc.org/14420): actually reject. RTC_HISTOGRAM_BOOLEAN("WebRTC.PeerConnection.ValidBundledPayloadTypes", error.ok()); - // Validate that there are no collisions of bundled header extensions ids. - error = ValidateBundledRtpHeaderExtensions(*sdesc->description()); - // TODO(bugs.webrtc.org/14782): actually reject. - RTC_HISTOGRAM_BOOLEAN("WebRTC.PeerConnection.ValidBundledExtensionIds", - error.ok()); - if (!pc_->ValidateBundleSettings(sdesc->description(), bundle_groups_by_mid)) { return RTCError(RTCErrorType::INVALID_PARAMETER, kBundleWithoutRtcpMux); diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc index bdd3301060..ecac7f8de8 100644 --- a/pc/sdp_offer_answer_unittest.cc +++ b/pc/sdp_offer_answer_unittest.cc @@ -238,130 +238,6 @@ TEST_F(SdpOfferAnswerTest, BundleCodecCollisionInDifferentBundlesAllowed) { "WebRTC.PeerConnection.ValidBundledPayloadTypes", false)); } -TEST_F(SdpOfferAnswerTest, BundleMeasuresHeaderExtensionIdCollision) { - auto pc = CreatePeerConnection(); - std::string sdp = - "v=0\r\n" - "o=- 0 3 IN IP4 127.0.0.1\r\n" - "s=-\r\n" - "t=0 0\r\n" - "a=group:BUNDLE 0 1\r\n" - "a=fingerprint:sha-1 " - "4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n" - "a=setup:actpass\r\n" - "a=ice-ufrag:ETEn\r\n" - "a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n" - "m=audio 9 UDP/TLS/RTP/SAVPF 111\r\n" - "c=IN IP4 0.0.0.0\r\n" - "a=rtcp-mux\r\n" - "a=sendonly\r\n" - "a=mid:0\r\n" - "a=rtpmap:111 opus/48000/2\r\n" - "a=extmap:3 " - "http://www.ietf.org/id/" - "draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n" - "m=video 9 UDP/TLS/RTP/SAVPF 112\r\n" - "c=IN IP4 0.0.0.0\r\n" - "a=rtcp-mux\r\n" - "a=sendonly\r\n" - "a=mid:1\r\n" - "a=rtpmap:112 VP8/90000\r\n" - "a=extmap:3 " - "http://www.ietf.org/id/" - "draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n"; - auto desc = CreateSessionDescription(SdpType::kOffer, sdp); - ASSERT_NE(desc, nullptr); - RTCError error; - pc->SetRemoteDescription(std::move(desc), &error); - EXPECT_TRUE(error.ok()); - EXPECT_METRIC_EQ(1, - webrtc::metrics::NumEvents( - "WebRTC.PeerConnection.ValidBundledExtensionIds", true)); -} - -// extmap:3 is used with two different URIs which is not allowed. -TEST_F(SdpOfferAnswerTest, BundleRejectsHeaderExtensionIdCollision) { - auto pc = CreatePeerConnection(); - std::string sdp = - "v=0\r\n" - "o=- 0 3 IN IP4 127.0.0.1\r\n" - "s=-\r\n" - "t=0 0\r\n" - "a=group:BUNDLE 0 1\r\n" - "a=fingerprint:sha-1 " - "4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n" - "a=setup:actpass\r\n" - "a=ice-ufrag:ETEn\r\n" - "a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n" - "m=audio 9 UDP/TLS/RTP/SAVPF 111\r\n" - "c=IN IP4 0.0.0.0\r\n" - "a=rtcp-mux\r\n" - "a=sendonly\r\n" - "a=mid:0\r\n" - "a=rtpmap:111 opus/48000/2\r\n" - "a=extmap:3 " - "http://www.ietf.org/id/" - "draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n" - "m=video 9 UDP/TLS/RTP/SAVPF 112\r\n" - "c=IN IP4 0.0.0.0\r\n" - "a=rtcp-mux\r\n" - "a=sendonly\r\n" - "a=mid:1\r\n" - "a=rtpmap:112 VP8/90000\r\n" - "a=extmap:3 urn:3gpp:video-orientation\r\n"; - auto desc = CreateSessionDescription(SdpType::kOffer, sdp); - ASSERT_NE(desc, nullptr); - RTCError error; - pc->SetRemoteDescription(std::move(desc), &error); - EXPECT_TRUE(error.ok()); - EXPECT_METRIC_EQ( - 1, webrtc::metrics::NumEvents( - "WebRTC.PeerConnection.ValidBundledExtensionIds", false)); -} - -// transport-wide cc is negotiated with two different ids 3 and 4. -// This is not a good idea but tolerable. -TEST_F(SdpOfferAnswerTest, BundleAcceptsDifferentIdsForSameExtension) { - auto pc = CreatePeerConnection(); - std::string sdp = - "v=0\r\n" - "o=- 0 3 IN IP4 127.0.0.1\r\n" - "s=-\r\n" - "t=0 0\r\n" - "a=group:BUNDLE 0 1\r\n" - "a=fingerprint:sha-1 " - "4A:AD:B9:B1:3F:82:18:3B:54:02:12:DF:3E:5D:49:6B:19:E5:7C:AB\r\n" - "a=setup:actpass\r\n" - "a=ice-ufrag:ETEn\r\n" - "a=ice-pwd:OtSK0WpNtpUjkY4+86js7Z/l\r\n" - "m=audio 9 UDP/TLS/RTP/SAVPF 111\r\n" - "c=IN IP4 0.0.0.0\r\n" - "a=rtcp-mux\r\n" - "a=sendonly\r\n" - "a=mid:0\r\n" - "a=rtpmap:111 opus/48000/2\r\n" - "a=extmap:3 " - "http://www.ietf.org/id/" - "draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n" - "m=video 9 UDP/TLS/RTP/SAVPF 112\r\n" - "c=IN IP4 0.0.0.0\r\n" - "a=rtcp-mux\r\n" - "a=sendonly\r\n" - "a=mid:1\r\n" - "a=rtpmap:112 VP8/90000\r\n" - "a=extmap:4 " - "http://www.ietf.org/id/" - "draft-holmer-rmcat-transport-wide-cc-extensions-01\r\n"; - auto desc = CreateSessionDescription(SdpType::kOffer, sdp); - ASSERT_NE(desc, nullptr); - RTCError error; - pc->SetRemoteDescription(std::move(desc), &error); - EXPECT_TRUE(error.ok()); - EXPECT_METRIC_EQ(1, - webrtc::metrics::NumEvents( - "WebRTC.PeerConnection.ValidBundledExtensionIds", true)); -} - TEST_F(SdpOfferAnswerTest, LargeMidsAreRejected) { auto pc = CreatePeerConnection(); std::string sdp =