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

Validate arguments passed to Client.* functions #95

Closed
arkgil opened this issue Apr 21, 2017 · 2 comments
Closed

Validate arguments passed to Client.* functions #95

arkgil opened this issue Apr 21, 2017 · 2 comments
Milestone

Comments

@arkgil
Copy link
Contributor

arkgil commented Apr 21, 2017

Right now there are not validations, so passing bad arguments results in crash of client process in most cases. We could also look into narrowing the scope of Client.ip, or create the new type for IPv4 addresses, because we don't support IPv6 TURN extension (yet?).

@arkgil arkgil added the TURN label Apr 21, 2017
@arkgil arkgil changed the title Validate arguments passed to Cleint.* functions Validate arguments passed to Client.* functions Apr 21, 2017
@arkgil arkgil added enhancement and removed TURN labels May 5, 2017
@arkgil arkgil added this to the v0.2.0 milestone May 5, 2017
@arkgil
Copy link
Contributor Author

arkgil commented May 8, 2017

Hm, I'm wondering if its worth the effort. Channels seem to be more important feature.
/cc @rslota @erszcz

@erszcz
Copy link
Member

erszcz commented May 8, 2017

create the new type for IPv4 addresses

The distinction between IPv4 and IPv6 is never going to disappear, so it's OK to resemble that in the types we use and specify which functions can operate on both (probably public / closer to the public interface) and which are specific to one of them (the underpinnings of the interface).

Regarding prioritisation, IMO specific types (visible in the function signatures) and crashes on misuse are good enough, while validation is nice, of course, but not strictly necessary.

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

2 participants