-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
IMAP: doesn't quote atoms if they contain quotes! #1902
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
Comments
Are you saying that the password you sent contained one of those symbols but curl didn't send it quoted? I just tested with Can you show us a protocol trace of what curl did and what you expected curl to send instead? |
you can use password following: |
@bagder What he's referring to is imap_atom the string is only wrapped in double quotes when Lines 1745 to 1754 in 8839c05
According to the IMAP RFC:
(CTL is I can only find one reference to atom-specials in the entire RFC:
It would seem, at least in that case, that escape_only when atom-specials are present may not be appropriate unless the calling function is going to quote the string. I think we could expand our character set of atom-specials at least. Whether the password should be sent as |
This doesn't match my test. I've modified test 800 to use this command line:
It makes curl send this line:
Surely the double quote used in the atom needs to be backslash-escaped ? |
@jay wrote:
The only current user of |
Aha! When using a string like |
To sum up, if the username or password is included " or \, not included one of (){ %*] , curl will sends password without double quotes. |
Sure, I'm sure some clients default to always quoting but that's a different thing. The IMAP protocol doesn't mandate quoting for a string like |
Maybe this? --- a/lib/imap.c
+++ b/lib/imap.c
@@ -1795,20 +1795,20 @@ static char *imap_atom(const char *str, bool escape_only)
/* Does the input contain any "atom-special" characters? */
if(!backsp_count && !quote_count && !others_exists)
return strdup(str);
/* Calculate the new string length */
- newlen = strlen(str) + backsp_count + quote_count + (others_exists ? 2 : 0);
+ newlen = strlen(str) + backsp_count + quote_count + (escape_only ? 0 : 2);
/* Allocate the new string */
newstr = (char *) malloc((newlen + 1) * sizeof(char));
if(!newstr)
return NULL;
/* Surround the string in quotes if necessary */
p2 = newstr;
- if(others_exists) {
+ if(!escape_only) {
newstr[0] = '"';
newstr[newlen - 1] = '"';
p2++;
}
|
... as the test cases themselves do that and it makes it easier to add crazy test cases. Test 800 updated to use user name + password that need quoting. Test 856 updated to trigger an auth fail differently. Ref: #1902
Updates test 800 to verify Fixes #1902
if you change it like this, |
Correct, and that's on purpose. Are you saying that "coremail" requires the password to be quoted? If so, isn't that then a bug in the server that will cause problems to more clients than just curl? |
... as the test cases themselves do that and it makes it easier to add crazy test cases. Test 800 updated to use user name + password that need quoting. Test 856 updated to trigger an auth fail differently. Ref: #1902
There is a bug for imap protocol:
I did this:
Only with these symbols
(){ %*]
can add double quotes at password both, if passworld includes symbol " or \ but exclude(){ %*]
, it doesn't. This will cause some mail servers(for example: coremail server) to fail to access properly.I expected the following:
The password should be added symbol " to both ends, whether or not it contains special symbols.
in file imap.c , function imap_atom(), set bool others_exists = TRUE, will resolve it.
libcurl version : 7.55.1
operating system : linux
The text was updated successfully, but these errors were encountered: