Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v1: serialization of SELECT / FETCH flags allows for empty strings #544

Open
ptrcnull opened this issue Sep 10, 2023 · 4 comments
Open

v1: serialization of SELECT / FETCH flags allows for empty strings #544

ptrcnull opened this issue Sep 10, 2023 · 4 comments

Comments

@ptrcnull
Copy link

ptrcnull commented Sep 10, 2023

failed to select mailbox: in response-data: in flag: imapwire: expected atom, got " "

The error originally comes from alps commit 652ea9c7, using go-imap v2.0.0-alpha.6.
Tracing the connection reveals the following exchange:

T4 SELECT INBOX
* FLAGS (\Seen \Answered \Flagged \Deleted \Draft  $Label1 $Label4 $forwarded $label1 \Recent junk nonjunk yeet)

I assume the issue lies in the two spaces between \Draft and $Label1 - however, that's being generated by go-imap v1.2.2-0.20220928192137-6fac715be9cf, running with maddy commit d9920f07.

For the record, RFC 9051 specifies a parenthesized list to be "delimited by space", which might be interpreted ambiguously... but seeing how Thunderbird, Evolution or other mail clients have no issue parsing that response, I imagine it's been assumed that it's valid grammar

@ptrcnull
Copy link
Author

which might be interpreted ambiguously

well, now i realised that same RFC has actual grammar rules in section 9, which say:

SP              = <Defined in RFC 5234> = %x20
flag-list       = "(" [flag *(SP flag)] ")"
mailbox-data    =  "FLAGS" SP flag-list / ...

which would mean that the serialization of flags is wrong, either in go-imap or maddy, and the other clients are just being overly flexible with parsing those

@ptrcnull
Copy link
Author

aaand attaching a debugger to maddy reveals that it's indeed passing an empty string to both Flags and PermanentFlags:

(dlv) p mbox
*github.com/emersion/go-imap.MailboxStatus {
	Name: "INBOX",
	ReadOnly: false,
	Items: map[github.com/emersion/go-imap.StatusItem]interface {} [
		"MESSAGES": nil, 
		"RECENT": nil, 
		"UIDNEXT": nil, 
		"UIDVALIDITY": nil, 
		"UNSEEN": nil, 
	],
	ItemsLocker: sync.Mutex {state: 0, sema: 0},
	Flags: []string len: 14, cap: 20, [
		"\\Seen",
		"\\Answered",
		"\\Flagged",
		"\\Deleted",
		"\\Draft",
		"",
		"$Label1",
		"$Label4",
		"$forwarded",
		"$label1",
		"\\Recent",
		"junk",
		"nonjunk",
		"yeet",
	],
	PermanentFlags: []string len: 15, cap: 24, [
		"\\Seen",
		"\\Answered",
		"\\Flagged",
		"\\Deleted",
		"\\Draft",
		"\\*",
		"",
		"$Label1",
		"$Label4",
		"$forwarded",
		"$label1",
		"\\Recent",
		"junk",
		"nonjunk",
		"yeet",
	],
	UnseenSeqNum: 10906,
	Messages: 11342,
	Recent: 2652,
	Unseen: 39,
	UidNext: 31161,
	UidValidity: 1527719296,
	AppendLimit: 0,}

while i think this should be a scenario that's being handled a little differently by this library, it's a maddy issue after all, so feel free to close it if there's no desire to fix this in some other way

@ptrcnull
Copy link
Author

the same issue exists with Message.Flags - empty string gets serialized as an empty parenthesized list element, which later breaks parsing

@ptrcnull ptrcnull changed the title failed to select mailbox: in response-data: in flag: imapwire: expected atom, got " " v1: serialization of SELECT / FETCH flags allows for empty strings Sep 11, 2023
@emersion
Copy link
Owner

emersion commented Oct 5, 2023

FWIW, go-imap v2's server would error out in this situation:

return len(s) > 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants