Preventively expect UTF8 strings from libwebrtc SDP and error messages
authoryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Apr 2018 21:49:28 +0000 (21:49 +0000)
committeryouenn@apple.com <youenn@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 11 Apr 2018 21:49:28 +0000 (21:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184509

Reviewed by Eric Carlson.

Source/WebCore:

Make WebCore code expect any libwebrtc string to contain UTF-8.
Currently SDPs do not contain any UTF-8 specific character
but https://tools.ietf.org/html/rfc4566 allows it.

Add Internals API to set track id so that we can inject UTF-8 inside some WebRTC tests.
Test: webrtc/utf8-sdp.html

* Modules/mediastream/MediaStreamTrack.h:
(WebCore::MediaStreamTrack::setIdForTesting):
* Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
(WebCore::fromStdString):
(WebCore::fromSessionDescription):
(WebCore::LibWebRTCMediaEndpoint::doSetLocalDescription):
(WebCore::LibWebRTCMediaEndpoint::doSetRemoteDescription):
(WebCore::LibWebRTCMediaEndpoint::mediaStreamFromRTCStream):
(WebCore::LibWebRTCMediaEndpoint::addRemoteTrack):
(WebCore::LibWebRTCMediaEndpoint::addDataChannel):
(WebCore::LibWebRTCMediaEndpoint::OnIceCandidate):
(WebCore::LibWebRTCMediaEndpoint::createSessionDescriptionSucceeded):
(WebCore::LibWebRTCMediaEndpoint::createSessionDescriptionFailed):
(WebCore::LibWebRTCMediaEndpoint::setLocalSessionDescriptionFailed):
(WebCore::LibWebRTCMediaEndpoint::setRemoteSessionDescriptionFailed):
(WebCore::trackId): Deleted.
* Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp:
(WebCore::fromStdString):
(WebCore::LibWebRTCPeerConnectionBackend::doAddIceCandidate):
(WebCore::createReceiverForSource):
* platform/mediastream/MediaStreamTrackPrivate.h:
(WebCore::MediaStreamTrackPrivate::setIdForTesting):
* testing/Internals.cpp:
(WebCore::Internals::setMediaStreamTrackIdentifier):
* testing/Internals.h:
* testing/Internals.idl:

LayoutTests:

* webrtc/utf8-sdp-expected.txt: Added.
* webrtc/utf8-sdp.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/webrtc/utf8-sdp-expected.txt [new file with mode: 0644]
LayoutTests/webrtc/utf8-sdp.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/mediastream/MediaStreamTrack.h
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp
Source/WebCore/Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp
Source/WebCore/platform/mediastream/MediaStreamTrackPrivate.h
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl

index 1ec456e..cbc6886 100644 (file)
@@ -1,3 +1,13 @@
+2018-04-11  Youenn Fablet  <youenn@apple.com>
+
+        Preventively expect UTF8 strings from libwebrtc SDP and error messages
+        https://bugs.webkit.org/show_bug.cgi?id=184509
+
+        Reviewed by Eric Carlson.
+
+        * webrtc/utf8-sdp-expected.txt: Added.
+        * webrtc/utf8-sdp.html: Added.
+
 2018-04-11  Alicia Boya García  <aboya@igalia.com>
 
         [GTK] Unreviewed test gardening
diff --git a/LayoutTests/webrtc/utf8-sdp-expected.txt b/LayoutTests/webrtc/utf8-sdp-expected.txt
new file mode 100644 (file)
index 0000000..b9640a9
--- /dev/null
@@ -0,0 +1,4 @@
+
+
+PASS Testing video exchange with UTF-8 track id 
+
diff --git a/LayoutTests/webrtc/utf8-sdp.html b/LayoutTests/webrtc/utf8-sdp.html
new file mode 100644 (file)
index 0000000..ca0ad0f
--- /dev/null
@@ -0,0 +1,39 @@
+<!doctype html>
+<html>
+    <head>
+        <meta charset="utf-8">
+        <title>Testing UTF-8 content in SDP</title>
+        <script src="../resources/testharness.js"></script>
+        <script src="../resources/testharnessreport.js"></script>
+    </head>
+    <body>
+        <video id="video" autoplay=""></video>
+        <script src ="routines.js"></script>
+        <script>
+promise_test(async (test) => {
+    if (!window.internals)
+        return Promise.reject("internals required");
+
+    const unicodeString = '世界你好';
+
+    const stream = await navigator.mediaDevices.getUserMedia({video: true});
+    internals.setMediaStreamTrackIdentifier(stream.getVideoTracks()[0], unicodeString);
+
+    const remoteStream = await new Promise((resolve, reject) => {
+        createConnections((firstConnection) => {
+            firstConnection.addTrack(stream.getVideoTracks()[0], stream);
+        }, (secondConnection) => {
+            secondConnection.ontrack = (trackEvent) => {
+                resolve(trackEvent.streams[0]);
+            };
+        });
+        setTimeout(() => reject("Test timed out"), 5000);
+    });
+
+    assert_equals(unicodeString, remoteStream.getVideoTracks()[0].id);
+    video.srcObject = remoteStream;
+    return video.play();
+}, "Testing video exchange with UTF-8 track id");
+        </script>
+    </body>
+</html>
index b899f37..8ef9f82 100644 (file)
@@ -1,3 +1,44 @@
+2018-04-11  Youenn Fablet  <youenn@apple.com>
+
+        Preventively expect UTF8 strings from libwebrtc SDP and error messages
+        https://bugs.webkit.org/show_bug.cgi?id=184509
+
+        Reviewed by Eric Carlson.
+
+        Make WebCore code expect any libwebrtc string to contain UTF-8.
+        Currently SDPs do not contain any UTF-8 specific character
+        but https://tools.ietf.org/html/rfc4566 allows it.
+
+        Add Internals API to set track id so that we can inject UTF-8 inside some WebRTC tests.
+        Test: webrtc/utf8-sdp.html
+
+        * Modules/mediastream/MediaStreamTrack.h:
+        (WebCore::MediaStreamTrack::setIdForTesting):
+        * Modules/mediastream/libwebrtc/LibWebRTCMediaEndpoint.cpp:
+        (WebCore::fromStdString):
+        (WebCore::fromSessionDescription):
+        (WebCore::LibWebRTCMediaEndpoint::doSetLocalDescription):
+        (WebCore::LibWebRTCMediaEndpoint::doSetRemoteDescription):
+        (WebCore::LibWebRTCMediaEndpoint::mediaStreamFromRTCStream):
+        (WebCore::LibWebRTCMediaEndpoint::addRemoteTrack):
+        (WebCore::LibWebRTCMediaEndpoint::addDataChannel):
+        (WebCore::LibWebRTCMediaEndpoint::OnIceCandidate):
+        (WebCore::LibWebRTCMediaEndpoint::createSessionDescriptionSucceeded):
+        (WebCore::LibWebRTCMediaEndpoint::createSessionDescriptionFailed):
+        (WebCore::LibWebRTCMediaEndpoint::setLocalSessionDescriptionFailed):
+        (WebCore::LibWebRTCMediaEndpoint::setRemoteSessionDescriptionFailed):
+        (WebCore::trackId): Deleted.
+        * Modules/mediastream/libwebrtc/LibWebRTCPeerConnectionBackend.cpp:
+        (WebCore::fromStdString):
+        (WebCore::LibWebRTCPeerConnectionBackend::doAddIceCandidate):
+        (WebCore::createReceiverForSource):
+        * platform/mediastream/MediaStreamTrackPrivate.h:
+        (WebCore::MediaStreamTrackPrivate::setIdForTesting):
+        * testing/Internals.cpp:
+        (WebCore::Internals::setMediaStreamTrackIdentifier):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2018-04-11  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Cache glyphs (using display lists) when painting at high frequency
index 73ea57c..da1b1c0 100644 (file)
@@ -138,6 +138,8 @@ public:
     // ActiveDOMObject API.
     bool hasPendingActivity() const final;
 
+    void setIdForTesting(String&& id) { m_private->setIdForTesting(WTFMove(id)); }
+
 protected:
     MediaStreamTrack(ScriptExecutionContext&, Ref<MediaStreamTrackPrivate>&&);
 
index b105752..415ef97 100644 (file)
 
 namespace WebCore {
 
+static inline String fromStdString(const std::string& value)
+{
+    return String::fromUTF8(value.data(), value.length());
+}
+
 LibWebRTCMediaEndpoint::LibWebRTCMediaEndpoint(LibWebRTCPeerConnectionBackend& peerConnection, LibWebRTCProvider& client)
     : m_peerConnectionBackend(peerConnection)
     , m_peerConnectionFactory(*client.factory())
@@ -116,9 +121,8 @@ static inline RefPtr<RTCSessionDescription> fromSessionDescription(const webrtc:
 
     std::string sdp;
     description->ToString(&sdp);
-    String sdpString(sdp.data(), sdp.size());
 
-    return RTCSessionDescription::create(fromSessionDescriptionType(*description), WTFMove(sdpString));
+    return RTCSessionDescription::create(fromSessionDescriptionType(*description), fromStdString(sdp));
 }
 
 // FIXME: We might want to create a new object only if the session actually changed for all description getters.
@@ -160,8 +164,7 @@ void LibWebRTCMediaEndpoint::doSetLocalDescription(RTCSessionDescription& descri
     std::unique_ptr<webrtc::SessionDescriptionInterface> sessionDescription(webrtc::CreateSessionDescription(sessionDescriptionType(description.type()), description.sdp().utf8().data(), &error));
 
     if (!sessionDescription) {
-        String errorMessage(error.description.data(), error.description.size());
-        m_peerConnectionBackend.setLocalDescriptionFailed(Exception { OperationError, WTFMove(errorMessage) });
+        m_peerConnectionBackend.setLocalDescriptionFailed(Exception { OperationError, fromStdString(error.description) });
         return;
     }
 
@@ -181,8 +184,7 @@ void LibWebRTCMediaEndpoint::doSetRemoteDescription(RTCSessionDescription& descr
     webrtc::SdpParseError error;
     std::unique_ptr<webrtc::SessionDescriptionInterface> sessionDescription(webrtc::CreateSessionDescription(sessionDescriptionType(description.type()), description.sdp().utf8().data(), &error));
     if (!sessionDescription) {
-        String errorMessage(error.description.data(), error.description.size());
-        m_peerConnectionBackend.setRemoteDescriptionFailed(Exception { OperationError, WTFMove(errorMessage) });
+        m_peerConnectionBackend.setRemoteDescriptionFailed(Exception { OperationError, fromStdString(error.description) });
         return;
     }
     m_backend->SetRemoteDescription(&m_setRemoteSessionDescriptionObserver, sessionDescription.release());
@@ -303,11 +305,6 @@ LibWebRTCMediaEndpoint::StatsCollector::StatsCollector(Ref<LibWebRTCMediaEndpoin
         m_id = track->id();
 }
 
-static inline String fromStdString(const std::string& value)
-{
-    return String(value.data(), value.length());
-}
-
 static inline void fillRTCStats(RTCStatsReport::Stats& stats, const webrtc::RTCStats& rtcStats)
 {
     stats.timestamp = Performance::reduceTimeResolution(Seconds::fromMicroseconds(rtcStats.timestamp_us())).milliseconds();
@@ -618,16 +615,11 @@ void LibWebRTCMediaEndpoint::OnSignalingChange(webrtc::PeerConnectionInterface::
     });
 }
 
-static inline String trackId(webrtc::MediaStreamTrackInterface& videoTrack)
-{
-    return String(videoTrack.id().data(), videoTrack.id().size());
-}
-
 MediaStream& LibWebRTCMediaEndpoint::mediaStreamFromRTCStream(webrtc::MediaStreamInterface& rtcStream)
 {
     auto mediaStream = m_streams.ensure(&rtcStream, [&rtcStream, this] {
         auto label = rtcStream.label();
-        auto stream = MediaStream::create(*m_peerConnectionBackend.connection().scriptExecutionContext(), MediaStreamPrivate::create({ }, String(label.data(), label.size())));
+        auto stream = MediaStream::create(*m_peerConnectionBackend.connection().scriptExecutionContext(), MediaStreamPrivate::create({ }, fromStdString(label)));
         auto streamPointer = stream.ptr();
         m_peerConnectionBackend.addRemoteStream(WTFMove(stream));
         return streamPointer;
@@ -667,7 +659,7 @@ void LibWebRTCMediaEndpoint::addRemoteTrack(rtc::scoped_refptr<webrtc::RtpReceiv
         return;
     case cricket::MEDIA_TYPE_AUDIO: {
         rtc::scoped_refptr<webrtc::AudioTrackInterface> audioTrack = static_cast<webrtc::AudioTrackInterface*>(rtcTrack);
-        auto audioReceiver = m_peerConnectionBackend.audioReceiver(trackId(*rtcTrack));
+        auto audioReceiver = m_peerConnectionBackend.audioReceiver(fromStdString(rtcTrack->id()));
 
         receiver = WTFMove(audioReceiver.receiver);
         audioReceiver.source->setSourceTrack(WTFMove(audioTrack));
@@ -675,7 +667,7 @@ void LibWebRTCMediaEndpoint::addRemoteTrack(rtc::scoped_refptr<webrtc::RtpReceiv
     }
     case cricket::MEDIA_TYPE_VIDEO: {
         rtc::scoped_refptr<webrtc::VideoTrackInterface> videoTrack = static_cast<webrtc::VideoTrackInterface*>(rtcTrack);
-        auto videoReceiver = m_peerConnectionBackend.videoReceiver(trackId(*rtcTrack));
+        auto videoReceiver = m_peerConnectionBackend.videoReceiver(fromStdString(rtcTrack->id()));
 
         receiver = WTFMove(videoReceiver.receiver);
         videoReceiver.source->setSourceTrack(WTFMove(videoTrack));
@@ -766,7 +758,7 @@ void LibWebRTCMediaEndpoint::addDataChannel(rtc::scoped_refptr<webrtc::DataChann
     init.ordered = dataChannel->ordered();
     init.maxPacketLifeTime = dataChannel->maxRetransmitTime();
     init.maxRetransmits = dataChannel->maxRetransmits();
-    init.protocol = String(protocol.data(), protocol.size());
+    init.protocol = fromStdString(protocol);
     init.negotiated = dataChannel->negotiated();
     init.id = dataChannel->id();
 
@@ -774,7 +766,7 @@ void LibWebRTCMediaEndpoint::addDataChannel(rtc::scoped_refptr<webrtc::DataChann
 
     auto handler =  std::make_unique<LibWebRTCDataChannelHandler>(WTFMove(dataChannel));
     ASSERT(m_peerConnectionBackend.connection().scriptExecutionContext());
-    auto channel = RTCDataChannel::create(*m_peerConnectionBackend.connection().scriptExecutionContext(), WTFMove(handler), String(label.data(), label.size()), WTFMove(init));
+    auto channel = RTCDataChannel::create(*m_peerConnectionBackend.connection().scriptExecutionContext(), WTFMove(handler), fromStdString(label), WTFMove(init));
 
     if (isOpened) {
         callOnMainThread([channel = channel.copyRef()] {
@@ -872,17 +864,13 @@ void LibWebRTCMediaEndpoint::OnIceCandidate(const webrtc::IceCandidateInterface
 
     std::string sdp;
     rtcCandidate->ToString(&sdp);
-    String candidateSDP(sdp.data(), sdp.size());
-
-    auto mid = rtcCandidate->sdp_mid();
-    String candidateMid(mid.data(), mid.size());
 
     auto sdpMLineIndex = safeCast<unsigned short>(rtcCandidate->sdp_mline_index());
 
-    callOnMainThread([protectedThis = makeRef(*this), mid = WTFMove(candidateMid), sdp = WTFMove(candidateSDP), sdpMLineIndex] {
+    callOnMainThread([protectedThis = makeRef(*this), mid = fromStdString(rtcCandidate->sdp_mid()), sdp = fromStdString(sdp), sdpMLineIndex]() mutable {
         if (protectedThis->isStopped())
             return;
-        protectedThis->m_peerConnectionBackend.newICECandidate(String(sdp), String(mid), sdpMLineIndex);
+        protectedThis->m_peerConnectionBackend.newICECandidate(WTFMove(sdp), WTFMove(mid), sdpMLineIndex);
     });
 }
 
@@ -895,22 +883,20 @@ void LibWebRTCMediaEndpoint::createSessionDescriptionSucceeded(std::unique_ptr<w
 {
     std::string sdp;
     description->ToString(&sdp);
-    String sdpString(sdp.data(), sdp.size());
 
-    callOnMainThread([protectedThis = makeRef(*this), sdp = WTFMove(sdpString)] {
+    callOnMainThread([protectedThis = makeRef(*this), sdp = fromStdString(sdp)]() mutable {
         if (protectedThis->isStopped())
             return;
         if (protectedThis->m_isInitiator)
-            protectedThis->m_peerConnectionBackend.createOfferSucceeded(String(sdp));
+            protectedThis->m_peerConnectionBackend.createOfferSucceeded(WTFMove(sdp));
         else
-            protectedThis->m_peerConnectionBackend.createAnswerSucceeded(String(sdp));
+            protectedThis->m_peerConnectionBackend.createAnswerSucceeded(WTFMove(sdp));
     });
 }
 
 void LibWebRTCMediaEndpoint::createSessionDescriptionFailed(const std::string& errorMessage)
 {
-    String error(errorMessage.data(), errorMessage.size());
-    callOnMainThread([protectedThis = makeRef(*this), error = WTFMove(error)] () mutable {
+    callOnMainThread([protectedThis = makeRef(*this), error = fromStdString(errorMessage)] () mutable {
         if (protectedThis->isStopped())
             return;
         if (protectedThis->m_isInitiator)
@@ -931,8 +917,7 @@ void LibWebRTCMediaEndpoint::setLocalSessionDescriptionSucceeded()
 
 void LibWebRTCMediaEndpoint::setLocalSessionDescriptionFailed(const std::string& errorMessage)
 {
-    String error(errorMessage.data(), errorMessage.size());
-    callOnMainThread([protectedThis = makeRef(*this), error = WTFMove(error)] () mutable {
+    callOnMainThread([protectedThis = makeRef(*this), error = fromStdString(errorMessage)] () mutable {
         if (protectedThis->isStopped())
             return;
         protectedThis->m_peerConnectionBackend.setLocalDescriptionFailed(Exception { OperationError, WTFMove(error) });
@@ -950,8 +935,7 @@ void LibWebRTCMediaEndpoint::setRemoteSessionDescriptionSucceeded()
 
 void LibWebRTCMediaEndpoint::setRemoteSessionDescriptionFailed(const std::string& errorMessage)
 {
-    String error(errorMessage.data(), errorMessage.size());
-    callOnMainThread([protectedThis = makeRef(*this), error = WTFMove(error)] () mutable {
+    callOnMainThread([protectedThis = makeRef(*this), error = fromStdString(errorMessage)] () mutable {
         if (protectedThis->isStopped())
             return;
         protectedThis->m_peerConnectionBackend.setRemoteDescriptionFailed(Exception { OperationError, WTFMove(error) });
index 429c781..31d22b8 100644 (file)
@@ -209,8 +209,7 @@ void LibWebRTCPeerConnectionBackend::doAddIceCandidate(RTCIceCandidate& candidat
     std::unique_ptr<webrtc::IceCandidateInterface> rtcCandidate(webrtc::CreateIceCandidate(candidate.sdpMid().utf8().data(), sdpMLineIndex, candidate.candidate().utf8().data(), &error));
 
     if (!rtcCandidate) {
-        String message(error.description.data(), error.description.size());
-        addIceCandidateFailed(Exception { OperationError, WTFMove(message) });
+        addIceCandidateFailed(Exception { OperationError, String::fromUTF8(error.description.data(), error.description.length()) });
         return;
     }
 
@@ -237,8 +236,7 @@ void LibWebRTCPeerConnectionBackend::addVideoSource(Ref<RealtimeOutgoingVideoSou
 
 static inline Ref<RTCRtpReceiver> createReceiverForSource(ScriptExecutionContext& context, Ref<RealtimeMediaSource>&& source)
 {
-    String id = source->id();
-    auto remoteTrackPrivate = MediaStreamTrackPrivate::create(WTFMove(source), WTFMove(id));
+    auto remoteTrackPrivate = MediaStreamTrackPrivate::create(WTFMove(source), String { source->id() });
     auto remoteTrack = MediaStreamTrack::create(context, WTFMove(remoteTrackPrivate));
 
     return RTCRtpReceiver::create(WTFMove(remoteTrack));
index ffc58f7..3a816fd 100644 (file)
@@ -100,6 +100,8 @@ public:
     enum class ReadyState { None, Live, Ended };
     ReadyState readyState() const { return m_readyState; }
 
+    void setIdForTesting(String&& id) { m_id = WTFMove(id); }
+
 private:
     MediaStreamTrackPrivate(Ref<RealtimeMediaSource>&&, String&& id);
 
index 93bdca0..bdadc28 100644 (file)
@@ -4286,6 +4286,10 @@ void Internals::simulateMediaStreamTrackCaptureSourceFailure(MediaStreamTrack& t
     track.source().captureFailed();
 }
 
+void Internals::setMediaStreamTrackIdentifier(MediaStreamTrack& track, String&& id)
+{
+    track.setIdForTesting(WTFMove(id));
+}
 #endif
 
 String Internals::audioSessionCategory() const
index fd5f20d..00b21b9 100644 (file)
@@ -622,6 +622,7 @@ public:
     void setMediaStreamTrackMuted(MediaStreamTrack&, bool);
     void removeMediaStreamTrack(MediaStream&, MediaStreamTrack&);
     void simulateMediaStreamTrackCaptureSourceFailure(MediaStreamTrack&);
+    void setMediaStreamTrackIdentifier(MediaStreamTrack&, String&& id);
 #endif
 
     String audioSessionCategory() const;
index 7844647..9771e80 100644 (file)
@@ -566,6 +566,7 @@ enum EventThrottlingBehavior {
     [Conditional=MEDIA_STREAM] void setMediaStreamTrackMuted(MediaStreamTrack track, boolean muted);
     [Conditional=MEDIA_STREAM] void removeMediaStreamTrack(MediaStream stream, MediaStreamTrack track);
     [Conditional=MEDIA_STREAM] void simulateMediaStreamTrackCaptureSourceFailure(MediaStreamTrack track);
+    [Conditional=MEDIA_STREAM] void setMediaStreamTrackIdentifier(MediaStreamTrack track, DOMString identifier);
 
     Promise<void> clearCacheStorageMemoryRepresentation();
     Promise<DOMString> cacheStorageEngineRepresentation();