Subject: [spfbis] New issue: ABNF Nits

From: Scott Kitterman <[email-masked]>
Date: Tue, 03 Jul 2012 16:39:27 -0400

I ran the RFC 4408 collected ABNF through Bill's ABNF Parser.  I collected a 
few potential issues, but no errors.

Format:

In the current ABNF, mechanisms an modifiers are all defined in a "name" 
"punctuation" styple.  BIll's suggests combining the strings, i.e.:

include = "include"  ":" domain-spec

would become:

include = "include:" domain-spec

Change these?  Leave them?  Don't care?

Defined but not used:

; record defined but not used
; explain-string defined but not used
; header-field defined but not used
; identity defined but not used

Record, explain-string, and header-field are all top level constructs that are 
used in the text, so I believe it's appropriate that they are not used.

identity is, however, used in key, but it is (I believe) improperly quoted.  I 
think (from the current draft):

	   key              = "client-ip" / "envelope-from" / "helo" /
	                      "problem" / "receiver" / "identity" /
	                       mechanism / name

Should be changed to:

	   key              = "client-ip" / "envelope-from" / "helo" /
	                      "problem" / "receiver" / identity /
	                       mechanism / name

I don't think this changes the ABNF, I think it corrects a defect.

None of the other quoted keys are defined in the ABNF.  Should they be?

Undefined terms:

; SP UNDEFINED
; DIGIT UNDEFINED
; ALPHA UNDEFINED

These should probably be defined (added to the imported definitions section).  
Where do I get them from?

Scott K

---

From: "John Levine" <[email-masked]>
Date: 3 Jul 2012 21:46:52 -0000

>Change these?  Leave them?  Don't care?

If we were writing this from scratch, I'd say to change them, but since we
aren't, as a general principle I would avoid changing things that we don't
have to.  If we leave them alone, people comparing 4408 and 4408bis can
instantly tell that nothing changed.  If we combine them, they have to
figure out whether it was purely editorial or the syntax changed.

>I don't think this changes the ABNF, I think it corrects a defect.

That's a bug, your fix looks correct.

>; ALPHA UNDEFINED

I'd suggest you import them from RFC 5234.

R's,
John

---

From: Scott Kitterman <[email-masked]>
Date: Tue, 03 Jul 2012 18:16:02 -0400

> figure out whether it was purely editorial or the syntax changed.

That makes sense.  I left them as they were.

> I'd suggest you import them from RFC 5234.

Perfect.  Changed those.  Attached is the relevant diff I have locally.

Thanks for the quick feedback,

Scott K

--- draft-ietf-spfbis-4408bis-02.txt	2012-06-29 15:01:10.316355215 -0400
+++ draft-ietf-spfbis-4408bis-03.txt	2012-07-03 18:07:59.354907520 -0400
@@ -311,7 +311,9 @@
 
 1.3.2.  Imported Definitions
 
-   The ABNF token "local-part" is defined in [RFC5321].
+   The ABNF tokens "ALPHA", "DIGIT", and "SP" are defined in [RFC5234].
+
+   The token "local-part" is defined in [RFC5321].
 
    "CFWS" and "FWS" are defined in [RFC5322]
 
@@ -2780,25 +2780,81 @@
    key-value-pair   = key [CFWS] "=" ( dot-atom / quoted-string )
 
    key              = "client-ip" / "envelope-from" / "helo" /
-                      "problem" / "receiver" / "identity" /
+                      "problem" / "receiver" / identity /
                        mechanism / name
 
    identity         = "mailfrom"   ; for the "MAIL FROM" identity
                       / "helo"     ; for the "HELO" identity
                       / name       ; other identities
 
+   ALPHA            = <A-Z / a-z as per [RFC5234]>
+   DIGIT            = <0-9 as per [RFC5234]>
+   SP               = <space character as per [RFC5234]>
    dot-atom         = <unquoted word as per [RFC5322]>
    quoted-string    = <quoted string as per [RFC5322]>
    comment          = <comment string as per [RFC5322]>
@@ -3048,7 +3106,8 @@
 
       Try and clarify informational nature of RFC3696
 
+      Fix ABNF nits per Bill's ABNF checker.

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Tue, 3 Jul 2012 20:52:30 -0700

> Change these?  Leave them?  Don't care?

Combine them.

> I don't think this changes the ABNF, I think it corrects a defect.

+1.

> None of the other quoted keys are defined in the ABNF.  Should they be?

If they're quoted, they don't need to be.

> Where do I get them from?

RFC5234.

-MSK

---

From: Scott Kitterman <[email-masked]>
Date: Wed, 04 Jul 2012 03:14:37 -0400

> > Change these?  Leave them?  Don't care?

I think John Levine had a good point about not adding meaningless noise to the 
ABNF diff.  Are you sure?

> > I don't think this changes the ABNF, I think it corrects a defect.

Fixed locally.

> If they're quoted, they don't need to be.

None of the other quoted terms are defined.  Doesn't that mean they'd need to 
be?

> > Where do I get them from?

Done locally.

Scott K

---

From: Alessandro Vesely <[email-masked]>
Date: Wed, 04 Jul 2012 11:20:20 +0200

> Perfect.  Changed those.  Attached is the relevant diff I have locally.

Besides all of them being collected in Appendix A, many also appear in
Section 7.  Why?  Shouldn't they go to Section 1.3.2 instead?

Scott, would you please post -03?  We'll hardly reach version -99 if
you keep changes locally :-)

---

From: Scott Kitterman <[email-masked]>
Date: Wed, 04 Jul 2012 10:46:01 -0400

> Section 7.  Why?  Shouldn't they go to Section 1.3.2 instead?

RFC 4408 didn't have an imported definitions section.  Each imported token was 
listed in the relevant portion of the document.  Appendix A is a collection of 
everything else.  SP, DIGIT, and ALPHA are used in multiple places, so there's 
no single place in the main body to put them.

You are correct that it's a bit inconsistent now in terms of where imports are 
defined.  I've added the items from section 7 to 1.3.2.  I didn't remove them 
from section 7 in the interests of keeping noise out of the ABNF diff (which I 
think is useful for making it clear what did or did not change between 4408 
and 4408bis).

> you keep changes locally :-)

I want to finish reorganizing the security considerations section first, issue 
#6, then I'll post.  Probably later today.

Scott K

---

From: "Murray S. Kucherawy" <[email-masked]>
Date: Wed, 4 Jul 2012 07:47:52 -0700

> I think John Levine had a good point about not adding meaningless noise to

The IESG has the discretion to be extra-sticky about getting ABNF right.
Although the change doesn't alter what productions are permitted, that the
BNF checker whined might be enough to set someone off.  I prefer the
change.  But, to quote Andrew, I'm not going to die on that particular hill.

> None of the other quoted terms are defined.  Doesn't that mean they'd need

I meant "defined" in the ABNF sense.  I think we ought to define them in
prose to explain what they should mean to consumers that see them.

-MSK

---

From: Scott Kitterman <[email-masked]>
Date: Wed, 04 Jul 2012 10:57:12 -0400

> prose to explain what they should mean to consumers that see them.

I see I misread your comment.  I inferred the word after "need to be." was 
quoted, not defined.  We do define them in a prose sense, so I think it's good.

Scott K