-
Notifications
You must be signed in to change notification settings - Fork 332
imapclient: fix parsing of BODYSTRUCTURE responses from Dovecot for message/global #704
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
base: v2
Are you sure you want to change the base?
Conversation
|
Marking as non-standard because this contradicts the spec: Would be nice to report a bug to Dovecot. Also we should add a comment to explain why we deviate from the spec. |
imapclient/fetch.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if err == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
err is ignored when non-nil. We should never throw away an error value: it can happen for many reasons, e.g. I/O error.
Instead, we should only parse the envelope if there's one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can we detect whether the envelope is there or not? All I could figure out to do was to try to parse it and see if I got an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does imapwire.Decoder have any sort of Peek or Unread feature?
|
According to section 6 of RFC 9755, treating message/global as equivalent to message/rfc822 is only required for IMAP4rev2. It's not quite clear to me, but it might not even be allowed under rev1 unless UTF8=ACCEPT has been enabled. |
The BODYSTRUCTURE response for a message/global attachment may or may not contain the data that would be included for message/rfc822. In my experience with Dovecot, it does not. Fixes emersion#678
|
Adding a method to imapwire.Decoder enabled a much cleaner implementation. |
|
I disagree with the "non-standard" tag on this PR. In RFC 3501 (IMAP4rev1), the definition of So this change is required for interoperability with IMAP4rev1 servers. |
BODYSTRUCTURE responses from Dovecot for message/global attachments don't include the extra data that would be included for message/rfc822 attachments. So parsing them was returning an error. This PR makes that data optional.