Re: [Gen-art] Gen-ART and OPS-Dir review of draft-ietf-json-text-sequence-09

["Black, David" <[e-mail]>]

Nico,

Thanks for the comprehensive response.

I've excerpted a few things for further comment - I'm fine with the
proposed actions for anything not mentioned here.

[A] JSON text parse failures

> >    If parsing of such an octet string as a JSON text fails, and the
> >    octet string is followed by an RS octet, the parser
> >    SHOULD nonetheless skip ahead to that RS octet and continue parsing
> >    the remainder of the sequence from there.
> 
> That's a weird way of saying that, wherever the JSON text parse fails,
> the sequence parser should then seek forward until the next RS-valued
> byte.  But it works for me; I'll take it.

Your alternative wording "whenever the JSON text parse fails, ..." is fine.

[D] Truncation

> A missing terminating LF is not a problem for strings, arrays, or
> objects.  I seem to recall that we did discuss this.  We could require
> that such texts fail to parse, but perhaps the more important thing is
> to require common parser behavior as to such truncations.
> 
> You ABNF proposal is certainly more strict than the one in the I-D.  I'm
> neutral as to whether this form or the one in the I-D (with the ws issue
> fixed) is better.  The stricter form is clearly easier to talk about,
> therefore preferable, but it will mean discarding texts where only that
> terminating LF is truncated.

I concur with both of the above paragraphs - my preference is to detect
incomplete JSON texts at the sequence level via the missing LF rather than
special-casing numbers and relying on failed JSON parses for everything else.
In general, earlier detection of errors increases the options for dealing
with them.

Once the incomplete text is detected, a JSON parse could be attempted,
with the JSON parser knowing that the text is incomplete (e.g., text
may fail to parse, a number at the end of the text must not be produced
as an incremental parse result).

I'll defer to the WG on whether/how to expand the decoder ABNF to better
detect incomplete texts and how to deal with any incomplete texts that are
detected.

As for RFC 20 ...

> > Nits/editorial comments:
> >
> > idnits didn't like the reference to RFC 20 for ASCII:
> >
> >   ** Downref: Normative reference to an Unknown state RFC: RFC   20
> 
> Is this resolved by now?  I can always reference only Unicode.

Keep the RFC 20 reference - I have no problem with it.  Moreover, as a
result of all the hubbub around this nit, the IESG has issued a Last Call
to reclassify RFC 20 as an Internet Standard ... so that this never
arises again ...

http://www.ietf.org/mail-archive/web/ietf-announce/current/msg13546.html

Thanks,
--David

> -----Original Message-----
> From: Nico Williams [mailto:[e-mail]]
> Sent: Monday, December 08, 2014 11:20 PM
> To: Black, David
> Cc: General Area Review Team ([e-mail]) [e-mail];
> [e-mail]; [e-mail]
> Subject: Re: Gen-ART and OPS-Dir review of draft-ietf-json-text-sequence-09
> 
> On Fri, Dec 05, 2014 at 02:51:04PM +0000, Black, David wrote:
> > This is a combined Gen-ART and OPS-Dir review.  Boilerplate for both follows
> ...
> 
> Thanks you.
> 
> > Minor issues:
> >
> > [A] Section 2.1:
> >
> >    If parsing of such an octet string as a JSON text fails, the parser
> >    SHOULD nonetheless continue parsing the remainder of the sequence.
> >
> > That's not quite right - there are two levels of parsing, JSON
> > sequence parsing and JSON text parsing of each text in the sequence,
> > both of which might be implemented in a single-pass parser.  For such an
> > implementation, the above sentence could be (mis-)read to imply that the
> > JSON text parse should resume from the point at which it failed, which
> > would be silly (although I've seen heroic PL/1 parsers do exactly that).
> > Instead, the parse needs to skip ahead to the next RS, ignoring the rest
> > of the JSON text that failed to parse.  I suggest:
> 
> Good point.
> 
> >    If parsing of such an octet string as a JSON text fails, and the
> >    octet string is followed by an RS octet, the parser
> >    SHOULD nonetheless skip ahead to that RS octet and continue parsing
> >    the remainder of the sequence from there.
> 
> That's a weird way of saying that, wherever the JSON text parse fails,
> the sequence parser should then seek forward until the next RS-valued
> byte.  But it works for me; I'll take it.
> 
> > [B] Section 2.3:
> >
> > Is incremental parsing of a JSON text within a sequence allowed, or
> > is the parser required to not produce any results until the parse of
> > the entire text is successful?  I'd expect that incremental parsing
> > is ok (so results may be produced from a text that ultimately fails
> > to parse), and I think that's worth stating.
> 
> It's permitted, of course, with the caveat that all incremental parsers
> have: the parse can ultimately fail.  I'll add this note:
> 
>    Incremental JSON text parsers may be used, though of course failure
>    to parse a given text may result after first producing some
>    incremental parse results.
> 
> to section 2.3.
> 
> > [C] Section 2.4:
> >
> >    Parsers MUST check that any JSON texts that are a top-level number
> >    include JSON whitespace ("ws" ABNF rule from [RFC7159]) after the
> >    number, otherwise the JSON-text may have been truncated.
> >
> > That reference to the "ws" rule doesn't get the job done because that
> > rule allows a match to no characters - it's of the form ws = *( ... )
> > where ... is the list of whitespace characters.  What's needed here is
> > a rule of the form vws = 1*( ...) to force there to be at least one
> > whitespace character, but see the next issue for a better way to deal
> > with this topic by pulling the appended LF into the sequence parse
> > instead of the text parse.
> 
> I'd wanted to not have to list the characters that are considered
> whitespace.  I agree that the "ws" rule is not appropriate though.
> 
> > [D] I wonder whether the possibility of incomplete texts ought to be
> > encoded into the parsing rules to directly catch JSON texts that must
> > be incomplete because the last character is not LF, e.g.:
> 
> A missing terminating LF is not a problem for strings, arrays, or
> objects.  I seem to recall that we did discuss this.  We could require
> that such texts fail to parse, but perhaps the more important thing is
> to require common parser behavior as to such truncations.
> 
> You ABNF proposal is certainly more strict than the one in the I-D.  I'm
> neutral as to whether this form or the one in the I-D (with the ws issue
> fixed) is better.  The stricter form is clearly easier to talk about,
> therefore preferable, but it will mean discarding texts where only that
> terminating LF is truncated.
> 
> One problem we get into with your ABNF is that RFC5234 does not discuss
> greediness (this came up on the list).  But this can be noted (see
> below).
> 
> One nice thing about the stricter rules is that we need not discuss
> top-level numbers (or booleans, or null) with normative text, just note
> them.
> 
> >      JSON-sequence = *(1*RS (possible-JSON / truncated-JSON / empty-JSON))
> >      RS = %x1E; "record separator" (RS), see RFC20
> >      possible-JSON = 1*(not-RS) LF ; attempt to parse as UTF-8-encoded
> >                                ; JSON text (see RFC7159)
> >      truncated-JSON = *(not-RS) not-LFRS); truncated, don't attempt
> > 					; to parse as JSON text
> >      empty-JSON = LF ; only the LF appended by the encoder, nothing to parse
> >
> >      not-RS = %x00-1D / %x1F-FF; any octet other than RS
> >      not-LFRS = %x00-09/ %x1B-1D / %x1F-FF; any octet other than RS or LF
> >
> > Note that this won't detect all incomplete JSON texts, because LF is allowed
> > within a JSON text (and this should be stated).
> 
> It will if ABNF matching is greedy and possible-JSON is defined as
> 
>    possible-JSON = 1*( 1*(not-RS) LF); ...
> 
> Advice as to which form to take?
> 
> > [E] Section 3 - Security Considerations
> >
> > Incomplete and malformed JSON texts can be used to attack JSON parsers -
> > that should be pointed out, as I don't see that in RFC 7159's security
> > considerations and incomplete texts are a relevant consideration for
> > this draft.
> 
> And surely also for RFC7159.  Besides requiring that they fail to parse
> (and the note about incremental parsing), are there any other missing
> considerations?  Ah yes, smuggling of data in sequences where parsers
> will ignore without warning any octet strings that fail to parse as JSON
> texts.
> 
> Proposed text:
> 
>    As usual, parsers must operate on as-good-as untrusted input. This
>    means that parsers must fail gracefully in the face of malicious
>    inputs. Note that incremental parsers can produce partial results and
>    later indicate failure to parse the remainder of a text. Note that
>    texts that fail to parse and are ignored can be used to smuggle data
>    past sequence parsers that don't warn about JSON text failures.
> 
> > [F] Section 4 - IANA Considerations
> >
> >    Security considerations: See <this document, once published>,
> >    Section 3.
> >
> >    Interoperability considerations: Described herein.
> >
> >    Published specification: <this document, once published>.
> >
> >    Applications that use this media type: <by publication time
> >    <https://stedolan.github.io/jq> is likely to support this format>.
> >
> > Replace all three instances of the angle bracketed text.  The first two
> > instances should be RFC references (e.g., RFC XXXX) w/a note to the RFC
> > Editor to insert the number of the RFC when published.  The third instance
> > should be resolved now, or could have an RFC Editor note added indicating
> > that the author will resolve that during Authors 48 hours.
> 
> Sure.
> 
> > Nits/editorial comments:
> >
> > idnits didn't like the reference to RFC 20 for ASCII:
> >
> >   ** Downref: Normative reference to an Unknown state RFC: RFC   20
> 
> Is this resolved by now?  I can always reference only Unicode.
> 
> Thanks for your excellent review,
> 
> Nico
> --