Filter SDP from ICE candidates in case of local ICE candidate filtering
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Jun 2017 08:13:26 +0000 (08:13 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 13 Jun 2017 08:13:26 +0000 (08:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=173120

Patch by Youenn Fablet <youenn@apple.com> on 2017-06-13
Reviewed by Eric Carlson.

Source/WebCore:

Covered by updated test.

Adding filtering of local description in getters and createOffer promise.

* Modules/mediastream/MediaEndpointSessionDescription.cpp:
(WebCore::MediaEndpointSessionDescription::toRTCSessionDescription):
* Modules/mediastream/PeerConnectionBackend.cpp:
(WebCore::PeerConnectionBackend::createOfferSucceeded):
(WebCore::filterICECandidate):
(WebCore::PeerConnectionBackend::filterSDP):
* Modules/mediastream/PeerConnectionBackend.h:
* Modules/mediastream/RTCSessionDescription.h:
(WebCore::RTCSessionDescription::setSdp):
* Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp:
(WebCore::LibWebRTCPeerConnectionBackend::currentLocalDescription):
(WebCore::LibWebRTCPeerConnectionBackend::pendingLocalDescription):
(WebCore::LibWebRTCPeerConnectionBackend::localDescription):

Source/WTF:

Adding split helper routine with functor parameter.

* wtf/text/WTFString.cpp:
(WTF::String::split):
* wtf/text/WTFString.h:
(WTF::String::contains):

LayoutTests:

* webrtc/datachannel/filter-ice-candidate.html: Minor clean-up and
adding assertions to ensure that ICE candidates are also filtered out.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@218168 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/webrtc/datachannel/filter-ice-candidate.html
Source/WTF/ChangeLog
Source/WTF/wtf/text/WTFString.cpp
Source/WTF/wtf/text/WTFString.h
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/MediaEndpointSessionDescription.cpp
Source/WebCore/Modules/mediastream/PeerConnectionBackend.cpp
Source/WebCore/Modules/mediastream/PeerConnectionBackend.h
Source/WebCore/Modules/mediastream/RTCSessionDescription.h
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp

index 1813447..8f9bd2f 100644 (file)
@@ -1,3 +1,13 @@
+2017-06-13  Youenn Fablet  <youenn@apple.com>
+
+        Filter SDP from ICE candidates in case of local ICE candidate filtering
+        https://bugs.webkit.org/show_bug.cgi?id=173120
+
+        Reviewed by Eric Carlson.
+
+        * webrtc/datachannel/filter-ice-candidate.html: Minor clean-up and
+        adding assertions to ensure that ICE candidates are also filtered out.
+
 2017-06-12  Charlie Turner  <cturner@igalia.com>
 
         [GTK] skip fast/scrolling/scrolling-tree-includes-frame.html
index e256acd..5ea9b8e 100644 (file)
@@ -8,12 +8,12 @@
   </head>
   <body>
     <script>
-promise_test(function() {
+promise_test((test) => {
+    if (window.internals)
+        internals.setICECandidateFiltering(false);
+
     return new Promise((resolve, reject) => {
         var counter = 0;
-        if (window.internals) {
-            internals.setICECandidateFiltering(false);
-        }
         var pc = new RTCPeerConnection();
         pc.createDataChannel('sendDataChannel');
         pc.onicecandidate = (event) => {
@@ -21,19 +21,24 @@ promise_test(function() {
                 counter++;
                 return;
             }
-            if (counter !== 0)
-                resolve();
-            else
+            assert_false(pc.localDescription.sdp.indexOf("a=candidate") === -1);
+            if (counter !== 0) {
+                // Redoing an offer now that we have some candidates.
+                pc.createOffer().then((offer) => {
+                    assert_false(offer.sdp.indexOf("a=candidate") === -1);
+                    resolve();
+                });
+            } else
                 reject("Host candidates should be found");
-            if (window.internals)
-                internals.setICECandidateFiltering(true);
-            resolve();
         };
         pc.createOffer().then((offer) => { pc.setLocalDescription(offer); });
-    })
+    });
 }, "Gathering ICE candidates from a data channel peer connection with ICE candidate filtering off");
 
-promise_test(function() {
+promise_test((test) => {
+    if (window.internals)
+        internals.setICECandidateFiltering(true);
+
     return new Promise((resolve, reject) => {
         var counter = 0;
         var pc = new RTCPeerConnection();
@@ -43,12 +48,19 @@ promise_test(function() {
                 counter++;
                 return;
             }
-            if (counter === 0)
-                resolve();
-            else
+            assert_equals(pc.localDescription.sdp.indexOf("a=candidate"), -1);
+            if (counter === 0) {
+                pc.createOffer().then((offer) => {
+                    assert_equals(offer.sdp.indexOf("a=candidate"), -1);
+                    resolve();
+                });
+            } else
                 reject("No candidate should be found");
         };
-        pc.createOffer().then((offer) => { pc.setLocalDescription(offer); });
+        pc.createOffer().then((offer) => {
+            assert_equals(offer.sdp.indexOf("a=candidate"), -1);
+            pc.setLocalDescription(offer);
+        });
     });
 }, "Gathering ICE candidates from a data channel peer connection with ICE candidate filtering on");
     </script>
index 275533d..08911ad 100644 (file)
@@ -1,3 +1,17 @@
+2017-06-13  Youenn Fablet  <youenn@apple.com>
+
+        Filter SDP from ICE candidates in case of local ICE candidate filtering
+        https://bugs.webkit.org/show_bug.cgi?id=173120
+
+        Reviewed by Eric Carlson.
+
+        Adding split helper routine with functor parameter.
+
+        * wtf/text/WTFString.cpp:
+        (WTF::String::split):
+        * wtf/text/WTFString.h:
+        (WTF::String::contains):
+
 2017-06-13  Don Olmstead  <don.olmstead@sony.com>
 
         [WTF] Remove redundant includes in config.h
index 4f49ebc..042d432 100644 (file)
@@ -743,19 +743,27 @@ void String::split(const String& separator, bool allowEmptyEntries, Vector<Strin
         result.append(substring(startPos));
 }
 
-void String::split(UChar separator, bool allowEmptyEntries, Vector<String>& result) const
+void String::split(UChar separator, bool allowEmptyEntries, SplitFunctor&& functor) const
 {
-    result.clear();
+    StringView view(*this);
 
     unsigned startPos = 0;
     size_t endPos;
     while ((endPos = find(separator, startPos)) != notFound) {
         if (allowEmptyEntries || startPos != endPos)
-            result.append(substring(startPos, endPos - startPos));
+            functor(view.substring(startPos, endPos - startPos));
         startPos = endPos + 1;
     }
     if (allowEmptyEntries || startPos != length())
-        result.append(substring(startPos));
+        functor(view.substring(startPos));
+}
+
+void String::split(UChar separator, bool allowEmptyEntries, Vector<String>& result) const
+{
+    result.clear();
+    split(separator, allowEmptyEntries, [&result](StringView item) {
+        result.append(item.toString());
+    });
 }
 
 CString String::ascii() const
index ab21cc2..ba8e611 100644 (file)
@@ -25,6 +25,8 @@
 // This file would be called String.h, but that conflicts with <string.h>
 // on systems without case-sensitive file systems.
 
+#include <functional>
+
 #include <wtf/text/ASCIIFastPath.h>
 #include <wtf/text/IntegerToStringConversion.h>
 #include <wtf/text/StringImpl.h>
@@ -257,11 +259,11 @@ public:
         { return caseSensitive ? reverseFind(str, start) : reverseFindIgnoringCase(str, start); }
 
     WTF_EXPORT_STRING_API Vector<UChar> charactersWithNullTermination() const;
-    
+
     WTF_EXPORT_STRING_API UChar32 characterStartingAt(unsigned) const; // Ditto.
-    
+
     bool contains(UChar c) const { return find(c) != notFound; }
-    bool contains(const LChar* str, bool caseSensitive = true, unsigned startOffset = 0) const 
+    bool contains(const LChar* str, bool caseSensitive = true, unsigned startOffset = 0) const
         { return find(str, startOffset, caseSensitive) != notFound; }
     bool contains(const String& str) const
         { return find(str) != notFound; }
@@ -364,6 +366,9 @@ public:
     {
         split(separator, false, result);
     }
+
+    using SplitFunctor = std::function<void(const StringView&)>;
+    WTF_EXPORT_STRING_API void split(UChar separator, bool allowEmptyEntries, SplitFunctor&&) const;
     WTF_EXPORT_STRING_API void split(UChar separator, bool allowEmptyEntries, Vector<String>& result) const;
     void split(UChar separator, Vector<String>& result) const
     {
@@ -425,7 +430,7 @@ public:
 
 #ifdef __OBJC__
     WTF_EXPORT_STRING_API String(NSString *);
-    
+
     // This conversion converts the null string to an empty NSString rather than to nil.
     // Given Cocoa idioms, this is a more useful default. Clients that need to preserve the
     // null string can check isNull explicitly.
@@ -453,7 +458,7 @@ public:
     // Tries to convert the passed in string to UTF-8, but will fall back to Latin-1 if the string is not valid UTF-8.
     WTF_EXPORT_STRING_API static String fromUTF8WithLatin1Fallback(const LChar*, size_t);
     static String fromUTF8WithLatin1Fallback(const char* s, size_t length) { return fromUTF8WithLatin1Fallback(reinterpret_cast<const LChar*>(s), length); };
-    
+
     // Determines the writing direction using the Unicode Bidi Algorithm rules P2 and P3.
     UCharDirection defaultWritingDirection(bool* hasStrongDirectionality = nullptr) const
     {
index 190c742..96827ea 100644 (file)
@@ -1,3 +1,28 @@
+2017-06-13  Youenn Fablet  <youenn@apple.com>
+
+        Filter SDP from ICE candidates in case of local ICE candidate filtering
+        https://bugs.webkit.org/show_bug.cgi?id=173120
+
+        Reviewed by Eric Carlson.
+
+        Covered by updated test.
+
+        Adding filtering of local description in getters and createOffer promise.
+
+        * Modules/mediastream/MediaEndpointSessionDescription.cpp:
+        (WebCore::MediaEndpointSessionDescription::toRTCSessionDescription):
+        * Modules/mediastream/PeerConnectionBackend.cpp:
+        (WebCore::PeerConnectionBackend::createOfferSucceeded):
+        (WebCore::filterICECandidate):
+        (WebCore::PeerConnectionBackend::filterSDP):
+        * Modules/mediastream/PeerConnectionBackend.h:
+        * Modules/mediastream/RTCSessionDescription.h:
+        (WebCore::RTCSessionDescription::setSdp):
+        * Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp:
+        (WebCore::LibWebRTCPeerConnectionBackend::currentLocalDescription):
+        (WebCore::LibWebRTCPeerConnectionBackend::pendingLocalDescription):
+        (WebCore::LibWebRTCPeerConnectionBackend::localDescription):
+
 2017-06-12  Myles C. Maxfield  <mmaxfield@apple.com>
 
         [iOS] [macOS] Stop soft-linking Core Text function in iOS 11 and macOS High Sierra
index 24ce373..cdf39b2 100644 (file)
@@ -83,7 +83,7 @@ RefPtr<RTCSessionDescription> MediaEndpointSessionDescription::toRTCSessionDescr
     // that same instance but with an updated sdp. It is used for RTCPeerConnection's description
     // atributes (e.g. localDescription and pendingLocalDescription).
     if (m_rtcDescription) {
-        m_rtcDescription->setSdp(sdpString);
+        m_rtcDescription->setSdp(WTFMove(sdpString));
         return m_rtcDescription;
     }
 
index b970abc..154ef2e 100644 (file)
@@ -62,7 +62,7 @@ void PeerConnectionBackend::createOfferSucceeded(String&& sdp)
         return;
 
     ASSERT(m_offerAnswerPromise);
-    m_offerAnswerPromise->resolve(RTCSessionDescription::create(RTCSdpType::Offer, WTFMove(sdp)));
+    m_offerAnswerPromise->resolve(RTCSessionDescription::create(RTCSdpType::Offer, filterSDP(WTFMove(sdp))));
     m_offerAnswerPromise = std::nullopt;
 }
 
@@ -92,7 +92,7 @@ void PeerConnectionBackend::createAnswerSucceeded(String&& sdp)
 {
     ASSERT(isMainThread());
     RELEASE_LOG(WebRTC, "Creating answer succeeded:\n%s\n", sdp.utf8().data());
-    
+
     if (m_peerConnection.isClosed())
         return;
 
@@ -301,34 +301,49 @@ void PeerConnectionBackend::disableICECandidateFiltering()
     m_pendingICECandidates.clear();
 }
 
-static inline String filterICECandidate(String&& sdp)
+static String filterICECandidate(String&& sdp)
 {
     ASSERT(!sdp.contains(" host "));
 
     if (!sdp.contains(" raddr "))
         return WTFMove(sdp);
 
-    Vector<String> items;
-    sdp.split(' ', items);
-
     bool skipNextItem = false;
     bool isFirst = true;
     StringBuilder filteredSDP;
-    for (auto& item : items) {
+    sdp.split(' ', false, [&](StringView item) {
         if (skipNextItem) {
             skipNextItem = false;
-            continue;
+            return;
         }
         if (item == "raddr" || item == "rport") {
             skipNextItem = true;
-            continue;
+            return;
         }
         if (isFirst)
             isFirst = false;
         else
             filteredSDP.append(" ");
         filteredSDP.append(item);
-    }
+    });
+    return filteredSDP.toString();
+}
+
+String PeerConnectionBackend::filterSDP(String&& sdp) const
+{
+    if (!m_shouldFilterICECandidates)
+        return sdp;
+
+    StringBuilder filteredSDP;
+    sdp.split('\n', false, [&filteredSDP](StringView line) {
+        if (!line.startsWith("a=candidate"))
+            filteredSDP.append(line);
+        else if (line.find(" host ", 11) == notFound)
+            filteredSDP.append(filterICECandidate(line.toString()));
+        else
+            return;
+        filteredSDP.append('\n');
+    });
     return filteredSDP.toString();
 }
 
@@ -340,7 +355,7 @@ void PeerConnectionBackend::newICECandidate(String&& sdp, String&& mid)
         fireICECandidateEvent(RTCIceCandidate::create(WTFMove(sdp), WTFMove(mid), 0));
         return;
     }
-    if (sdp.contains(" host ")) {
+    if (sdp.find(" host ", 0) != notFound) {
         m_pendingICECandidates.append(PendingICECandidate { WTFMove(sdp), WTFMove(mid)});
         return;
     }
@@ -379,9 +394,9 @@ void PeerConnectionBackend::markAsNeedingNegotiation()
 {
     if (m_negotiationNeeded)
         return;
-    
+
     m_negotiationNeeded = true;
-    
+
     if (m_peerConnection.signalingState() == RTCSignalingState::Stable)
         m_peerConnection.scheduleNegotiationNeededEvent();
 }
index 8f95a1b..87b38cc 100644 (file)
@@ -133,6 +133,8 @@ protected:
     void addIceCandidateSucceeded();
     void addIceCandidateFailed(Exception&&);
 
+    String filterSDP(String&&) const;
+
 private:
     virtual void doCreateOffer(RTCOfferOptions&&) = 0;
     virtual void doCreateAnswer(RTCAnswerOptions&&) = 0;
index 41d4537..109b2a9 100644 (file)
@@ -52,7 +52,7 @@ public:
     RTCSdpType type() const { return m_type; }
 
     const String& sdp() const { return m_sdp; }
-    void setSdp(const String& sdp) { m_sdp = sdp; }
+    void setSdp(String&& sdp) { m_sdp = WTFMove(sdp); }
 
 private:
     RTCSessionDescription(RTCSdpType, String&& sdp);
index 1d84993..aecd967 100644 (file)
@@ -41,6 +41,7 @@
 #include "RTCSessionDescription.h"
 #include "RealtimeIncomingAudioSource.h"
 #include "RealtimeIncomingVideoSource.h"
+#include <wtf/text/StringBuilder.h>
 
 namespace WebCore {
 
@@ -277,7 +278,10 @@ std::unique_ptr<RTCDataChannelHandler> LibWebRTCPeerConnectionBackend::createDat
 
 RefPtr<RTCSessionDescription> LibWebRTCPeerConnectionBackend::currentLocalDescription() const
 {
-    return m_endpoint->currentLocalDescription();
+    auto description = m_endpoint->currentLocalDescription();
+    if (description)
+        description->setSdp(filterSDP(String(description->sdp())));
+    return description;
 }
 
 RefPtr<RTCSessionDescription> LibWebRTCPeerConnectionBackend::currentRemoteDescription() const
@@ -287,7 +291,10 @@ RefPtr<RTCSessionDescription> LibWebRTCPeerConnectionBackend::currentRemoteDescr
 
 RefPtr<RTCSessionDescription> LibWebRTCPeerConnectionBackend::pendingLocalDescription() const
 {
-    return m_endpoint->pendingLocalDescription();
+    auto description = m_endpoint->pendingLocalDescription();
+    if (description)
+        description->setSdp(filterSDP(String(description->sdp())));
+    return description;
 }
 
 RefPtr<RTCSessionDescription> LibWebRTCPeerConnectionBackend::pendingRemoteDescription() const
@@ -297,7 +304,10 @@ RefPtr<RTCSessionDescription> LibWebRTCPeerConnectionBackend::pendingRemoteDescr
 
 RefPtr<RTCSessionDescription> LibWebRTCPeerConnectionBackend::localDescription() const
 {
-    return m_endpoint->localDescription();
+    auto description = m_endpoint->localDescription();
+    if (description)
+        description->setSdp(filterSDP(String(description->sdp())));
+    return description;
 }
 
 RefPtr<RTCSessionDescription> LibWebRTCPeerConnectionBackend::remoteDescription() const