Last Call Review of draft-ietf-json-rfc4627bis-07
review-ietf-json-rfc4627bis-07-genart-lc-davies-2013-11-29-00

Request Review of draft-ietf-json-rfc4627bis
Requested rev. no specific revision (document currently at 10)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2013-11-25
Requested 2013-11-14
Other Reviews Genart Telechat review of -09 by Elwyn Davies (diff)
Secdir Last Call review of -07 by Tobias Gondrom (diff)
Secdir Telechat review of -09 by Tobias Gondrom (diff)
Review State Completed
Reviewer Elwyn Davies
Review review-ietf-json-rfc4627bis-07-genart-lc-davies-2013-11-29
Posted at http://www.ietf.org/mail-archive/web/gen-art/current/msg09364.html
Reviewed rev. 07 (document currently at 10)
Review result Almost Ready
Draft last updated 2013-11-29
Review completed: 2013-11-29

Review
review-ietf-json-rfc4627bis-07-genart-lc-davies-2013-11-29

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 wait for direction from your document shepherd
or AD before posting a new version of the draft.

Document: draft-ietf-json-rfc4627bis-07.txt
Reviewer: Elwyn Davies
Review Date: 28 November 2013
IETF LC End Date: 25 November 2013
IESG Telechat date: 5 December 2013

Summary:
[Apologies for missing the end of last call.]


Almost ready for publication as a standard track RFC.  There are a 


couple of minor issues and some nits that should be addressed before 


publication.






After I submitted the original version of this review three additional 


points occurred to me:



- The virtue of providing an appendix with the ABNF altogether in one place
- the inconsistency of sections 9 and 10.
- minor inconsistency in layout of example 1.



There have been discussions of the use of BOM characters at the start of 


JSON strings, whether the top level production should just be a 'value' 


(with ws), what the status of the reference to ECMA-404 shoould be and 


the "heavy encouragement" of the use of UTF-8 encoding.  IANA also 


require some clarification of the update of the application/json MIME 


type. This review doesn't address these issues.




Major issues:
None.

Minor issues:


s1, para 4: It would probably be useful to clarify that 'value' has the 


same meaning as in the previous para.  Also that it is not required that 


the array elements in a single array be all the same type.






s4: The unordered nature of the entries in an object is not mentioned. 


Presumably it should state that two objects with the same sets of 


name-value pairs ("members") in any order are logically equivalent.






s10: s9 allows there to be parsers that accept extensions.  s10 only 


allows generators to produce strict JSON.  Perhaps one should allow for 


strict generators and extended generators that match with extended parsers?



s12, last sentence:



The same
    consideration applies in any other programming language in which JSON
    texts conform to that language's syntax.


Whilst there is clearly a security issue when a JSON text is fed to the 


equivalent of JavaScript's eval() function, it strikes me that this is 


only a problem if the other programming language also overloads its 


equivalent of eval() to parse JSON texts.  I think the text isn't clear 


on this.  Perhaps add 'and the JSON text parser does the equivalent of 


eval()' to the end. (Actually the change note in Appendix A is *much* 


clearer on what is meant!)






A potentially more serious issue comes from the standard buffer 


overrun/execution of data problems in that the JSON text may be parsed 


(effectively compiled) into a piece of executable binary. This should 


probably be called out.




Nits/editorial comments:


General: Many RFCs that use ABNF specifications where parts of the ABNF 


are presented in separate sections also provide a section or appendix 


with the whole of the ABNF in one place.  Whilst this provides a very 


minor double maintenance problem, it does aid implementers and 


facilitates checking of the complete grammar to catch mismatches.






s1, para 1 and s1.2, para 2: The statements of ECMAscript being 'Third 


Edition' in s1 and 'version 5.1' in s1.2 is slightly confusing - would 


it be possible to explain what is going on here please?






s5: Would be worth reiterating that there is no requirement for the 


values in an array to be the same type.






s6: To be absolutely definite, assert that numbers are represented in 


base 10.






 s11: There is inconsistency in the presentation of email addresses: 


The two instances of <iesg at ietf.org are (presumably) missing a trailing 


'>'. For consisteency should douglas at crockford.com be 


<douglas at crockford.com>?






s13: (very nitty - but could be confusing) The indentations of the 


"Animated" member and the trailing } are inconsistent with pretty 


printing scheme for the rest of the example. ("Animated" s/b right 


shifted by 4 spaces, } s/b left shifted by 2 spaces)




s13: Might be useful to insert examples with
- an array with elements that are not the same type
- equivalent objects with the members in different orders.


These could also show off escape sequences and maximally compact formats 


(using the equivalent object example to show that the white space is 


irrelevant).