Page MenuHomeVyOS Platform
Create Task
Maniphest T6622

Add support for non-decimal numbers to the numeric validator
Open, LowPublic
Actions

Description

Right now, our numeric validator can only validate decimal numbers. Trying to give it something else will cause errors:

vyos@vyos# /usr/libexec/vyos/validators/numeric 0x999 && echo Valid
'0x999' is not a valid number

There are use cases for hex values, though, like ethertype numbers, connection marks, etc.

OCaml's built-in int_of_string already supports radix prefixes, so we don't need to write anything to support that — it's only our regex validation that doesn't support non-decimal numbers.

utop # int_of_string "0xDEADBEEF" ;;
- : int = 3735928559

However, the UI design is an interesting question. To answer it, we need to find out:

  1. Are there any use cases where a target application allows either decimal or non-decimal numbers, so that 32 and 0x20 are equally acceptable?
  2. Are there any use cases for non-decimal ranges, like 0x20-0x30?
  3. Do we want to allow mixed ranges like 0x20-48?

My feeling is that the base should be orthogonal to everything else, but I'm not sure if we should go for an "allow" or a "require" approach.

If we go for "require", people still can write something like numeric --octal --hex --decimal 0x20 to allow any supported base. If we go for "allow", we may still need "require" options for cases when the number can only be hex, for example.

Details

Difficulty level
Unknown (require assessment)
Version
-
Why the issue appeared?
Will be filled on close
Is it a breaking change?
Perfectly compatible
Issue type
Internal change (not visible to end users)

Event Timeline

dmbaturin created this task.Jul 30 2024, 9:54 AM
talmakion subscribed.Jul 30 2024, 11:34 AM

My opinion is largely based on my recent use-case for this, the GRE-match ethertype fields. I remember grumbling a couple of times about needing to convert bases to decimal in the past, but it's been too long to remember specifically what I was configuring at the time.

  1. I don't see a big drawback to continuing to allow existing decimal values, but also permit radix prefixes in future. So, anywhere we might logically want to allow hex but don't is already a use case. There aren't a huge number, but I spot some all-decimal protocol & ethertype matches in the QoS tree with a quick grep.
  2. I don't have any specific use cases for ranges but if it's not difficult and can already be done for decimal, we'll likely find some. There might be someone who enjoys using hex for DSCP or ToS range matches?
  3. Mixed ranges might miss genuine errors (missing a prefix by accident). It seems neater to have it available as an option, but I can't think of a scenario where it'd make good sense in front of a user.

I'd go straight for a "require", defaulting to decimal and allowing permissible radices to be combined as you've suggested. Maybe even a --multi-radix convenience option. I think "require" provides the best combo of flexibility and safety. There's a ton of code that either tries to cast config tree strings to ints assuming base 10, or emit them directly into templated configs and shell commands. nft and most of iproute2 probably won't mind, but nginx might not appreciate an octal port binding if someone gets creative. We can probably refactor or wrap integer type casts everywhere, but that plus a bulk template retrofit seems like a lot of work for not much payoff.

My suggestion for allowing it in the validator is around common usage and/or convenience - we can selectively permit it where the underlying can definitely handle it, because someone might want to use it. For eg, bitfields are easier to eyeball in hex than decimal and ethertypes are commonly written in hex.