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

Hi,

On 12/10/2017 18:15, Acee Lindem (acee) wrote:
> Hi Adam,
>
> On 10/12/17, 12:12 PM, "Adam Roach" <adam@nostrum.com> wrote:
>
>> 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]
> Yes - this doesn’t appear to be a complicate fix at all. We’re going to
> get more eyes on it and do some tests with https://yangcatalog.org/yangre/
>   but we should be able to fix this.
For what its worth, and I'm not proposing this is changed now given that 
this has been discussed previously, but I do think that this is another 
example of the dangers inherent with specifying precise, but more 
complex, regular expressions in YANG for checking numerical values.

E.g. I still prefer "[0-9]{0,10}" instead of 
"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".

Yes, it allows some invalid numerical values, which would presumably 
fail when the value is converted to a internal numerical format, but it 
is easier to read/review, and is harder to get wrong.  Besides even if a 
stricter regex validates that it the value is syntactically correct it 
still doesn't mean that the correct intended value has been used ...

>
>> ____
>>
>> 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))|'
>
> Although this is a somewhat controversial subject, we used “[0-9]" for
> portability for implementations using non-standard regular expression
> parsers.
The XML regex language that is reused by YANG is built for Unicode 
support.  Hence '\d' matches all digit characters in all scripts, i.e. 
it actually matches 500+ characters rather than just the ASCII digits 
0-9, and in most network configuration models that is probably not what 
is wanted.  I'm not sure whether it matters that \d matches a much 
larger class of characters, but at least with [0-9] it is entirely 
unambiguous.

Thanks,
Rob

>   
>
> Thanks,
> Acee
>
>>
>> /a
> _______________________________________________
> rtgwg mailing list
> rtgwg@ietf.org
> https://www.ietf.org/mailman/listinfo/rtgwg