Subject: Gen-ART and OPS-Dir review of draft-ietf-json-text-sequence-10

From: "Black, David" <[email-masked]>
Date: Wed, 10 Dec 2014 15:51:05 +0000

The -10 version of this draft resolves items [A]-[E] from the
Gen-ART and OPS-Dir review of -09, and the IESG is in the process of
resolving the (silly) idnits complaint about RFC 20 being a possible
downref.

For item [D], a different approach was taken instead of modifying
the ABNF - the resulting new Section 2.4 is a definite improvement
to the draft, and is significantly clearer than the modified ABNF
would have been.  Nicely done.

Item [F] about the <angle-bracketed> text in the IANA Considerations
(Section 4) remains open - if the intent is to not deal with replacing
that text until after IESG approval, an RFC Editor Note to that effect
should be added to Section 4.

I have an additional editorial concern - given all the discussion about
UTF-8, it would be good for the draft to make it clear early on 
that JSON text sequences are UTF-8 only.  Here are some suggested changes.

Abstract:

   This document describes the JSON text sequence format and associated
   media type, "application/json-seq".  A JSON text sequence consists of
   any number of JSON texts, each prefix by an Record Separator
   (U+001E), and each ending with a newline character (U+000A).

"any number of JSON texts" -> "any number of UTF-8 encoded JSON texts"

It also looks like ASCII names for RS and LF are being mixed w/Unicode
codepoints in the second sentence in the abstract.  I'm not sure that's
a good thing to do, especially as the body of the draft refers to RS and
LF as being ASCII.  Here are a couple of changes that would remedy this:

   "an Record Separator (U+001E)" -> "an ASCII Record Separator (0x1E)"
   "a newline character (U+000A)" -> "an ASCII newline character (0x0A)"

Section 2 JSON Text Sequence Format:

I suggest adding this sentence as a separate paragraph at the end of this
section (i.e., just before Section 2.1):

   JSON text sequences MUST use UTF-8 encoding; other encodings of JSON
   (i.e., UTF-16 and UTF-32) MUST NOT be used.

Aside from item [F], all of the above are editorial suggestions, but I
think making this clear early in the draft will help avoid potential
implementer confusion.

Thanks,
--David

> -----Original Message-----
> From: Black, David
> Sent: Friday, December 05, 2014 9:51 AM
> To: [email-masked]; General Area Review Team ([email-masked]); ops-
> [email-masked]
> Cc: [email-masked]; [email-masked]; Black, David
> Subject: Gen-ART and OPS-Dir review of draft-ietf-json-text-sequence-09
> 
> This is a combined Gen-ART and OPS-Dir review.  Boilerplate for both follows
> ...
> 
> 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.
> 
> I have reviewed this document as part of the Operational directorate's ongoing
> effort to review all IETF documents being processed by the IESG.  These
> comments
> were written primarily for the benefit of the operational area directors.
> Document editors and WG chairs should treat these comments just like any other
> last call comments.
> 
> Document: draft-ietf-json-text-sequence-09
> Reviewer: David Black
> Review Date: Dec 5, 2014
> IETF LC End Date: Dec 5, 2014
> IESG Telechat date: Dec 18, 2014
> 
> Summary: This draft is on the right track, but has open issues
>  		described in the review.
> 
> This draft specifies a format that packs multiple JSON texts into a
> single string.  The ASCII RS (0x1E) character is used to separate texts,
> and a linefeed is appended to each text to ensure that a complete text
> always ends with a whitespace character.
> 
> All of the open issues are minor - the most important ones center on
> treatment of incomplete JSON texts - that appears to be an afterthought
> in this draft and needs more attention.  I also found a couple of
> minor issues in the Security and IANA Considerations sections, both of
> which are almost nits.
> 
> Major issues: None.
> 
> 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:
> 
>    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 also covers the case where there is nothing more to parse after the
> JSON text that caused the parse failure.
> 
> [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.
> 
> [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.
> 
> [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.:
> 
>      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).
> 
> [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.
> 
> [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.
> 
> Nits/editorial comments:
> 
> idnits didn't like the reference to RFC 20 for ASCII:
> 
>   ** Downref: Normative reference to an Unknown state RFC: RFC   20
> 
> RFC 5234 (ABNF) uses this, which looks like a better reference:
> 
>    [US-ASCII]  American National Standards Institute, "Coded Character
>                Set -- 7-bit American Standard Code for Information
>                Interchange", ANSI X3.4, 1986.
> 
> --- Selected RFC 5706 Appendix A Q&A for OPS-Dir review ---
> 
> Most of these questions are n/a because this draft describes a format
> that will be used in other protocols to which RFC 5706's concerns would apply.
> 
> A.1.4   Have the Requirements on other protocols and functional
>        components been discussed?
> 
> The specification of the interaction of the JSON sequence parser with the
> JSON text parser is not as clear as it should be for incomplete or malformed
> JSON texts.  See Minor Issues [A]-[E] above.
> 
> A.1.8   Are there fault or threshold conditions that should be reported?
> 
> Yes, incomplete JSON texts - this is covered in sections 2.3 and 2.4.
> 
> Thanks,
> --David
> ----------------------------------------------------
> David L. Black, Distinguished Engineer
> EMC Corporation, 176 South St., Hopkinton, MA  01748
> +1 (508) 293-7953             FAX: +1 (508) 293-7786
> [email-masked]        Mobile: +1 (978) 394-7754
> ----------------------------------------------------

---

From: John Cowan <[email-masked]>
Date: Wed, 10 Dec 2014 20:19:51 -0500

>    "a newline character (U+000A)" -> "an ASCII newline character (0x0A)"

That should be "an ASCII Line Feed character".  RFC 20 says that the
Line Feed character may have a New Line [sic] *function*.

-- 
John Cowan          http://www.ccil.org/~cowan        [email-masked]
I am expressing my opinion.  When my honorable and gallant friend is
called, he will express his opinion.  This is the process which we
call Debate.                   --Winston Churchill

---

From: Paul Hoffman <[email-masked]>
Date: Thu, 11 Dec 2014 10:29:16 -0800

<shepherd hat on>

Thanks for the followup comments on -10. In general, I think they are fine, and Nico could put out a -11 before IESG telechat review. See below.

> should be added to Section 4.

David: I disagree with the need for this change. The RFC Editor can interpret the current wording just fine.

> "any number of JSON texts" -> "any number of UTF-8 encoded JSON texts"

This change concerns me, because it sounds like a JSON text sequence could consist of JSON texts encoded in UTF-8 and other encodings. I would instead prefer "any number of JSON texts, all encoded in UTF-8,".

I also just now noticed that there is a typo in the abstract: it should say "each prefix*ed*". 

>   "a newline character (U+000A)" -> "an ASCII newline character (0x0A)"

With John Cowan's change ("an ASCII Line Feed character (0x1E)" instead of "an ASCII Record Separator (0x1E)"), that would indeed be clearer.

>   (i.e., UTF-16 and UTF-32) MUST NOT be used.

That seems like a good clarifying addition as well.

Nico: could you issue a -11 with the above changes?

--Paul Hoffman

---

From: "Black, David" <[email-masked]>
Date: Thu, 11 Dec 2014 19:07:25 +0000

All of this is fine, with the correction below (looks like a copy/paste
problem), and a -11 would be a fine thing to put out w/these changes.

Thanks,
--David

> From: Paul Hoffman [mailto:[email-masked]]
> Sent: Thursday, December 11, 2014 1:29 PM
> To: Black, David
> Cc: Nico Williams; General Area Review Team ([email-masked]); ops-
> [email-masked]; [email-masked]; [email-masked]
> Subject: Re: [Json] Gen-ART and OPS-Dir review of draft-ietf-json-text-
> sequence-10
> 
> <shepherd hat on>
> 
> Thanks for the followup comments on -10. In general, I think they are fine,
> and Nico could put out a -11 before IESG telechat review. See below.
> 

Ok.

> This change concerns me, because it sounds like a JSON text sequence could
> consist of JSON texts encoded in UTF-8 and other encodings. I would instead
> prefer "any number of JSON texts, all encoded in UTF-8,".

Ok.

> With John Cowan's change ("an ASCII Line Feed character (0x1E)" instead of "an
> ASCII Record Separator (0x1E)"), that would indeed be clearer.

I'm sure Paul meant to write: ("an ASCII Line Feed character (0x0A)"
instead of "an ASCII newline character (0x0A)").

---

From: Nico Williams <[email-masked]>
Date: Thu, 11 Dec 2014 13:55:47 -0600

I'll submit a -11 ASAP.

---

From: Nico Williams <[email-masked]>
Date: Thu, 11 Dec 2014 16:46:50 -0600

> interpret the current wording just fine.

I'll leave them as-is then.

> This change concerns me, because it sounds like a JSON text sequence
> could consist of JSON texts encoded in UTF-8 and other encodings. I
> would instead prefer "any number of JSON texts, all encoded in
> UTF-8,".

I'll take this.

> should say "each prefix*ed*". 

Fixed.

> With John Cowan's change ("an ASCII Line Feed character (0x1E)"
> instead of "an ASCII Record Separator (0x1E)"), that would indeed be
> clearer.

OK.

> Nico: could you issue a -11 with the above changes?

I'll confirm the posting seconds from sending this.

---

From: "Manger, James" <[email-masked]>
Date: Fri, 12 Dec 2014 09:50:53 +1100

>> codepoints in the second sentence in the abstract.  I'm not sure 
>> that's a good thing to do, especially as the body of the draft refers 
>> to RS and LF as being ASCII.  Here are a couple of changes that would remedy this:
>> 

>With John Cowan's change ("an ASCII Line Feed character (0x1E)" instead of "an ASCII Record Separator (0x1E)"), that would indeed be clearer.

Please no. That would give an even worse mix of UTF-8 and ASCII, bytes and characters, in the 1 sentence.

  ".. any number of JSON texts, all encoded in UTF-8, each prefixed by an ASCII Record Separator (0x1E) .."

How about:

  "A JSON text sequence consists of any number of JSON texts,
   each prefixed by a Record Separator (U+001E) character, and
   each suffixed by an End of Line (U+000A) character. It is
   UTF-8 encoded."

Say "Information Separator Two (U+001E)" if you really want to be pure.

Mention in the body that "Record Separator" and "Information Separator Two" are the ASCII and Unicode names for the same character (as are "Line Feed" and "End of Line"), which is why RS and LF are used as ABNF names.

P.S. The spec still defines the same ABNF names twice (RS, JSON-sequence): once as bytes; once as Unicode scalars. Yuck. Just give them different names.

--
James Manger

---

From: "Black, David" <[email-masked]>
Date: Thu, 11 Dec 2014 23:04:36 +0000

I'm not concerned about this - the draft is UTF-8-only (it now explicitly
forbids UTF-16 and UTF-32) and is written on the assumption that it's common
knowledge that 7-bit ASCII (as octets with zero in the most significant bit)
is a UTF-8 subset.

Thanks,
--David

> From: Manger, James [mailto:[email-masked]]
> Sent: Thursday, December 11, 2014 5:51 PM
> To: Paul Hoffman; Black, David
> Cc: Nico Williams; General Area Review Team ([email-masked]); [email-masked];
> [email-masked]; [email-masked]
> Subject: RE: [Json] Gen-ART and OPS-Dir review of draft-ietf-json-text-
> sequence-10
> 
> >> Abstract:
> >>
> >With John Cowan's change ("an ASCII Line Feed character (0x1E)" instead of
> "an ASCII Record Separator (0x1E)"), that would indeed be clearer.
> 
> 
> Mention in the body that "Record Separator" and "Information Separator Two"
> are the ASCII and Unicode names for the same character (as are "Line Feed" and
> "End of Line"), which is why RS and LF are used as ABNF names.
> 
> P.S. The spec still defines the same ABNF names twice (RS, JSON-sequence):
> once as bytes; once as Unicode scalars. Yuck. Just give them different names.
> 
> --
> James Manger

---

From: Nico Williams <[email-masked]>
Date: Thu, 11 Dec 2014 17:05:48 -0600

> and characters, in the 1 sentence.

Well, I just submitted -11 :(

But I don't think it's a problem, really.  UTF-8 is a superset of ASCII.

> P.S. The spec still defines the same ABNF names twice (RS,
> JSON-sequence): once as bytes; once as Unicode scalars. Yuck. Just
> give them different names.

They're two different sets of rules, yes.  If people want me to, I'll
rename one of them.

Nico
--

---

From: "Black, David" <[email-masked]>
Date: Fri, 12 Dec 2014 01:22:33 +0000

If we're going to split hairs, "MUST use UTF-8 encoding" does not imply
"MUST be valid UTF-8" (e.g., courtesy of truncation in the middle of a
Unicode character) - the latter is a stricter requirement, and is not
what I suggested.

Thanks,
--David

> From: John Cowan [mailto:[email-masked]] On Behalf Of John Cowan
> Sent: Thursday, December 11, 2014 8:04 PM
> To: Paul Hoffman
> Cc: Black, David; Nico Williams; General Area Review Team ([email-masked]);
> [email-masked]; [email-masked]; [email-masked]
> Paul Hoffman scripsit:
> I continue to think this is a bad idea.  In particular, you cannot claim
> that because a text sequence entity body is not valid UTF-8, that it is
> not a valid JSON text sequence; as I explained before, the JSON texts
> may be truncated or otherwise damaged such that they contain only part
> of a UTF-8-encoded character.
> 
> --
> Deshil Holles eamus.  Deshil Holles eamus.  Deshil Holles eamus.
> Send us, bright one, light one, Horhorn, quickening, and wombfruit. (3x)
> Hoopsa, boyaboy, hoopsa!  Hoopsa, boyaboy, hoopsa!  Hoopsa, boyaboy, hoopsa!
>   --Joyce, Ulysses, "Oxen of the Sun"

---

From: Nico Williams <[email-masked]>
Date: Thu, 11 Dec 2014 19:37:55 -0600

> I continue to think this is a bad idea.  In particular, you cannot claim

What is?

> of a UTF-8-encoded character.

RS cannot accidentally result from truncating a multi-byte UTF-8
encoded codepoint.

Nico
--

---

From: Nico Williams <[email-masked]>
Date: Thu, 11 Dec 2014 21:22:40 -0600

On Thu, Dec 11, 2014 at 08:41:40PM -0500, John Cowan wrote:
> Nico Williams scripsit:
> I realize that.  But because such truncations are possible, we can't
> assert that a well-formed JSON text sequence is necessarily well-formed
> UTF-8 as a whole.

Oh, I see what you're saying, you're objecting to the sentence that
David wanted added to section 2, before 2.1.

We could prefix "Well-formed " to that sentence and make this concern go
away :)

---

From: John Cowan <[email-masked]>
Date: Thu, 11 Dec 2014 20:04:18 -0500

---

From: John Cowan <[email-masked]>
Date: Thu, 11 Dec 2014 20:12:08 -0500

> Say "Information Separator Two (U+001E)" if you really want to be pure.

The trouble with that is that U+001E has no official Unicode name or
function; those come from ISO 6429, which is incorporated (in relevant
part) into US-ASCII, which is described in RFC 20.

-- 
John Cowan          http://www.ccil.org/~cowan        [email-masked]
To say that Bilbo's breath was taken away is no description at all.  There are
no words left to express his staggerment, since Men changed the language that
they learned of elves in the days when all the world was wonderful. --The Hobbit

---

From: John Cowan <[email-masked]>
Date: Thu, 11 Dec 2014 20:41:40 -0500

-- 
John Cowan          http://www.ccil.org/~cowan        [email-masked]
Objective consideration of contemporary phenomena compel the conclusion
that optimum or inadequate performance in the trend of competitive
activities exhibits no tendency to be commensurate with innate capacity,
but that a considerable element of the unpredictable must invariably be
taken into account. --Ecclesiastes 9:11, Orwell/Brown version

---

From: Patrik Fältström <[email-masked]>
Date: Sat, 13 Dec 2014 08:02:26 +0100

> On 12 dec 2014, at 02:12, John Cowan <[email-masked]> wrote:
> 
> Manger, James scripsit:
> 
>> How about:
>> 

Although it does not have a Unicode Name, the alias is as close as we can get, which is "INFORMATION SEPARATOR TWO":

# grep ^001E UnicodeData.txt
001E;<control>;Cc;0;B;;;;;N;INFORMATION SEPARATOR TWO;;;;
#

So I suggest to use that.

It is I think wrong to say "Record Separator" and then still reference the Unicode Tables.

Alternatively one just write (and make it more clear how this works, and this is my understanding):

> A JSON text sequence consists of any number of JSON texts, each prefixed by U+001E character and each suffixed by U+000A. The JSON texts as well as the whole JSON text sequence is encoded in UTF-8 although any JSON text might be truncated and because of that not a valid UTF-8 sequence. Any occurance of the UTF-8 encoding of U+001E (the byte 0x1E) is to be viewed as the first byte before each JSON text, and occurrance of the byte 0x0A is to be viewed as the first byte after a complete JSON text. If the JSON text is truncated, the 0x0A byte will not be present.

I.e. the grammar is sort of (before coffee in the morning):

sequence := 0x1E text

text := complete-text | truncated-text

complete-text := proper-UTF8 0x0A

truncated-text := proper-UTF8 broken-UTF8

proper-UTF8 := "" | "a sequence of bytes, possible to parse as a series of UTF8 encoded Unicode characters"

broken-UTF8 := "a sequence of bytes not possible to parse as a UTF8 encoded unicode character"

   Patrik

---

From: Jari Arkko <[email-masked]>
Date: Thu, 18 Dec 2014 08:45:22 -0500

David  -thank you for the review!

My understanding of this thread and the -11 is that we are done with respect to the modifications coming out of your review. Let me know otherwise.

Thanks, all. 

Jari

---

From: "Black, David" <[email-masked]>
Date: Thu, 18 Dec 2014 14:09:09 +0000

Yes, -11 is fine wrt my review: http://www.ietf.org/mail-archive/web/gen-art/current/msg11048.html

Thanks,
--David

> From: Jari Arkko [mailto:[email-masked]]
> Sent: Thursday, December 18, 2014 8:45 AM
> To: Patrik Fältström; Black, David
> Cc: John Cowan; [email-masked]; [email-masked]; Paul Hoffman; Manger, James;
> General Area Review Team ([email-masked])
> My understanding of this thread and the -11 is that we are done with respect
> to the modifications coming out of your review. Let me know otherwise.
> 
> Thanks, all.
> 
> Jari
> 
> > Although it does not have a Unicode Name, the alias is as close as we can
> get, which is "INFORMATION SEPARATOR TWO":
> >
> >> A JSON text sequence consists of any number of JSON texts, each prefixed by
> U+001E character and each suffixed by U+000A. The JSON texts as well as the
> whole JSON text sequence is encoded in UTF-8 although any JSON text might be
> truncated and because of that not a valid UTF-8 sequence. Any occurance of the
> UTF-8 encoding of U+001E (the byte 0x1E) is to be viewed as the first byte
> before each JSON text, and occurrance of the byte 0x0A is to be viewed as the
> first byte after a complete JSON text. If the JSON text is truncated, the 0x0A
> byte will not be present.
> >