Re: Adam Roach's No Objection on draft-ietf-rtgwg-routing-types-16: (with COMMENT)

[Adam Roach <[e-mail]>]

On 10/11/17 20:16, Acee Lindem (acee) wrote:
>
>> ____
>>
>> There are several patterns in the YANG definition that perform significant
>> restriction of numbers (e.g., to ensure they don't fall outside the range
>> that
>> can be stored in 16 or 32 bits). In many cases, these patterns include the
>> ability to zero-prefix some (but not all) decimal values. For example, the
>> production for route-origin would allow leading zeros in "2:0100:0555"
>> but not
>> in "2:04294967295:065535" (even though "2:4294967295:65535" is okay). I
>> don't
>> know offhand whether it makes sense to allow leading zeros in these
>> fields, but
>> I would argue that the production should be consistent in allowing or
>> disallowing them. This issue arises in various forms in route-target,
>> ipv6-route-target, route-origin, and ipv6-route-origin.
> We’ll look at this and get back to you - a lot of time has already gone
> into formulating and testing these patterns.


Yes, and it would be a shame if that work resulted in publishing 
patterns with known issues.

This flaw arises in three formulations (each of which appear multiple 
times), and would be quite easy to fix. These fixes should be obvious by 
inspection.

32 bits (0-4,294,967,295)
   Replace: [0-3]?[0-9]{0,8}[0-9]
   With:    [1-3][0-9]{9}|[1-9][0-9]{0,8}|0

16 bits (0-65535)
   Replace: [0-5]?[0-9]{0,3}[0-9]
   With:    [1-5][0-9]{4}|[1-9][0-9]{0,3}|0

8 bits (0-255)
   Replace: [01]?[0-9]?[0-9]
   With:    1[0-9]{2}|[1-9]?[0-9]

____

As an aside: replacing "[0-9]" with "\d" everywhere would make these 
patterns easier to read in general, but this is merely a readability 
improvement rather than a bug fix. Compare:

          + '(2:(429496729[0-5]|42949672[0-8][0-9]|'
          +     '4294967[01][0-9]{2}|'
          +     '429496[0-6][0-9]{3}|42949[0-5][0-9]{4}|'
          +     '4294[0-8][0-9]{5}|'
          + '429[0-3][0-9]{6}|42[0-8][0-9]{7}|4[01][0-9]{8}|'
          +     '[1-3][0-9]{9}|[1-9][0-9]{0,8}|0):'
          +     '(6553[0-5]|655[0-2][0-9]|65[0-4][0-9]{2}|'
          +     '6[0-4][0-9]{3}|'
          +     '[1-5][0-9]{4}|[1-9][0-9]{0,3}|0))|'

Becomes:

          + '(2:(429496729[0-5]|42949672[0-8]\d|'
          +     '4294967[01]\d{2}|'
          +     '429496[0-6]\d{3}|42949[0-5]\d{4}|'
          +     '4294[0-8]\d{5}|'
          +     '429[0-3]\d{6}|42[0-8]\d{7}|4[01]\d{8}|'
          +     '[1-3]\d{9}|[1-9]\d{0,8}|0):'
          +     '(6553[0-5]|655[0-2]\d|65[0-4]\d{2}|'
          +     '6[0-4]\d{3}|'
          +     '[1-5]\d{4}|[1-9]\d{0,3}|0))|'


/a