[evolution-patches] Use of invalid HELO.

[David Woodhouse <dwmw2 infradead org>]

Evolution happily attempts to use the domain name given by DNS even if
it's invalid and contains underscores, etc. OK, admittedly the reverse
DNS _shouldn't_ contain such things, but when on free wavelan in an
airport one can't be picky. So let's make Evo validate the result of
getnameinfo() and pretend there was no result if it would be invalid
according to RFC2821...

This problem also highlighted two other bugs -- firstly,
camel_getnameinfo() appears to return an empty string on failure
sometimes, and smtp_helo() would leak it. I've worked around this by
checking for that case in smtp_helo() -- but I suspect the right answer
may be to change camel_getnameinfo() to not do that. Someone more
familiar with the other callers needs to make that call though.

The second bug is a usability problem -- I actually had to log in to the
mail server and view its logs, or run Evolution with debugging enabled
in a terminal and observe the SMTP conversation which said:

550-Mail rejected. Underscores in HELO are not permitted by RFC2821.
550-Ask your local system administrator to fix your broken mail server.
550-For more information see http://www.ietf.org/rfc/rfc2821.txt or
550 mail postmaster infradead org

Instead of displaying that message, Evolution simply made something up
instead -- it said "Requested action not taken: mailbox unavailable".
I'll look into fixing that bug later.

I know this has been discussed before, and it was pointed out that
messages from my own local mail server aren't translatable -- but still,
surely it's better to display the message from my _local_ mailserver
untranslated than it is to just make something up?

--- evolution-data-server-1.2.0/camel/providers/smtp/camel-smtp-transport.c~	2005-01-24 09:16:34.000000000 -0700
+++ evolution-data-server-1.2.0/camel/providers/smtp/camel-smtp-transport.c	2005-03-16 09:23:30.000000000 -0700
@@ -875,6 +875,39 @@ smtp_set_exception (CamelSmtpTransport *
 	}
 }
 
+static gboolean smtp_helo_is_valid(char *name)
+{
+	enum { ALNUM, DASH, DOT } state = DOT;
+	int dotseen = 0;
+
+	while (*name) {
+		switch(state) {
+		case ALNUM:
+			if (*name == '-') {
+				state = DASH;
+				break;
+			} else if (*name == '.') {
+				dotseen = 1;
+				state = DOT;
+				break;
+			} /* else ... */
+		case DOT:
+		case DASH:
+			if (!isalnum(*name))
+				return FALSE;
+			state = ALNUM;
+			break;
+		}
+		name++;
+	}
+
+	/* If it didn't end with an alphanumeric character, or there were no dots, it's invalid */
+	if (state != ALNUM || !dotseen)
+		return FALSE;
+	else
+		return TRUE;
+}
+
 static gboolean
 smtp_helo (CamelSmtpTransport *transport, CamelException *ex)
 {
@@ -896,8 +929,19 @@ smtp_helo (CamelSmtpTransport *transport
 	
 	camel_operation_start_transient (NULL, _("SMTP Greeting"));
 
-	/* force name resolution first, fallback to numerical, we need to know when it falls back */
-	if (camel_getnameinfo(transport->localaddr, transport->localaddrlen, &name, NULL, NI_NAMEREQD, NULL) != 0) {
+	/* force name resolution first, but check the resulting name is valid according to RFC2821 */
+	if (camel_getnameinfo(transport->localaddr, transport->localaddrlen, &name, NULL, NI_NAMEREQD, NULL) == 0 &&
+	    !smtp_helo_is_valid(name)) {
+		g_free(name);
+		name = NULL;
+	}
+	/* camel_getnameinfo() can return an empty string on failure, bizarrely */
+	if (name && !*name) {
+		g_free(name);
+		name = NULL;
+	}
+	/* If the name lookup failed, or the name wasn't acceptable, then try to use a numeric HELO domain */
+	if (!name) {
 		if (camel_getnameinfo(transport->localaddr, transport->localaddrlen, &name, NULL, NI_NUMERICHOST, NULL) != 0)
 			name = g_strdup("localhost.localdomain");
 		else {


-- 
dwmw2