Subject: [Jcardcal] Genart LC review: draft-ietf-jcardcal-jcal-09

From: Robert Sparks <[email-masked]>
Date: Tue, 11 Mar 2014 13:55:09 -0500

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-jcardcal-jcal-09
Reviewer: Robert Sparks
Review Date: 11Mar2014
IETF LC End Date: 12Mar2014
IESG Telechat date: 27Mar2014

Summary: Ready with nits

This is a solid document, and its development has left good artifacts 
showing a pattern of careful review.
(such as <http://trac.tools.ietf.org/wg/jcardcal/trac/report/6>).

Here are some nits to consider:

I agree with moving the reference to RFC4627 to normative, as already 
discussed.

Please consider adding a reference to clarify "JSON escaping" where it 
is mentioned in the 2nd paragraph of page 5.
Perhaps section 2.5 of rfc4627 would be a good reference?

The MUST in the third paragraph of 3.4.1.1 stuck out - is looks like a 
restatement of RFC5545 - that spec doesn't _allow_ anything but a 
semicolon for this particular separator. Would this be better written 
without 2119?
Perhaps: "When converting from jCal to iCalendar, be careful to use a 
semi-colon as the separator between the two values as required by RFC5545."

(This may be more than a nit): In the ABNF in section 3.6.5, where is 
the implementer supposed to go to find the definition of 'zone'? (Or the 
other production names?) I think _this_ chunk of ABNF (as opposed to 
that compiled in the appendix) is intended to be normative, yes? FWIW, 
it's not reflected in Appendix B.

I haven't extracted the BNF in appendix B and verified it, but it must 
fail - there is at least one typo. The expansion of param-multi includes 
"value-separtor" which should have been "value-separator".
Where is value-separator defined?

Just curious - has anyone tried converting a document from 
iCal->xCal->jCal->iCal? That might turn up some interesting corners that 
simple round-tripping might mask.

To try to save other reviewers some time, here are a couple of things I 
flagged that turned out to be non-issues:
* I was concerned with whether there would be issues with the forced 
conversion between upper and lower case. A little digging shows there is 
no issue - all the names this is done to are limited to the 
ascii-compatible characters.
* I verified that the syntax numbers with fractional parts is the same 
in both iCal in jCal. Specifically "4." is not valid in either grammar, 
so there is no need to discuss something like adding a 0 or remove the 
decimal point during conversion.

---

From: Robert Sparks <[email-masked]>
Date: Tue, 11 Mar 2014 14:06:46 -0500

Actually, there are perhaps some corners to consider knocking off 
related to:
> in both iCal in jCal. Specifically "4." is not valid in either 
> grammar, so there is no need to discuss something like adding a 0 or 
> remove the decimal point during conversion.
looking again, iCal says:

float      = (["+"] / "-") 1*DIGIT ["." 1*DIGIT]

but json says:

number = [ minus ] int [ frac ] [ exp ]
int = zero / ( digit1-9 *DIGIT )

So, in iCal, you could have '+4' . json doesn't allow the plus sign.

iCal appears to allow leading 0s. json does not.

---

From: Philipp Kewisch <[email-masked]>
Date: Thu, 20 Mar 2014 21:32:38 +0100

Hi Robert,

Thank you for your valuable comments.
> I agree with moving the reference to RFC4627 to normative, as already
Done.

> Perhaps section 2.5 of rfc4627 would be a good reference?
Sounds good. Here is my suggestion:

OLD
   When converting from iCalendar to jCal, first iCalendar lines MUST be
   unfolded.  Afterwards, any iCalendar escaping MUST be unescaped.
   Finally JSON escaping (e.g., for control characters) MUST be applied.

   The reverse order applies when converting from jCal to iCalendar.
   First, JSON escaping MUST be unescaped.  Afterwards, iCalendar
   escaping MUST be applied.  Finally, long lines SHOULD be folded as
   described in [RFC5545].
NEW
   When converting from iCalendar to jCal, first iCalendar lines MUST be
   unfolded.  Afterwards, any iCalendar escaping MUST be unescaped.
   Finally JSON escaping, as described in [RFC7195] Section 7, MUST be
   applied.

   The reverse order applies when converting from jCal to iCalendar.
   First, JSON escaping MUST be unescaped.  Afterwards, iCalendar
   escaping MUST be applied.  Finally, long lines SHOULD be folded as
   described in [RFC5545].
END

> semi-colon as the separator between the two values as required by
Thanks for the suggestion, I've changed it locally.

> the implementer supposed to go to find the definition of 'zone'? (Or
> the other production names?) I think _this_ chunk of ABNF (as opposed
> to that compiled in the appendix) is intended to be normative, yes?
> FWIW, it's not reflected in Appendix B.
Indeed, there are some shortcomings here. For the
year/month/day/hour/minute/second production names I can add an explicit
reference to ISO.8601.2004 Section 2.2. The zone designator is only
marginally mentioned in ISO.8601.2004 Section 4.3.2.

The exact date format is not reflected in Appendix B because the
Appendix does not intend to capture the structure of each different
property type.

As the two jcardcal drafts are my first rfc documents, I am not quite
sure when its a good idea to make the ABNF normative. I could probably
declare it informative by virtue of the reference to ISO 8601 and
RFC5545. The ABNF was added in draft 08, in alignment with the changes
we've made in jcard (rfc7095). There was a lot of discussion on the
jcardcal list on the date formats and it became clear that the specific
variations of ISO 8601 2001 and 2004 used in iCalendar and vCard make it
hard to grasp. By explicitly mentioning the date format I wanted to
counteract.

I'd appreciate some feedback on how to further handle this issue.

> fail - there is at least one typo. The expansion of param-multi
> includes "value-separtor" which should have been "value-separator".
I've fixed two typos and run it through
<http://www.apps.ietf.org/content/chris-newmans-abnf-validator>.
Validation was successful.

value-separator is defined in section 2 of rfc 7159.

> iCal->xCal->jCal->iCal? That might turn up some interesting corners
> that simple round-tripping might mask.
I haven't tried that, it might be interesting though. My library cannot
convert xCal to jCal (yet), but maybe Cyrus' library does it and he can
take a look.

Regards,
Philipp

---

From: Philipp Kewisch <[email-masked]>
Date: Thu, 20 Mar 2014 21:39:29 +0100

On 3/11/14, 8:06 PM, Robert Sparks wrote:
>> * I verified that the syntax numbers with fractional parts is the
>> same in both iCal in jCal. Specifically "4." is not valid in either
> [...]

Maybe this just requires some adjustment in wording. The gist is that
numbers should be represented in whatever way needed by the underlying
rfc, i.e 5545 or 7159. If a number represented with a leading + sign is
found in iCal, the same number can be represented without a leading +
sign in JSON. Analogous for leading zeros. Would you prefer this is
explicitly stated?

>
>
>
>
>
> _______________________________________________
> jcardcal mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/jcardcal

---

From: Robert Sparks <[email-masked]>
Date: Thu, 20 Mar 2014 17:20:56 -0500

Inline
>     escaping MUST be applied.  Finally, long lines SHOULD be folded as
wfm
> I'd appreciate some feedback on how to further handle this issue.
Please work with your AD on this one.
It's a little detail, not a big problem, but we do need to be clear 
about what the grammar actually is.
> value-separator is defined in section 2 of rfc 7159.
Provide that reference at the point it's used in the document please.

---

From: Robert Sparks <[email-masked]>
Date: Thu, 20 Mar 2014 17:22:31 -0500

On 3/20/14, 3:39 PM, Philipp Kewisch wrote:
> rfc, i.e 5545 or 7159. If a number represented with a leading + sign 
> is found in iCal, the same number can be represented without a leading 
> + sign in JSON. Analogous for leading zeros. Would you prefer this is 
> explicitly stated?
Yes. And I don't think it needs much text. Just let the implementer know 
the issue is there to take care of.

---

From: Pete Resnick <[email-masked]>
Date: Thu, 20 Mar 2014 17:34:38 -0500

> about what the grammar actually is.

Hmm....I wonder why neither RFC 5545 nor this document reference RFC 
3339 instead of ISO 8601? That would get you all of the ABNF you need.

pr

-- 
Pete Resnick<http://www.qualcomm.com/~presnick/>
Qualcomm Technologies, Inc. - +1 (858)651-4478

---

From: Peter Saint-Andre <[email-masked]>
Date: Thu, 20 Mar 2014 17:04:04 -0600

> 3339 instead of ISO 8601? That would get you all of the ABNF you need.

That's a good point. RFC 3339 is, IMHO, nicely clear and tightly scoped 
compared to ISO 8601 (which leaves quite a few options open). However, 
even when referencing RFC 3339 it's important to document all the 
details about various options, such as use of non-UTC time zones and 
fractional seconds.

Peter

---

From: Philipp Kewisch <[email-masked]>
Date: Fri, 21 Mar 2014 00:22:58 +0100

On 3/21/14, 12:04 AM, Peter Saint-Andre wrote:
> That's a good point. RFC 3339 is, IMHO, nicely clear and tightly
> scoped compared to ISO 8601 (which leaves quite a few options open).
> However, even when referencing RFC 3339 it's important to document all
> the details about various options, such as use of non-UTC time zones
> and fractional seconds.
While rfc3339 is more closely scoped than ISO 8601, there are still
various options that require documenting, which is why I decided to stay
with ISO8601. For consistency with jCard I think it would be unwise to
change the rfc the date format is based now.

---

From: Peter Saint-Andre <[email-masked]>
Date: Thu, 20 Mar 2014 17:24:02 -0600

> change the rfc the date format is based now.

Yes, I do think consistency between jCard and jCal is more important in 
this instance.

Peter

---

From: Philipp Kewisch <[email-masked]>
Date: Fri, 21 Mar 2014 00:26:37 +0100

> Provide that reference at the point it's used in the document please.
Just above the ABNF in the introduction to the section, I write "ABNF
Symbols not described here are taken from [RFC7159]". Isn't that sufficient?

Philipp

---

From: Jari Arkko <[email-masked]>
Date: Wed, 26 Mar 2014 14:28:49 +0800

Thanks for the review, Robert. Some changes are being discussed, but I do not see a new draft. Shouldn't that appear before we make the final approval of this document?

Jari

On Mar 12, 2014, at 2:55 AM, Robert Sparks <[email-masked]> wrote:

> This is a solid document, and its development has left good artifacts showing a pattern of careful review.
> I agree with moving the reference to RFC4627 to normative, as already discussed.
> 
> Please consider adding a reference to clarify "JSON escaping" where it is mentioned in the 2nd paragraph of page 5.
> The MUST in the third paragraph of 3.4.1.1 stuck out - is looks like a restatement of RFC5545 - that spec doesn't _allow_ anything but a semicolon for this particular separator. Would this be better written without 2119?
> Perhaps: "When converting from jCal to iCalendar, be careful to use a semi-colon as the separator between the two values as required by RFC5545."
> 
> (This may be more than a nit): In the ABNF in section 3.6.5, where is the implementer supposed to go to find the definition of 'zone'? (Or the other production names?) I think _this_ chunk of ABNF (as opposed to that compiled in the appendix) is intended to be normative, yes? FWIW, it's not reflected in Appendix B.
> 
> I haven't extracted the BNF in appendix B and verified it, but it must fail - there is at least one typo. The expansion of param-multi includes "value-separtor" which should have been "value-separator".
> Just curious - has anyone tried converting a document from iCal->xCal->jCal->iCal? That might turn up some interesting corners that simple round-tripping might mask.
> 
> To try to save other reviewers some time, here are a couple of things I flagged that turned out to be non-issues:
> * I was concerned with whether there would be issues with the forced conversion between upper and lower case. A little digging shows there is no issue - all the names this is done to are limited to the ascii-compatible characters.
> * I verified that the syntax numbers with fractional parts is the same in both iCal in jCal. Specifically "4." is not valid in either grammar, so there is no need to discuss something like adding a 0 or remove the decimal point during conversion.
> 
> 
> 
> 
> 
> 
> Gen-art mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/gen-art

---

From: Philipp Kewisch <[email-masked]>
Date: Wed, 26 Mar 2014 09:43:21 +0100

I will upload a new draft today that incorporates changes from various
reviews.

Philipp

---

From: Philipp Kewisch <[email-masked]>
Date: Wed, 26 Mar 2014 11:03:19 +0100

On 3/21/14, 2:16 PM, Robert Sparks wrote:
>
> On 3/20/14, 6:26 PM, Philipp Kewisch wrote:
> I missed it. Others are likely to as well.
> Building an explicit list of what symbols you're counting on from that
> document
> would let you check that you haven't missed defining any others.
> Putting the list
> in the document will help others later find where to go when they're
> missing the
> symbol by using grep on this document.
> (You should have that list already from the ABNF verification step you
> performed).
>>
>> Philipp
>
That list is quite a bit, I personally think the note at the beginning
of the section should be sufficient. Don't you think most implementers
will assume these rules belong to JSON and re-read the section if they
are really looking for why the rule is missing? If you think this is a
blocking issue, I'm happy to include it though.

; These symbols originate from RFC7159 and are only
begin-array     = ws %x5B ws  ; [ left square bracket
begin-object    = ws %x7B ws  ; { left curly bracket
end-array       = ws %x5D ws  ; ] right square bracket
end-object      = ws %x7D ws  ; } right curly bracket
name-separator  = ws %x3A ws  ; : colon
value-separator = ws %x2C ws  ; , comma
string = quotation-mark *char quotation-mark
quotation-mark = %x22      ; " 
ws = *( %x20 / %x09 / %x0A / %x0D ) ; Space, Horizontal tab, line feed
                                    ; or new line, carriage return
false = %x66.61.6c.73.65   ; false
true  = %x74.72.75.65      ; true
number = [ minus ] int [ frac ] [ exp ]
decimal-point = %x2E       ; . 
digit1-9 = %x31-39         ; 1-9
e = %x65 / %x45            ; e E
exp = e [ minus / plus ] 1*DIGIT
frac = decimal-point 1*DIGIT
int = zero / ( digit1-9 *DIGIT )
minus = %x2D               ; - 
plus = %x2B                ; + 
zero = %x30                ; 0

---

From: Philipp Kewisch <[email-masked]>
Date: Wed, 26 Mar 2014 11:26:46 +0100

On 3/20/14, 11:22 PM, Robert Sparks wrote:
>> numbers should be represented in whatever way needed by the
>> underlying rfc, i.e 5545 or 7159. If a number represented with a
>> leading + sign is found in iCal, the same number can be represented
>> without a leading + sign in JSON. Analogous for leading zeros. Would
>> you prefer this is explicitly stated?
> Yes. And I don't think it needs much text. Just let the implementer
> know the issue is there to take care of.
I've added the following paragraph to the Pre-processing section 3.1:

NEW
          There is a subtle difference in the number representations between
          jCal and iCalendar. While in iCalendar a number may have leading
          zeros, as well as a leading plus sign, this is not the case in
JSON.
          Numbers should be represented in whatever way needed for the
          underlying format.
END

---

From: Philipp Kewisch <[email-masked]>
Date: Wed, 26 Mar 2014 11:43:59 +0100

On 3/26/14, 11:26 AM, Philipp Kewisch wrote:
>>> Maybe this just requires some adjustment in wording. The gist is
>>> that numbers should be represented in whatever way needed by the
>           zeros, as well as a leading plus sign, this is not the case
> in JSON.

Of course that should say:

NEW
          There is a subtle difference in the number representations between
          JSON and iCalendar. While in iCalendar a number may have leading
          zeros, as well as a leading plus sign, this is not the case in
JSON.
          Numbers should be represented in whatever way needed for the
          underlying format.
END

---

From: Philipp Kewisch <[email-masked]>
Date: Wed, 26 Mar 2014 12:31:32 +0100

I have just uploaded a new version of the document, it contains all
considerations from the Gen-ART and secdir review, as well as changes
based on the IESG evaluations.

http://www.ietf.org/internet-drafts/draft-ietf-jcardcal-jcal-10.txt

There are still 1-2 issues where I am waiting on email replies, but I
wanted to have a version ready for the IESG Telechat tomorrow.

To jcarcal folks: I'd apprecicate if you could take a look at
http://trac.tools.ietf.org/wg/jcardcal/trac/ and comment on the
outstanding issues.

Philipp

---

From: Robert Sparks <[email-masked]>
Date: Wed, 26 Mar 2014 08:22:09 -0500

I'll let you work that out with Pete.

If you do decide to list them, I wouldn't repeat their definition.
The important thing is that you verified there wasn't anything left 
dangling.
Thanks for taking the time to go through that again.

RjS

---

From: Philipp Kewisch <[email-masked]>
Date: Wed, 26 Mar 2014 16:25:46 +0100

Ah ok, just listing the names again in a comment in the ABNF is easy enough, thats totally fine with me.

Thanks,
Philipp

> On Mar 26, 2014, at 14:22, Robert Sparks <[email-masked]> wrote:
> 
> The important thing is that you verified there wasn't anything left dangling.

---

From: Peter Saint-Andre <[email-masked]>
Date: Thu, 27 Mar 2014 10:58:25 -0600

> wanted to have a version ready for the IESG Telechat tomorrow.

Have you received replies to those email messages?

> http://trac.tools.ietf.org/wg/jcardcal/trac/ and comment on the

I looked at the issues you posted here:

http://trac.tools.ietf.org/wg/jcardcal/trac/report/1

I think 66 and 68 can be closed because they were fixed in version -10, 
right?

I think 69 deserves to be wontfix, because it's easy enough to find the 
relevant ABNF constructions in RFC 7159. However, for completeness you 
could list them individually. As far as I can see, the constructions we 
include are:

begin-array
begin-object
end-array
end-object
name-separator
value-separator
string
number
true
false

IMHO 67 is a nice little research project but I don't particularly think 
it's necessary to complete that research in order to advance the jCal 
specification.

Peter

---

From: Philipp Kewisch <[email-masked]>
Date: Thu, 27 Mar 2014 18:19:28 +0100

> On Mar 27, 2014, at 17:58, Peter Saint-Andre <[email-masked]> wrote:
> 
I have answered all emails I am aware of. I also mentioned the new version on this gen-art thread. 

> I think 66 and 68 can be closed because they were fixed in version -10, right?
Yes, I believe so too. 

> 
> I think 69 deserves to be wontfix, because it's easy enough to find the relevant ABNF constructions in RFC 7159. However, for completeness you could list them individually. As far as I can see, the constructions we include are:
> 
> begin-array
> begin-object
> end-array
> end-object
> name-separator
> value-separator
> string
> number
> true
> false
I don't mind mentioning these, I can upload a new version with this if you like. Or can we have this added by the editor?

> 
> IMHO 67 is a nice little research project but I don't particularly think it's necessary to complete that research in order to advance the jCal specification.
I agree.

Philipp

---

From: Peter Saint-Andre <[email-masked]>
Date: Thu, 27 Mar 2014 11:40:22 -0600

> I have answered all emails I am aware of. I also mentioned the new version on this gen-art thread.

Great.

> I don't mind mentioning these, I can upload a new version with this if you like. Or can we have this added by the editor?

I think we can handle this as an RFC Editor note (via Pete, our AD) or 
during AUTH48. Feel free to add it to your XML source (or Markdown or 
whatever you use) so you don't forget. :-)

Peter