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

wrong identifier used in imap call #2789

Closed
nicklasaven opened this issue Jul 25, 2018 · 5 comments
Closed

wrong identifier used in imap call #2789

nicklasaven opened this issue Jul 25, 2018 · 5 comments

Comments

@nicklasaven
Copy link
Contributor

I might be wrong, but I think this looks like a bug.
If so, fixing it will break things.

More explanation here:
https://curl.haxx.se/mail/archive-2018-07/0006.html

The imap-protocol have 2 different identifiers
of an email.

  1. UID
    https://tools.ietf.org/html/rfc3501#section-2.3.1.1
  2. Message Sequence Number Message Attribute
    https://tools.ietf.org/html/rfc3501#section-2.3.1.2

If emails are deleted from the mail box those will differ.

If I for example have had 100 mails to my mail box in total, but have deleted 90 of them
the last email will still have UID = 100, but the Message Sequence Number will be 10.

so, to get the last email I would according to the spec:
https://tools.ietf.org/html/rfc5092#section-9

do:

imaps://the.web.server/INBOX;UID=100;SECTION=HEADER.FIELDS%20(SUBJEC 
T)

but that doesn't work in cURL. In cURL I do instead get the last email with:

imaps://the.web.server/INBOX;UID=10;SECTION=HEADER.FIELDS%20(SUBJEC 
T)

This seams wrong, and according to this part of the specification the URL should be converted to
UID FETCH
instead of just FETCH as now.

SO, this would fix it, but would break applications relying on todays behavior

diff --git a/lib/imap.c b/lib/imap.c 
index 942fe7d1e..7cf4d17f4 100644 
--- a/lib/imap.c 
+++ b/lib/imap.c 
@@ -692,12 +692,12 @@ static CURLcode imap_perform_fetch(struct 
connectdata *conn) 
  
   /* Send the FETCH command */ 
   if(imap->partial) 
-    result = imap_sendf(conn, "FETCH %s BODY[%s]<%s>", 
+    result = imap_sendf(conn, "UID FETCH %s BODY[%s]<%s>", 
                         imap->uid, 
                         imap->section ? imap->section : "", 
                         imap->partial); 
   else 
-    result = imap_sendf(conn, "FETCH %s BODY[%s]", 
+    result = imap_sendf(conn, "UID FETCH %s BODY[%s]", 
                         imap->uid, 
                         imap->section ? imap->section : ""); 

For example the example in the bottom of this link will break:
https://debian-administration.org/article/726/Performing_IMAP_queries_via_curl

since the correct behavior don't expect all UID to be present (not the deleted) But at the other hand the UID will be persistent.

@nicklasaven
Copy link
Contributor Author

Ah, I have missed this thread discussing this several years ago. I thought I had googled enough.

The thread says that todays behavior is ok in the standard. I have some problems seeing that (will have to read and reread some more).

But I absolutely agree with the need of backward compability.

@bagder
Copy link
Member

bagder commented Jul 30, 2018

I absolutely agree with the need of backward compatibility.

Well, since we can't really do both ways by default we just have to decide which way is default and then offer an option that provides the old way/new way. I'd say it would probably be a good idea to do the existing (old) way only if a specific option is set.

Then we can probably at the same time bring up a discussion about deprecating this new option if we truly deem the new way to be the "one true way" of how things are done IMAP wise.

@bagder
Copy link
Member

bagder commented Jul 30, 2018

Feel free to work on a PR to get this work started!

@nicklasaven
Copy link
Contributor Author

Yes, I will give it a try.

I missed to copy the link in my last message. Here is the discussion I was referring to
https://curl.haxx.se/mail/lib-2014-04/0069.html

nicklasaven added a commit to nicklasaven/curl that referenced this issue Jul 31, 2018
As described in curl#2789, this is a suggested solution.
Changing UID=xx to actually get mail with UID xx and add "MAILINDEX"
to get a mail with a special index in the mail box (old behavior).
So MAILINDEX=1 gives the first non deleted mail in the mail box.
@nicklasaven
Copy link
Contributor Author

nicklasaven commented Jul 31, 2018

Added a pull request for the discussion.

It is a suggestion how to handle it.

In addition to UID= I added MAILINDEX=

I don't know if "MAILINDEX" is a good word, but as far as I have seen there is no wording for this in the spec.

So, with the patch UID= will get the mail with the requested UID

The difference is showing when mails are deleted in the mail box.

If we have received in total 2 mails in the mail box and the first is deleted:
UID=1 will not give any mail back, while MAILINDEX=1 will give the one mail that is in the mail box.

At the other hand UID=2 will give the same mail both before and after deleting the first mail.

nicklasaven added a commit to nicklasaven/curl that referenced this issue Aug 8, 2018
As described in curl#2789, this is a suggested solution.
Changing UID=xx to actually get mail with UID xx and add "MAILINDEX"
to get a mail with a special index in the mail box (old behavior).
So MAILINDEX=1 gives the first non deleted mail in the mail box.
@bagder bagder closed this as completed in 6987fce Sep 6, 2018
falconindy pushed a commit to falconindy/curl that referenced this issue Sep 10, 2018
... and add "MAILINDEX".

As described in curl#2789, this is a suggested solution.  Changing UID=xx to
actually get mail with UID xx and add "MAILINDEX" to get a mail with a
special index in the mail box (old behavior).  So MAILINDEX=1 gives the
first non deleted mail in the mail box.

Fixes curl#2789
Closes curl#2815
@lock lock bot locked as resolved and limited conversation to collaborators Dec 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

2 participants