ValidationError & ValidationErrorKind::Invalid is useless without information of what the invalid value and what type was expected

[epilys]

ValidationError & ValidationErrorKind::Invalid is useless without information of what the invalid value and what type was expected.

Both types derive traits which would break if we added extra information:

imap-codec/imap-types/src/error.rs

Lines 9 to 13 in 086523d

	/// This error can be returned during validation of a value, e.g., a tag, atom, etc.
	#[derive(Clone, Debug, Eq, Error, Hash, Ord, PartialEq, PartialOrd)]
	pub struct ValidationError {
	kind: ValidationErrorKind,
	}

But is a validation error really equal to another validation error if the value domain and/or the value are different? 🤔 Do these errors need to be hashable/orderable/comparable?

[epilys]

These are the server/client transactions intermixed with error/trace logs that prompted this post, for posterity:

S: * 1 FETCH (UID 246 MODSEQ (9563485) FLAGS (\\Deleted))\r\n
Mon, 21 Oct 2024 02:28 [DEBUG]: melib::imap::untagged: fetch uid 246 (Flag(TRASHED), [])
Mon, 21 Oct 2024 02:28 [TRACE]: melib::imap::connection: ImapType::new sent: M62 ExpungeUid { sequence_set: SequenceSet([Single(Value(246))]+) }
C: M62 UID EXPUNGE 246\r\n
S: * 1 EXPUNGE\r\n
S: * 0 EXISTS\r\n
S: M62 OK Success\r\n
Mon, 21 Oct 2024 02:28 [TRACE]: meli: "IMAP transaction validation failed\nCaused by:\n[2] Validation failed: Invalid value"

[duesee]

These... are not well thought-out :-) We can review and refactor for v2.0.0! The error message is really not great.

[epilys]

@duesee any clue what part of that imap session would trigger this error?

[HenningHolmDE]

I just tried throwing the responses into the corresponding examples. I may have overlooked something, but for me, it seems like * 1 FETCH (UID 246 MODSEQ (9563485) FLAGS (\\Deleted))\r\n the cannot be parsed because of the MODSEQ response data item.

@duesee Does the crate already fully implement RFC 4551 (IMAP Extension for Conditional STORE Operation or Quick Flag Changes Resynchronization)? (see https://www.rfc-editor.org/rfc/rfc4551.html#section-3.3.2 for the FETCH response)

(I tried enabling the ext_condstore_qresync feature, which did not change anything.)

@epilys As we cannot see this on the log snippet: Did the client command include a MODSEQ message data item or did the server just reply with them?

[epilys]

@HenningHolmDE The command did include the MODSEQ item, yes. I should have included it but the items where what the server replied with.

[duesee]

@epilys I think that the error tells that some data meli wanted to put into a command is invalid. imap-codec then refuses to create such a command with this error. M63 would be interesting to dissect, I think. Can you find which command it would be? Maybe even dbg!() the parameters?

[epilys]

Ugh, looked at it again and it seems like meli handles untagged EXISTS wrongly. Upon an * n EXISTS response it does CommandBody::fetch(n, common_attributes(), false) in

https://github.com/meli/meli/blob/b2200ec3abe9c5649d2628f88afe636f02a91be6/melib/src/imap/untagged.rs#L239-L245

instead of calculating a message sequence set compared to the previous EXISTS value, if there was any, like recommended in RFC9051 2.3.1.2 Message Sequence Number Message Attribute

In addition to accessing messages by relative position in the mailbox, message sequence numbers can be used in mathematical calculations. For example, if an untagged "11 EXISTS" is received, and previously an untagged "8 EXISTS" was received, three new messages have arrived with message sequence numbers of 9, 10, and 11. As another example, if message 287 in a 523-message mailbox has UID 12345, there are exactly 286 messages that have lesser UIDs and 236 messages that have greater UIDs.

So in this case the error must have been that a FETCH with a value of 0 was performed.

[duesee]

Makes sense!

[duesee]

I'll try to see how we could have done this obvious in the first place, like, better error message :-)

[epilys]

It should show

• context (e.g. "command body for fetch command")
• the invalid value (e.g. "0")
• the value field/item/title/descriptor (e.g. "message sequence argument of fetch body")
• the valid value domain (e.g. "non zero 32bit sized integer, or range, etc")