-----Original Message-----
From: Brian Trammell [mailto:ietf(_at_)trammell(_dot_)ch]
Sent: Wednesday, May 28, 2014 5:22 AM
To: Black, David
Cc: General Area Review Team (gen-art(_at_)ietf(_dot_)org);
ipfix(_at_)ietf(_dot_)org; ietf(_at_)ietf(_dot_)org
Subject: Re: Gen-ART review of draft-ietf-ipfix-text-adt-05
hi David,
Many thanks for the review. Comments and questions inline.
On 24 May 2014, at 04:10, Black, David <david(_dot_)black(_at_)emc(_dot_)com>
wrote:
I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
Please resolve these comments along with any other Last Call comments
you may receive.
Document: draft-ietf-ipfix-text-adt-05
Reviewer: David L. Black
Review Date: May 23, 2014
IETF LC End Date: May 28, 2014
Summary: This draft is on the right track, but has open issues
described in the review.
This is a relatively short draft defining textual representations of
IPFIX data elements. It's clear and easy to read.
I assume that all the ABNF has been checked.
Yep.
The open issues involve use of Unicode.
This is unsurprising. :)
Minor issues:
Section 4.7 string
As Information Elements of the string type are simply UTF-8 encoded
strings, they are represented directly, subject to the escaping and
encoding rules of the Enclosing Context.
There's nothing "simply" about use of UTF-8 encoded strings :-).
Well, no. But most of the string normalization nightmares with UTF-8 are also
problems in the Enclosing Contexts as well, so the assumption here is that if
you're already representing strings in, say, UTF-8 encoded JSON, you're
already paying the UTF-8 tax, so this document presents no additional
considerations.
There is an additional point here, though, that the Enclosing Context may
prefer or require a different encoding than UTF-8, so we should address that:
As Information Elements of the string type are simply Unicode
strings (encoded as UTF-8 when appearing in Data Sets in IPFIX
Messages [RFC 7011]), they are represented directly, using the
Unicode encoding rules and quoting and escaping rules of the
Enclosing Context.
There appear to be no restrictions on Unicode codepoint usage and no
requirements for string normalization or other preparation either in this
draft or RFC 7011.
We largely presume these to be the responsibility of the Enclosing Context (in
this document) and the Metering Process that created the string in the first
place. Neither 7011 nor this representation present additional considerations
here: we just take the Unicode string we get from whoever wants to send/store
it and shovel it out.
I'm not sure it makes sense to enforce normalization in this context.
This can be a formula for all sorts of mischief, so
some warnings about what's possible should be added somewhere - some of
these comments may be raising Unicode concerns in RFC 7011 that would
be better addressed there.
Generally, text-adt is intended to allow the use of the IPFIX Information
Model separately from the protocol described in RFC 7011, so any concern there
should also be raised here.
A general warning about unreliability of Unicode string comparison
is in order. This also applies if an identifier that is not limited
to ASCII characters is substituted for an integer as described in
Section 4.2.
Good point; is there a good cite for this?
In addition, the concerns around visually similar
characters discussed in section 10.5 of the précis framework draft
(draft-ietf-précis-framework) apply; a short summary and pointer
to that section of that draft should suffice.
Section 4.1.5 of the précis framework draft warns against use of mixed-
direction Unicode strings, as "there is currently no widely accepted and
implemented solution for the processing and safe display of mixed-
direction strings." That warning deserves repetition here.
Lots of mischief is possible with non-printing and control characters -
I would expect that the Enclosing Context contains sufficient restrictions
on use of Unicode to deal with most of this concern, and would state that
expectation. This comment is definitely specific to this draft.
Nits/editorial comments:
Section 4.4 float32 and float64
exponent = ( "e" / "E" ) [sign] 1*3DIGIT
Please explain why no more than 3 digits are ever required.
Will do (i.e., because the maximum ranges on the type range from O(1e-3xx) -
O(1e3xx)).
Section 4.8 dateTime*
The '*' in the section title, dateTime* is clever, but it's meaning is not
obvious. I suggest "The dateTime Data Types" as a better section title.
Yep, thanks.
Section 5 Security Considerations
The security considerations for the IPFIX Protocol [RFC7011] apply;
this document presents no additional security considerations.
That's ok, although adding a direct mention of the [UTF8-EXPLOIT] TR
cited in RFC 7011 would be helpful.
Will do.
idnits 2.13.01 warns that the JSON reference (RFC 4627) is obsolete, and
needs to be replaced with one or two current RFC references.
Oops; will replace.
Thanks again, cheers,
Brian