imapserver: advertise APPENDLIMIT capability #692
Conversation
| // DiscloseLimit indicates whether the limit should be advertised in the CAPABILITY | ||
| // response. If false, only "APPENDLIMIT" will be listed, without the actual limit. | ||
| // If true, "APPENDLIMIT=<limit>" will be listed. | ||
| DiscloseLimit() bool |
There was a problem hiding this comment.
What's the purpose of this method? Servers can already reject literals which are too large in APPEND without advertising a limit (by returning an error).
There was a problem hiding this comment.
APPENDLIMIT capability can be advertised as APPENDLIMIT only or with explicit limit APPENDLIMIT=. In first you have to try to discover the limit while in the second you know before you try. The method allows to server to configure whether it will advertise it or not.
There was a problem hiding this comment.
Hm, I'm not sure I'm following. If APPENDLIMIT is listed in Caps, then we already know that the server wants to disclose the limit?
How about the following:
- If
AppendLimitis not listed inCaps, never advertiseAppendLimit - If
AppendLimitis listed inCaps:- If
SessionAppendLimitis implemented and returns a non-zero value, advertiseAPPENDLIMIT=n - Otherwise, advertise
APPENDLIMITwithout a value
- If
| } else { | ||
| caps = append(caps, imap.CapAppendLimit) | ||
| } | ||
| } else if limit, ok := available.AppendLimit(); ok { |
There was a problem hiding this comment.
This adds two ways to set the global limit, either with a SessionAppendLimit, either with manual string formatting in Caps. I'd prefer to keep only the former.
| // Capabilities which require backend support and apply to both | ||
| // IMAP4rev1 and IMAP4rev2 |
There was a problem hiding this comment.
Can we keep this comment next to the addAvailableCaps list below?
| @@ -86,6 +88,21 @@ func (c *Conn) availableCaps() []imap.Cap { | |||
|
|
|||
| // Capabilities which require backend support and apply to both | |||
| // IMAP4rev1 and IMAP4rev2 | |||
| if appendLimitSession, ok := c.session.(SessionAppendLimit); ok { | |||
There was a problem hiding this comment.
Per the RFC, we should only advertise a limit in the authenticated state.
| } | ||
|
|
||
| // NewUserSessionWithAppendLimit creates a new user session with a custom append limit. | ||
| func NewUserSessionWithAppendLimit(user *User, appendLimit uint32, discloseLimit bool) *UserSession { |
There was a problem hiding this comment.
Could we keep this simple and only have a single global append limit in imapmemserver?
No description provided.