RTCSessionDescription: sdp is not a read-only attribute
Reported by
fi...@appear.in,
Nov 7 2016
|
||||||
Issue description
UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/53.0.2785.143 Chrome/53.0.2785.143 Safari/537.36
Steps to reproduce the problem:
1. try this:
var desc = new RTCSessionDescription({ type: 'offer', sdp: "v=0\no=- 6276735615230473072 2 IN IP4 127.0.0.1\ns=-\nt=0 0\na=group:BUNDLE audio video\na=msid-semantic: WMS *\na=ice-ufrag:nGx0Ag==\na=ice-pwd:MEA9YrgtlxtSZ3/M69h/oc3Svvs2KA==\na=ice-lite\na=setup:actpass\na=fingerprint:sha-256 05:EC:AE:05:A1:EF:7F:13:8A:7D:E3:C6:05:67:E3:CE:8D:16:6B:7A:78:92:58:F9:18:FC:FB:84:4A:5D:3C:F6\nm=audio 9 UDP/TLS/RTP/SAVPF 111\nc=IN IP4 0.0.0.0\na=rtcp:9 IN IP4 0.0.0.0\na=mid:audio\na=extmap:3 http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time\na=maxptime:60\na=recvonly\na=rtcp-mux\na=rtpmap:111 opus/48000/2\na=fmtp:111 minptime=10;useinbandfec=1\nm=video 9 UDP/TLS/RTP/SAVPF 100\nc=IN IP4 0.0.0.0\na=rtcp:9 IN IP4 0.0.0.0\na=mid:video\na=extmap:2 urn:ietf:params:rtp-hdrext:toffset\na=extmap:3 http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time\na=extmap:4 urn:3gpp:video-orientation\na=recvonly\na=rtcp-mux\na=rtpmap:100 VP8/90000\na=rtcp-fb:100 ccm fir\na=rtcp-fb:100 nack\na=rtcp-fb:100 nack pli\na=rtcp-fb:100 goog-remb\n" });
desc.sdp = 'something else';
What is the expected behavior?
this should fail since the sdp is defined as read-only in the latest spec:
http://w3c.github.io/webrtc-pc/#rtcsessiondescription-class
What went wrong?
The sdp is not read-only in Chrome here (thanks hta@): https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/modules/peerconnection/RTCSessionDescription.idl?q=RTCSessionDescription+idl&sq=package:chromium&dr=C&l=43
Did this work before? N/A
Does this work in other browsers? N/A
Chrome version: 53.0.2785.143 Channel: n/a
OS Version:
Flash Version: Shockwave Flash 16.0 r0
originally from webrtcinwebkit @ https://twitter.com/pnormand/status/795538925012008960
,
Nov 8 2016
,
Nov 8 2016
The IDL of both Blink and Gecko are the same, and disagree with the spec: https://github.com/mozilla/gecko-dev/blob/master/dom/webidl/RTCSessionDescription.webidl [Constructor ...] interface RTCSessionDescription { attribute RTCSdpType? type; attribute DOMString? sdp; serializer = {attribute}; // in Blink, jsonifier in Gecko };
,
Nov 8 2016
The spec change was in https://github.com/w3c/webrtc-pc/issues/573
,
Nov 8 2016
WebKit and Edge also have it read-write, but nonullable. https://developer.microsoft.com/en-us/microsoft-edge/platform/documentation/apireference/interfaces/rtcsessiondescription/ https://github.com/WebKit/webkit/blob/master/Source/WebCore/Modules/mediastream/RTCSessionDescription.idl
,
Nov 8 2016
fippo@, do you plan to change appear.in while the discussion is ongoing? It's possible that appear.in is the biggest user of the setter, in which case it'd be nice to actually match the spec.
,
Nov 8 2016
foolip: I already fixed the offending code on our side yesterday ;-) Moving forward I plan to longer use RTCSessionDescription directly, pass in JS objects to SRD et al and let adapter.js do the heavy lifting. Firefox is tracking this in https://bugzilla.mozilla.org/show_bug.cgi?id=1313966
,
Nov 8 2016
WebKit is using readonly as requested by the spec, that is why we found the issue in the first place:
...
[
Conditional=WEB_RTC,
Constructor(Dictionary dictionary),
ConstructorMayThrowException,
ImplementationLacksVTable,
PrivateIdentifier,
PublicIdentifier,
] interface RTCSessionDescription {
[SetterMayThrowException] readonly attribute RTCSdpType type;
readonly attribute DOMString sdp;
serializer = {type, sdp};
};
...
,
Nov 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3fdd951cdccbd48de88767e6156bad40ff52738d commit 3fdd951cdccbd48de88767e6156bad40ff52738d Author: foolip <foolip@chromium.org> Date: Tue Nov 08 18:29:24 2016 Measure usage of RTCSessionDescription's type and sdp attributes Also measure missing members in the init dict, that is how the members would end up as null, and will show how often adding required would throw a TypeError due to missing type. BUG=662898 R=guidou@chromium.org Review-Url: https://codereview.chromium.org/2490543002 Cr-Commit-Position: refs/heads/master@{#430662} [modify] https://crrev.com/3fdd951cdccbd48de88767e6156bad40ff52738d/third_party/WebKit/Source/core/frame/UseCounter.h [modify] https://crrev.com/3fdd951cdccbd48de88767e6156bad40ff52738d/third_party/WebKit/Source/modules/peerconnection/RTCSessionDescription.cpp [modify] https://crrev.com/3fdd951cdccbd48de88767e6156bad40ff52738d/third_party/WebKit/Source/modules/peerconnection/RTCSessionDescription.h [modify] https://crrev.com/3fdd951cdccbd48de88767e6156bad40ff52738d/third_party/WebKit/Source/modules/peerconnection/RTCSessionDescription.idl [modify] https://crrev.com/3fdd951cdccbd48de88767e6156bad40ff52738d/third_party/WebKit/Source/modules/peerconnection/RTCSessionDescriptionInit.idl [modify] https://crrev.com/3fdd951cdccbd48de88767e6156bad40ff52738d/tools/metrics/histograms/histograms.xml
,
Nov 8 2016
Note to self: when looking at the use counters, if the conclusion is that the change cannot fly, remember to also revisit https://github.com/w3c/webrtc-pc/issues/922
,
Nov 23 2016
,
Mar 13 2018
,
Mar 14 2018
,
Mar 14 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by guidou@chromium.org
, Nov 7 2016Status: Assigned (was: Unconfirmed)