w3c/webrtc-pc

Consider making RTCIceCandidatePair an interface

Closed this issue · 5 comments

jan-ivar commented

Currently, RTCIceCandidatePair is a dictionary containing two interfaces:

dictionary RTCIceCandidatePair {
  RTCIceCandidate local;
  RTCIceCandidate remote;
};

This seems like a mistake, allowing JS to compose invalid pairs and pass them as inputs.

Valid pairs are formed exclusively by the ICE Agent, a reality better expressed by a more restrictive contract:

[Exposed=Window]
interface RTCIceCandidatePair {
  readonly attribute RTCIceCandidate local;
  readonly attribute RTCIceCandidate remote;
};

This would benefit web developers by enforcing an invariant, and spec writers by improved WebIDL type checking, simpler algorithms, and even simpler APIs in the future.

I believe this should be mostly backwards compatible since all APIs that take pairs expect valid ones. cc @sam-vi

I propose we make this change.

jan-ivar commented

@sam-vi thoughts on this? Should we discuss it at the next meeting perhaps?

foolip commented

This dictionary is also used as a method argument in webrtc-extensions. How would that be affected?

jan-ivar commented

I think the goal would be for selectCandidatePair and removeCandidatePair to take the interface object (instead of a dictionary with two member interface objects).

The two methods throw NotFoundError on invalid pairs already. Pairs serve as handles to resources effectively.

I didn't find any WPT tests for these methods, so they're hopefully not implemented yet @sam-vi is that right?

jan-ivar commented

Pairs serve as handles to resources effectively.

Not in an owning way, I should say. E.g. we could have used IDs for this instead, but interface objects seem better.

sam-vi commented

@sam-vi thoughts on this? Should we discuss it at the next meeting perhaps?

I agree with this proposal. I will work on some next steps.

This seems like a mistake, allowing JS to compose invalid pairs and pass them as inputs.

I can't be certain about the motivation for the original definition as a dictionary. But I imagine that prior to selectCandidatePair and removeCandidatePair, when the only use of RTCIceCandidatePair was as a return value for getSelectedCandidatePair, a less restrictive definition was adequate and there was no possibility of misuse of a malformed candidate pair.

I think the goal would be for selectCandidatePair and removeCandidatePair to take the interface object (instead of a dictionary with two member interface objects).

+1. With [[CandidatePairs]], it is also possible to avoid pitfalls such as #2906.

I didn't find any WPT tests for these methods, so they're hopefully not implemented yet @sam-vi is that right?

Correct. I recently got back from parental leave. I will continue work on the implementation and next steps for the spec.

  • 👍1