Webrtc-pc: Using setConfiguration() to add application certificates to an RTCPeerConnection post-construction?

[soareschen]

This issue is similar to #1447. The last step of setConfiguration saves the provided configuration object to the [[Configuration]] internal slot, overriding the previous values. #1447 covers the fields with default values, so they cannot be unset. But fields like iceServers, peerIdentity and certificates can seemingly be "unset" from the user-visible configuration. For example:

const pc = new RTCPeerConnection({ 
  iceServers: [...], 
  certificates: [...], 
  peerIdentity: ... 
});

pc.setConfiguration({});
const config = pc.getConfiguration();

config.iceServers // undefined
config.certificates // undefined
config.peerIdentity //undefined

Note that this only affects the user-visible configuration. Since the spec lets RTCPeerConnection save the actual configuration internally elsewhere, the "unset" never affects the actual connection.

[soareschen]

A way to fix this is to update the individual fields in the [[Configuration]] dictionary, if the field is set in the provided configuration and update is allowed. This way the other fields cannot get "unset" from the result of getConfiguration().

[aboba]

getConfiguration() says "When this method is called, the user agent must return the RTCConfiguration object stored in the [[ configuration]] internal slot." So why would iceServers, certficates and peerIdentity be undefined?

[soareschen]

The last step of setConfiguration() says:

Store the configuration in the [[configuration]] internal slot.

Which means the object in [[configuration]] gets replaced with the new configuration object passed to setConfiguration().

[aboba]

In the example, setConfiguration() will throw an exception since it is attempting to change parameters (such as certificates) that cannot be modified post-construction. So getConfiguration() will return the original configuration.

[taylor-b]

Unsetting the certs will fail with InvalidModificationError. But if I did:

pc = new RTCPeerConnection( { iceServers: [...] } );
pc.setConfiguration( { iceTransportPolicy: "relay" } );

The ICE servers would get unset. And I don't see a fundamental problem with this; maybe it's a little confusing but it's how the API was designed, and is consistent with how getParameters/setParameters work. You have to pass in the full dictionary every time, not just the things you want to change.

[soareschen]

If that is the intended behavior then that's fine. However note that the certificates in configuration _can_ be "unset" because InvalidModificationError is thrown only if configuration.certificates is set, and certificates is not a required field and have no default value in RTCConfiguration.

If configuration.certificates is set and the set of certificates differs from the ones used when connection was constructed, throw an InvalidModificationError.

[aboba]

@jan-ivar @taylor-b If an RTCPeerConnection is constructed with no certificates supplied by the application, can setConfiguration() be used to add certificates later? The current language seems to allow this.

[taylor-b]

If an RTCPeerConnection is constructed with no certificates supplied by the application, can setConfiguration() be used to add certificates later?

No, it appears to be the opposite. That's what @soareschen is pointing out. The quote is saying "if the certificates in the argument are _set_, and they differ from the existing certificates, throw an exception." That implies an exception will be thrown if the argument is set and there are no existing certificates, assuming a nonempty set "differs" from an empty set.

I didn't realize this. This "if set" stipulation exists for other attributes, but of course for many it's moot since they have default values. So we need to decide on one API model or the other; do unset arguments represent "change to default value", or "don't change anything"? If we decide on the former, we need to remove these "if set" stipulations. If we decide on the latter, we need to remove the default values.

I think it's likely the intent was "unset arguments mean don't change anything", and we just didn't realize the default values were a problem. The "if set" condition started with the certificates field, and it was extended to other fields that had default values in this PR: https://github.com/w3c/webrtc-pc/pull/861

[jan-ivar]

I think it's likely the intent was "unset arguments mean don't change anything"

@taylor-b Given how the configuration is finally set, I'm unsure. Might it be more that we were tripping over overloading absence with meaning to avoid nullable members?

That would be in keeping with how setParameters and applyConstraints work, which I'm for.

We might also give iceServers and certificates a default value of [].

[aboba]

@stefhak Can we close this?

[stefhak]

@aboba yes, I think we can close.