After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 688639 - SMTP doesn't deal with 535 Authentication error correctly
SMTP doesn't deal with 535 Authentication error correctly
Status: RESOLVED FIXED
Product: evolution-data-server
Classification: Platform
Component: Mailer
3.4.x (obsolete)
Other Linux
: Normal normal
: ---
Assigned To: evolution-mail-maintainers
Evolution QA team
Depends on:
Blocks:
 
 
Reported: 2012-11-19 14:03 UTC by Paul Menzel
Modified: 2012-11-20 18:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH] Bug 688639 – SMTP: Correctly deal with 535 authentication error (2.34 KB, patch)
2012-11-20 14:12 UTC, Paul Menzel
committed Details | Review

Description Paul Menzel 2012-11-19 14:03:18 UTC
The following code from `camel/providers/smtp/camel-smtp-transport.c` [1] does not deal correctly with the case, when getting 535 response.

	/* If our authentication data was rejected, destroy the
	 * password so that the user gets prompted to try again. */
	if (strncmp (respbuf, "535", 3) == 0)
		result = CAMEL_AUTHENTICATION_REJECTED;
	else
		result = CAMEL_AUTHENTICATION_ACCEPTED;

It should go to `exit` here as …

	/* Catch any other errors. */
	if (strncmp (respbuf, "235", 3) != 0) {
		g_set_error (
			error, CAMEL_SERVICE_ERROR,
			CAMEL_SERVICE_ERROR_CANT_AUTHENTICATE,
			_("Bad authentication response from server."));
		goto lose;
	}

otherwise the if statement above takes precedent.

	goto exit;

break_and_lose:
	/* Get the server out of "waiting for continuation data" mode. */
	d(fprintf (stderr, "sending : *\n"));
	camel_stream_write (transport->ostream, "*\r\n", 3, cancellable, NULL);
	respbuf = camel_stream_buffer_read_line (
		CAMEL_STREAM_BUFFER (transport->istream), cancellable, NULL);
	d(fprintf (stderr, "received: %s\n", respbuf ? respbuf : "(null)"));

lose:
	result = CAMEL_AUTHENTICATION_ERROR;

exit:
	camel_operation_pop_message (cancellable);

	g_object_unref (sasl);
	g_free (respbuf);

	return result;

I will cook up a patch shortly.

[1] http://git.gnome.org/browse/evolution-data-server/tree/camel/providers/smtp/camel-smtp-transport.c?id=ea8bc94c312777a26d5039d85f994dc7aa13c26b#n565
Comment 1 Paul Menzel 2012-11-19 14:09:57 UTC
Before commit [1]

    commit 68c269a66bd484768b9c1517e7dc2dd30f682485
    Author: Matthew Barnes <mbarnes@redhat.com>
    Date:   Sat Oct 15 09:51:35 2011 -0400

        smtp: Adapt to Camel's new authentication API.

the correspending code reset the password.

-       /* Work around broken SASL implementations. */
-       if (auth_challenge && strncmp (respbuf, "334", 3) == 0)
-               goto broken_smtp_server;
-
-       /* If our authentication data was rejected, destroy the
-        * password so that the user gets prompted to try again. */
-       if (strncmp (respbuf, "535", 3) == 0)
-               camel_service_set_password (service, NULL);
-
-       /* Catch any other errors. */
-       if (strncmp (respbuf, "235", 3) != 0) {
-               g_set_error (
-                       error, CAMEL_SERVICE_ERROR,
-                       CAMEL_SERVICE_ERROR_CANT_AUTHENTICATE,
-                       _("Bad authentication response from server."));
-               goto lose;
-       }

[1] http://git.gnome.org/browse/evolution-data-server/commit/camel/providers/smtp/camel-smtp-transport.c?id=68c269a66bd484768b9c1517e7dc2dd30f682485
Comment 2 Milan Crha 2012-11-19 15:17:31 UTC
(In reply to comment #0)
>     /* If our authentication data was rejected, destroy the
>      * password so that the user gets prompted to try again. */
>     if (strncmp (respbuf, "535", 3) == 0)
>         result = CAMEL_AUTHENTICATION_REJECTED;
>     else
>         result = CAMEL_AUTHENTICATION_ACCEPTED;
> 
> It should go to `exit` here as …
> 
>     /* Catch any other errors. */
>     if (strncmp (respbuf, "235", 3) != 0) {
>         g_set_error (

Either that, of rather, less goto-like,
     if (strncmp (respbuf, "535", 3) == 0)
         result = CAMEL_AUTHENTICATION_REJECTED;
     else if (strncmp (respbuf, "235", 3) == 0)
         result = CAMEL_AUTHENTICATION_ACCEPTED;
     else {
         /* Catch any other errors. */
         g_set_error (
Comment 3 Paul Menzel 2012-11-20 14:12:25 UTC
Created attachment 229473 [details] [review]
[PATCH] Bug 688639 – SMTP: Correctly deal with 535 authentication error

Please apply this patch, which I could successfully test applied to Evolution Data Server 3.4.4. It would be great if you could apply it also to the gnome-3-4 branch.

Also, I would prefer to also commit the elaborate commit message explaining the problem and the solution.
Comment 4 Milan Crha 2012-11-20 18:02:45 UTC
Thanks for the patch, it looks fine. The only thing, gnome-3-4 is dead, thus I'm committing to gnome-3-6 and master "only".

Created commit 3429da8 in eds master (3.7.3+)
Created commit 77d7277 in eds gnome-3-6 (3.6.3+)