Re: [RFC] imap-send: escape backslash in password

[Junio C Hamano <gitster@pobox.com>]

Nicolas Morey-Chaisemartin <nicolas@morey-chaisemartin.com> writes:

> Password containing backslashes need to have them doubled to have them properly interpreted by the imap server.

Please wrap this into lines with reasonable lengths like 72 cols.

Is the quoting rules documented somewhere?  If so, please also give
a reference to it here.  RFC3501 "6.2.3 LOGIN Command" does not say
much (other parts of the RFC may specify the rules that apply to
arguments in general, but I didn't look for them).  Without such
reference, it is hard to judge if this change is sufficient or even
correct (in an extreme case, the IMAP server you are talking with
that prompted you to make this change might be in violation).

For example, FRC3501 "9. Formal Syntax" says that both "password"
and "userid" are "astring"; it looks strange that the code with this
patch only touches cred.password while sending cred.username as-is.

> +static char* imap_escape_password(const char *passwd)

In our codebase, asterisk sticks to identifier, not typename.  I.e.

	static char *imap_escape(...)

> +{
> +	const unsigned passwd_len = strlen(passwd);
> +	char *escaped = xmalloc(2 * passwd_len + 1);
> +	const char *passwd_cur = passwd;
> +	char *escaped_cur = escaped;
> +
> +	do {
> +		char *next = strchr(passwd_cur, '\\');
> +
> +		if (!next) {
> +			strcpy(escaped_cur, passwd_cur);
> +		} else {
> +			int len = next - passwd_cur + 1;
> +
> +			memcpy(escaped_cur, passwd_cur, len);
> +			escaped_cur += len;
> +			next++;
> +			*(escaped_cur++) = '\\';
> +		}
> +		passwd_cur = next;
> +	} while(passwd_cur);
> +
> +	return escaped;
> +}

I wonder if we should use strbuf here perhaps like so:

	struct strbuf encoded = STRBUF_INIT;
	const char *p;

	for (p = passwd; *p; p++) {
		if (need_bs_quote(*p))
			strbuf_addch(&encoded, '\\');
		strbuf_addch(&encoded, *p);
	}
	return strbuf_detach(&encoded, NULL);

>  static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *folder)
>  {
>  	struct credential cred = CREDENTIAL_INIT;
> @@ -1090,7 +1116,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, char *f
>  			if (!srvc->user)
>  				srvc->user = xstrdup(cred.username);
>  			if (!srvc->pass)
> -				srvc->pass = xstrdup(cred.password);
> +				srvc->pass = imap_escape_password(cred.password);
>  		}
>  
>  		if (srvc->auth_method) {

Thanks.