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

Fix IMAP search issues #1611

Merged
merged 1 commit into from
Jun 5, 2024
Merged

Fix IMAP search issues #1611

merged 1 commit into from
Jun 5, 2024

Conversation

nevans
Copy link
Contributor

@nevans nevans commented May 21, 2024

The code had several issues with existing IMAP UID search:

  • net-imap <= v0.3 returns nil when the server returns nothing. (>= v0.4 returns an empty array when the server returns nothing.)
  • net-imap v0.4.8 and v0.4.9 return a frozen SearchResult. SearchResult inherits from Array, but because it's frozen, mutating it with reverse! results in an exception. See Frozen SearchResult breaks IMAP.find with what: :last option ruby/net-imap#262. (v0.4.10 and above doesn't freeze SearchResult.)
  • newer net-imap (>= 0.5) will support servers that send ESEARCH results, but those will be returned as an ESearchResult struct (probably frozen). IMAP4rev1 servers shouldn't return ESEARCH unless the client has activated that extension, but IMAP4rev2 servers will always return ESEARCH.
  • RFC3501 says nothing about sorting the UIDs that come back from UID SEARCH. Almost all servers do return sorted UIDs, most of the time. But it isn't 100% reliable.

The fix is simple:

  • use #to_a to convert both nil and ESearchResult into an array of UIDs.
  • use #sort to ensure the UIDs are sorted. This also returns a new array, without mutating the original (which may be frozen).

The code had several issues with existing IMAP UID search:
* net-imap <= v0.3 returns `nil` when the server returns nothing.
  (>= v0.4 returns an empty array when the server returns nothing.)
* net-imap v0.4.8 and v0.4.9 return a _frozen_ `SearchResult`.
  `SearchResult` inherits from `Array`, but because it's frozen,
  mutating it with `reverse!` results in an exception.
  See ruby/net-imap#262.
  (v0.4.10 and above doesn't freeze SearchResult.)
* newer net-imap (>= 0.5) will support servers that send `ESEARCH`
  results, but those will be returned as an `ESearchResult` struct
  (probably frozen).  `IMAP4rev1` servers shouldn't return `ESEARCH`
  unless the client has activated that extension, but `IMAP4rev2`
  servers will _always_ return `ESEARCH`.
* RFC3501 says nothing about sorting the UIDs that come back from
  `UID SEARCH`.  Almost all servers do return sorted UIDs, most of the
  time.  But it isn't 100% reliable.

The fix is simple:
* use `#to_a` to convert both `nil` and `ESearchResult` into an array.
* use `#sort` to ensure the UIDs are sorted.  This also returns a new
  array, without mutating the original (which may be frozen).
@nevans
Copy link
Contributor Author

nevans commented Jun 3, 2024

Any thoughts? Would you like me to add some tests to simulate the potential return values?

  • unsorted (depends on the server, any version of net-imap)
  • nil (returned by net-imap <= 0.3)
  • frozen (net-imap, >= 0.4.8, <= 0.4.9, and maybe >= 0.6 or >= 0.7)
  • an extended search result object which implements to_a for backward-compatibility (IMAP4rev2 servers, net-imap >= 0.5)

@eval
Copy link
Collaborator

eval commented Jun 3, 2024

Looks mergeable to me 👍🏻

@eval eval merged commit 1fe6b59 into mikel:master Jun 5, 2024
15 checks passed
@eval
Copy link
Collaborator

eval commented Jun 5, 2024

Thanks for the extensive writeup!

@nevans nevans deleted the fix-imap-search-issues branch June 5, 2024 19:36
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

Successfully merging this pull request may close these issues.

2 participants