[webrtc-pc] Clarify or fix racy peerIdentity validation failure (#2148)

jan-ivar has just created a new issue for https://github.com/w3c/webrtc-pc:

== Clarify or fix racy peerIdentity validation failure ==
I was reviewing [some identity code](https://github.com/web-platform-tests/wpt/blob/master/webrtc-identity/RTCPeerConnection-peerIdentity.html#L75) I think is racy on failure:
```js
const offer = await pc1.createOffer();
await pc2.setRemoteDescription(offer);
const identityAssertion = await pc2.peerIdentity; // hangs here on validation failure
```
...because `pc2` in this instance has no [target peer identity](http://w3c.github.io/webrtc-pc/#target-peer-identity) before [SRD](http://w3c.github.io/webrtc-pc/#dom-peerconnection-setremotedescription):

> *"If a target peer identity is set, then the identity validation MUST be completed before the promise returned setRemoteDescription is resolved. If identity validation fails, then the promise returned by setRemoteDescription is rejected.*
> 
> *If there is no target peer identity, then setRemoteDescription does not await the completion of identity validation."*

I read these two paragraphs to mean: when there's no target peer identity, then if identity validation fails, the promise returned by setRemoteDescription is **not** rejected.

**We should probably say that explicitly in the second paragraph, and not rely on people inferring this from *"not await the completion of"*.**

This is important, because when the [peerIdentity](https://w3c.github.io/webrtc-identity/identity.html#dom-rtcpeerconnection-peeridentity) *"promise is rejected, a new unresolved value is created, unless a target peer identity has been established."*

This means that if validation fails *before* SRD completes, then `pc2.peerIdentity` will have been set to a new unresolved value by the time we `await` it, and we hang forever.

The fix is:
```js
const offer = await pc1.createOffer();
const p = pc2.peerIdentity;
await pc2.setRemoteDescription(offer);
const identityAssertion = await p;
```
This seems like a bit of a footgun, but there's precedence here with the `track` event which also fires before SRD completes. Just filing this issue so we're all on the same page that this is the behavior we intended.

I'll provide a PR with the spec clarification. The wpt test is being fixed in [bug 1534692](https://bugzilla.mozilla.org/show_bug.cgi?id=1534692).

Please view or discuss this issue at https://github.com/w3c/webrtc-pc/issues/2148 using your GitHub account

Received on Thursday, 28 March 2019 04:19:53 UTC