AMWA-TV / is-06

AMWA IS-06 NMOS Network Control Specification (Deprecated)
https://specs.amwa.tv/is-06
Apache License 2.0
14 stars 10 forks source link

Fix bandwidth/speed regex #31

Closed garethsb closed 4 years ago

garethsb commented 5 years ago

Related to #24.

Regex for these properties is currently: ^(?:0.|[1-9][0-9]*[.])?[0-9]+[kMG]bit\/s$

This seems to be intended to allow integer or decimal numbers with no leading zeros. However, all of the following are allowed:

Is that intentional, especially allowing leading zeros on integer values but not on decimal values?

And a minor note: Other NMOS specs do not use ?: to indicate non-capturing groups.

More consistent with the JSON number format would be: ^(0|[1-9][0-9]*)(\.[0-9]+)?[kMG]bit\/s$ (which allows all of the above, except for the example with leading zeros)

andrewbonney commented 5 years ago

I can't imagine that any existing implementation actually uses the leading zero representation, even if the schemas permit it. I'd hope that could be resolved via a spec bug fix. The trailing zeros (more than one) also seem unlikely but it would be good to confirm that with any known implementations.

As for the difference between 100 and 100.0, this seems harder to deal with via a regex alone. I'm not sure if there are cases which require representation of unusual fractions, but ideally I'd have thought it preferable to use '100Mbit/s' over '0.1Gbit/s' and to do away with the decimal point altogether for consistency. Fixing this without breaking changes seems hard, but if a convention for using integer representations only could be introduced that might help to avoid implementation errors.

garethsb commented 4 years ago

Proposal: adjust the regex to prohibit leading zeroes, and add a recommendation to use integer values without a decimal point, with the largest unit that allows that.

I.e.

With a note that explains that basic query syntax uses exact string matching, as per https://github.com/AMWA-TV/nmos-network-control/issues/24#issuecomment-557479382