Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lack of validation in RTCIceCandidate constructor #1165

Closed
soareschen opened this issue May 5, 2017 · 6 comments
Closed

Lack of validation in RTCIceCandidate constructor #1165

soareschen opened this issue May 5, 2017 · 6 comments
Assignees

Comments

@soareschen
Copy link
Contributor

There is no validation requirement for the arguments inside RTCIceCandidate constructor other than that sdpMid and sdpMLineIndex must not be both null. This seems to imply that the values in RTCIceCandidateInit can be anything as long as they are of valid type.

I suspect that the rationale for this is that the actual validation is only done when the candidate is passed to addIceCandidate. If so it is helpful to put a note in the RTCIceCandidate constructor section.

I also have difficulty understanding the use of the other fields in RTCIceCandidate that are not in RTCIceCandidateInit. Since these fields are read only there doesn't seem to have way to set their values at all. There is also no place in the spec where these fields are being read, including addIceCandidate.

@foolip
Copy link
Member

foolip commented May 5, 2017

I also filed #1166

@stefhak
Copy link
Contributor

stefhak commented May 11, 2017

If adding a note is enough then this is editorial.

@foolip
Copy link
Member

foolip commented May 12, 2017

"There is also no place in the spec where these fields are being read, including addIceCandidate" is not an editorial issue.

@rwaldron
Copy link

It struck me as odd that:

sdpMid of type DOMString, readonly, nullable
sdpMLineIndex of type unsigned short, readonly, nullable

That is: both are nullable, but then...

When run, if both the sdpMid and sdpMLineIndex dictionary members are null, throw a TypeError.

@foolip
Copy link
Member

foolip commented May 17, 2017

That's a case where Web IDL syntax can't express the constraints, and so it's done in prose instead. If a single dictionary member is required, the required keyword is used, but if it's more complicated, then things end up looking like this...

@soareschen
Copy link
Contributor Author

This is now fixed by #1229.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants