Hi Peter/Barry,

Thanks for the detailed review. 

We will go through the review comments and update the draft accordingly. 

Regards,
Jay
-----Original Message-----
From: imapext [mailto:[email protected]] On Behalf Of Barry Leiba
Sent: Sunday, December 27, 2015 10:18 PM
To: Peter Yee
Cc: [email protected]; General Area Review 
Team; [email protected]
Subject: Re: [imapext] FW: Gen-ART LC review of 
draft-ietf-imapapnd-appendlimit-extension-07

Hi, Peter, and thanks for the review.  I'm adding the working group mailing 
list here, so they have a record of this.  My comments below.

> I am the assigned Gen-ART reviewer for this draft.  The General Area 
> Review Team (Gen-ART) reviews all IETF documents being processed by 
> the IESG for the IETF Chair.  Please treat these comments just like 
> any other last call comment.  For background on Gen-ART, please see 
> the FAQ at <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>
>
> Document: draft-ietf-imapapnd-appendlimit-extension-07
> Reviewer: Peter Yee
> Review Date: December 27, 2015
> IETF LC End Date: January 1, 2016
> IESG Telechat date: TBD
>
> Summary: This draft is basically ready for publication as a standards 
> track RFC, but has nits that should be fixed before publication. 
> [Ready with nits]
>
> The draft describes an extension to IMAP4v1 that allows a server to 
> signal a maximum message upload size limit.
>
> Most of nits noted are linguistic, although there's a minor, repeated 
> mistake in the ABNF that's critical to fix.
>
> Comments/Questions:
>
> Section 1, 2nd paragraph, 2nd sentence: the claim that this extension 
> allows a server to avoid processing overly large messages (or 
> attachments) is only true if a client implements and honors the 
> extension.  A malicious client could still upload large messages and 
> cause the server to process the message up to the point where it 
> exceeds the server's limit.  While these overly large uploads would 
> not be saved to disk, the server would still have to process them up 
> to a point in order to determine that they should be discarded and a 
> TOOBIG response returned.  Other mechanisms would be needed to fend off 
> malicious clients that persist in such uploads.

Indeed; as with any IMAP extension, this helps only compliant client-server 
combinations.  I think the Security Considerations adequately covers the issue 
of a malicious client.

> Page 6, Section 6, 2nd full sentence: In light of the last paragraph 
> of section 5 indicating that the number is a fixed maximum value, how 
> would submitting a little too large message work?  Why is the server 
> being lenient here?

The point is to warn against that sort of leniency.  One might think that if 
you set a limit of, say, 2 MB, that you might act harshly to a client that 
tries to post 6 MB, but not be so strict when a client posts 2.1 MB.  The text 
warns that such leniency might be used to enable an attack.

> Nits:

I noted in my AD review (posted to the imapext mailing list) that the document 
is in need of a significant review for English corrections.
In discussion of that, we decided that the RFC Editor will have to handle that: 
the authors have done a great job on this, but are not native English writers, 
and we don't have natives at the ready to help.

Authors, Peter has provided, below, a good list to help you with this.
If you can incorporate the changes he suggests, it'll help a lot, and the RFC 
Editor can deal with any that remain.

One that I need to call out that's buried in Peter's list, the one he mentions 
about ABNF (and this one is my fault, as I provided the current version of ABNF 
and made the typo myself):

> Page 5, Section 5, ABNF: change "/=" to "=/" for the definitions of 
> "capability", "status-att", and "status-att-val".

Yes.  I think I make that goof regularly.  Sigh.  Thanks for catching it.

Barry


> Page 1, Abstract, 1st sentence: change "mail" to "message".  Delete "of".
>
> Page 2, Section 1, 1st paragraph, 1st sentence: change "mail" to "message".
>
> Page 2, Section 1, 1st paragraph, 4th sentence: change "mail" to "message".
> Change "attachment" to "attachments".

Actually, make it "messages".

> Page 2, Section 1, 2nd paragraph, 1st sentence: insert "a" before "maximum".
> Insert "the" before "email".
>
> Page 2, Section 1, 2nd paragraph, 2nd sentence: change "server side" 
> to "server-side".
>
> Page 3, Section 2, 1st paragraph, 1st sentence: insert "the" before 
> the first "APPENDLIMIT".  Insert "the" before "authenticated".
>
> Page 3, Section 2, 1st paragraph, last sentence: insert "An" at the 
> beginning of the sentence.
>
> Page 3, Section 2, 1st paragraph after (a), 1st sentence: delete "the"
> before "mailboxes".
>
> Page 3, Section 2, 1st paragraph after (a), 2nd sentence: insert "the"
> before "same".
>
> Page 3, Section 2, 3rd paragraph after (b), 1st sentence: insert "an" 
> before "APPENDLIMIT".  Insert "a" before "STATUS".
>
> Page 3, Section 2, 3rd paragraph after (b), 2nd sentence: change "New" 
> to "A new".  Change "mailbox specific" to "mailbox-specific".
>
> Page 3, Section 2, 3rd paragraph after (b), 3rd sentence: insert "to" 
> before "section".  Insert "the" before "response".
>
> Page 3, Section 2, last paragraph, 1st sentence: insert "An" at the 
> beginning of the sentence.  Delete "kind of".
>
> Page 3, Section 2, last paragraph, 2nd sentence: insert "a" before "client".
> Insert "the" before "advertised".
>
> Page 3, Section 3, heading: change "Mailbox specific" to "Mailbox-specific".
>
> Page 3, Section 3, 1st paragraph: insert "the" before "CAPABILITY".
>
> Page 4, Section 3.1, 1st paragraph, 1st sentence: insert "a" before 
> "STATUS".
>
> Page 4, Section 3.1, 1st paragraph, 2nd sentence: insert "An" before "IMAP".
> Insert "a" before "STATUS".  Insert "an" before "APPENDLIMIT".  Change 
> "mailbox specific" to "mailbox-specific".
>
> Page 4, Section 3.1, 1st paragraph, 3rd sentence: delete the comma.
>
> Page 4, Section 3.2, 1st paragraph, 2nd sentence: delete the comma.

Actually, this can go wither way, and because of the lengths of the clauses 
here, I think the comma should stay.  We can let the RFC Editor weigh in on it 
with their final edits.

> Page 5, Section 4, 1st paragraph, 1st sentence: insert "a" before "client".
> Change "mail" to "message".  Change "to" to "for" before "that".  
> Insert "the" before "server".
>
> Page 5, Section 4, 1st paragraph, 2nd sentence: insert "to" before 
> "[RFC4469]".  Change "(4) to "4".
>
> Page 5, Section 4, 2nd paragraph, 1st sentence: change "Client" to "A 
> client".  Insert "the" before "maximum".
>
> Page 5, Section 4, 2nd paragraph, 2nd sentence: insert "to" before 
> "section".
>
> Page 5, Section 5, ABNF: change "/=" to "=/" for the definitions of 
> "capability", "status-att", and "status-att-val".
>
> Page 6, Section 8: append a comma after "Long".
>

_______________________________________________
imapext mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/imapext

_______________________________________________
Gen-art mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/gen-art

Reply via email to