dovecot-2.2: auth: Code cleanup: Merged extra_cache_fields into ...

dovecot at dovecot.org dovecot at dovecot.org
Wed Jan 30 22:17:28 EET 2013


details:   http://hg.dovecot.org/dovecot-2.2/rev/cc4472f02f70
changeset: 15684:cc4472f02f70
user:      Timo Sirainen <tss at iki.fi>
date:      Wed Jan 30 19:46:58 2013 +0200
description:
auth: Code cleanup: Merged extra_cache_fields into extra_fields.
They are separated using a hidden-flag in the extra field. This required a
new implementation for auth-streams.

diffstat:

 src/auth/auth-master-connection.c |    5 +-
 src/auth/auth-request-handler.c   |    7 +-
 src/auth/auth-request.c           |   53 +++++------
 src/auth/auth-request.h           |    3 -
 src/auth/auth-stream.c            |  170 +++++++++++++++++++------------------
 src/auth/auth-stream.h            |   24 ++++-
 src/auth/auth-worker-client.c     |   14 +--
 src/auth/db-checkpassword.c       |   21 ++--
 src/auth/userdb-blocking.c        |    2 +-
 9 files changed, 153 insertions(+), 146 deletions(-)

diffs (truncated from 596 to 300 lines):

diff -r f11ae64365b5 -r cc4472f02f70 src/auth/auth-master-connection.c
--- a/src/auth/auth-master-connection.c	Wed Jan 30 19:11:30 2013 +0200
+++ b/src/auth/auth-master-connection.c	Wed Jan 30 19:46:58 2013 +0200
@@ -275,7 +275,7 @@
 		str_printfa(str, "USER\t%u\t", auth_request->id);
 		str_append_tabescaped(str, auth_request->user);
 		str_append_c(str, '\t');
-		auth_stream_reply_append(auth_request->userdb_reply, str);
+		auth_stream_reply_append(auth_request->userdb_reply, str, FALSE);
 		break;
 	}
 
@@ -325,7 +325,8 @@
 		str_append_tabescaped(str, auth_request->user);
 		if (!auth_stream_is_empty(auth_request->extra_fields)) {
 			str_append_c(str, '\t');
-			auth_stream_reply_append(auth_request->extra_fields, str);
+			auth_stream_reply_append(auth_request->extra_fields,
+						 str, FALSE);
 		}
 		break;
 	case PASSDB_RESULT_USER_UNKNOWN:
diff -r f11ae64365b5 -r cc4472f02f70 src/auth/auth-request-handler.c
--- a/src/auth/auth-request-handler.c	Wed Jan 30 19:11:30 2013 +0200
+++ b/src/auth/auth-request-handler.c	Wed Jan 30 19:46:58 2013 +0200
@@ -170,7 +170,7 @@
 		return;
 
 	str_append_c(dest, '\t');
-	auth_stream_reply_append(request->extra_fields, dest);
+	auth_stream_reply_append(request->extra_fields, dest, FALSE);
 
 	if (request->proxy && !request->auth_only) {
 		/* we're proxying */
@@ -338,7 +338,8 @@
 		if (reply_size > 0) {
 			str = t_str_new(MAX_BASE64_ENCODED_SIZE(reply_size));
 			base64_encode(auth_reply, reply_size, str);
-			auth_stream_reply_add(request->extra_fields, "resp", str_c(str));
+			auth_stream_reply_add(request->extra_fields, "resp",
+					      str_c(str), 0);
 		}
 		ret = auth_request_proxy_finish(request,
 				auth_request_handler_proxy_callback);
@@ -635,7 +636,7 @@
 	case USERDB_RESULT_OK:
 		str_printfa(str, "USER\t%u\t", request->id);
 		str_append_tabescaped(str, request->user);
-		auth_stream_reply_append(request->userdb_reply, str);
+		auth_stream_reply_append(request->userdb_reply, str, FALSE);
 
 		if (request->master_user != NULL &&
 		    !auth_stream_reply_exists(request->userdb_reply,
diff -r f11ae64365b5 -r cc4472f02f70 src/auth/auth-request.c
--- a/src/auth/auth-request.c	Wed Jan 30 19:11:30 2013 +0200
+++ b/src/auth/auth-request.c	Wed Jan 30 19:46:58 2013 +0200
@@ -427,11 +427,7 @@
 
 	if (!auth_stream_is_empty(request->extra_fields)) {
 		str_append_c(str, '\t');
-		auth_stream_reply_append(request->extra_fields, str);
-	}
-	if (!auth_stream_is_empty(request->extra_cache_fields)) {
-		str_append_c(str, '\t');
-		auth_stream_reply_append(request->extra_cache_fields, str);
+		auth_stream_reply_append(request->extra_fields, str, TRUE);
 	}
 	auth_cache_insert(passdb_cache, request, passdb->cache_key, str_c(str),
 			  result == PASSDB_RESULT_OK);
@@ -516,7 +512,7 @@
 				auth_stream_reply_init(request->pool);
 		}
 	        auth_stream_reply_add(request->extra_fields, "reason",
-				      "Password expired");
+				      "Password expired", 0);
 	} else if (request->passdb->next != NULL &&
 		   *result != PASSDB_RESULT_USER_DISABLED) {
 		/* try next passdb. */
@@ -826,16 +822,23 @@
 					   enum userdb_result result)
 {
 	struct userdb_module *userdb = request->userdb->userdb;
-	const char *str;
+	string_t *str;
+	const char *cache_value;
 
 	if (passdb_cache == NULL || userdb->cache_key == NULL ||
 	    request->master_user != NULL)
 		return;
 
-	str = result == USERDB_RESULT_USER_UNKNOWN ? "" :
-		auth_stream_reply_export(request->userdb_reply);
+	if (result == USERDB_RESULT_USER_UNKNOWN)
+		cache_value = "";
+	else {
+		str = t_str_new(128);
+		auth_stream_reply_append(request->userdb_reply, str, TRUE);
+		cache_value = str_c(str);
+	}
 	/* last_success has no meaning with userdb */
-	auth_cache_insert(passdb_cache, request, userdb->cache_key, str, FALSE);
+	auth_cache_insert(passdb_cache, request, userdb->cache_key,
+			  cache_value, FALSE);
 }
 
 static bool auth_request_lookup_user_cache(struct auth_request *request,
@@ -869,7 +872,7 @@
 
 	*result_r = USERDB_RESULT_OK;
 	*reply_r = auth_stream_reply_init(request->pool);
-	auth_stream_reply_import(*reply_r, value);
+	auth_stream_reply_import(*reply_r, value, 0);
 	return TRUE;
 }
 
@@ -1190,8 +1193,7 @@
 
 	if (request->extra_fields == NULL)
 		request->extra_fields = auth_stream_reply_init(request->pool);
-	auth_stream_reply_remove(request->extra_fields, name);
-	auth_stream_reply_add(request->extra_fields, name, value);
+	auth_stream_reply_add(request->extra_fields, name, value, 0);
 }
 
 static const char *
@@ -1305,11 +1307,8 @@
 		/* we'll need to get this field stored into cache,
 		   or we're a worker and we'll need to send this to the main
 		   auth process that can store it in the cache. */
-		if (request->extra_cache_fields == NULL) {
-			request->extra_cache_fields =
-				auth_stream_reply_init(request->pool);
-		}
-		auth_stream_reply_add(request->extra_cache_fields, name, value);
+		auth_stream_reply_add(request->extra_fields, name, value,
+				      AUTH_STREAM_FIELD_FLAG_HIDDEN);
 	}
 }
 
@@ -1363,9 +1362,9 @@
 				       "stat(%s) failed: %m", str_c(path));
 	} else {
 		auth_stream_reply_add(request->userdb_reply,
-				      "uid", dec2str(st.st_uid));
+				      "uid", dec2str(st.st_uid), 0);
 		auth_stream_reply_add(request->userdb_reply,
-				      "gid", dec2str(st.st_gid));
+				      "gid", dec2str(st.st_gid), 0);
 	}
 }
 
@@ -1398,7 +1397,7 @@
 		auth_request_set_uidgid_file(request, value);
 		return;
 	} else if (strcmp(name, "userdb_import") == 0) {
-		auth_stream_reply_import(request->userdb_reply, value);
+		auth_stream_reply_import(request->userdb_reply, value, 0);
 		return;
 	} else if (strcmp(name, "system_user") == 0) {
 		/* FIXME: the system_user is for backwards compatibility */
@@ -1410,8 +1409,7 @@
 		name = "system_groups_user";
 	}
 
-	auth_stream_reply_remove(request->userdb_reply, name);
-	auth_stream_reply_add(request->userdb_reply, name, value);
+	auth_stream_reply_add(request->userdb_reply, name, value, 0);
 }
 
 void auth_request_set_userdb_field_values(struct auth_request *request,
@@ -1439,7 +1437,7 @@
 			str_append(value, dec2str(gid));
 		}
 		auth_stream_reply_add(request->userdb_reply, name,
-				      str_c(value));
+				      str_c(value), 0);
 	} else {
 		/* add only one */
 		if (values[1] != NULL) {
@@ -1496,7 +1494,7 @@
 	} else if (!auth_request_proxy_is_self(request)) {
 		/* proxy destination isn't ourself - proxy */
 		auth_stream_reply_remove(request->extra_fields, "proxy_maybe");
-		auth_stream_reply_add(request->extra_fields, "proxy", NULL);
+		auth_stream_reply_add(request->extra_fields, "proxy", NULL, 0);
 		request->no_login = TRUE;
 	} else {
 		/* proxying to ourself - log in without proxying by dropping
@@ -1507,7 +1505,7 @@
 		if (proxy_always) {
 			/* director adds the host */
 			auth_stream_reply_add(request->extra_fields,
-					      "proxy", NULL);
+					      "proxy", NULL, 0);
 			request->proxy = TRUE;
 		}
 	}
@@ -1538,9 +1536,8 @@
 				"DNS lookup for %s took %u.%03u s",
 				host, result->msecs/1000, result->msecs % 1000);
 		}
-		auth_stream_reply_remove(request->extra_fields, "hostip");
 		auth_stream_reply_add(request->extra_fields, "hostip",
-				      net_ip2addr(&result->ips[0]));
+				      net_ip2addr(&result->ips[0]), 0);
 		for (i = 0; i < result->ips_count; i++) {
 			if (auth_request_proxy_ip_is_self(request,
 							  &result->ips[i])) {
diff -r f11ae64365b5 -r cc4472f02f70 src/auth/auth-request.h
--- a/src/auth/auth-request.h	Wed Jan 30 19:11:30 2013 +0200
+++ b/src/auth/auth-request.h	Wed Jan 30 19:46:58 2013 +0200
@@ -50,9 +50,6 @@
         /* extra_fields are returned in authentication reply. Fields prefixed
            with "userdb_" are automatically placed to userdb_reply instead. */
         struct auth_stream_reply *extra_fields;
-	/* extra_fields that aren't supposed to be sent to the client, but
-	   are supposed to be stored to auth cache. */
-	struct auth_stream_reply *extra_cache_fields;
 	/* the whole userdb result reply */
 	struct auth_stream_reply *userdb_reply;
 	struct auth_request_proxy_dns_lookup_ctx *dns_lookup_ctx;
diff -r f11ae64365b5 -r cc4472f02f70 src/auth/auth-stream.c
--- a/src/auth/auth-stream.c	Wed Jan 30 19:11:30 2013 +0200
+++ b/src/auth/auth-stream.c	Wed Jan 30 19:46:58 2013 +0200
@@ -1,6 +1,7 @@
 /* Copyright (c) 2005-2012 Dovecot authors, see the included COPYING file */
 
 #include "auth-common.h"
+#include "array.h"
 #include "str.h"
 #include "strescape.h"
 #include "ostream.h"
@@ -8,7 +9,8 @@
 #include "auth-stream.h"
 
 struct auth_stream_reply {
-	string_t *str;
+	pool_t pool;
+	ARRAY_TYPE(auth_stream_field) fields;
 };
 
 struct auth_stream_reply *auth_stream_reply_init(pool_t pool)
@@ -16,128 +18,132 @@
 	struct auth_stream_reply *reply;
 
 	reply = p_new(pool, struct auth_stream_reply, 1);
-	reply->str = str_new(pool, 128);
+	reply->pool = pool;
+	p_array_init(&reply->fields, pool, 16);
 	return reply;
 }
 
+static bool
+auth_stream_reply_find_idx(struct auth_stream_reply *reply, const char *key,
+			   unsigned int *idx_r)
+{
+	const struct auth_stream_field *fields;
+	unsigned int i, count;
+
+	fields = array_get(&reply->fields, &count);
+	for (i = 0; i < count; i++) {
+		if (strcmp(fields[i].key, key) == 0) {
+			*idx_r = i;
+			return TRUE;
+		}
+	}
+	return FALSE;
+}
+
 void auth_stream_reply_add(struct auth_stream_reply *reply,
-			   const char *key, const char *value)
+			   const char *key, const char *value,
+			   enum auth_stream_field_flags flags)
 {
+	struct auth_stream_field *field;
+	unsigned int idx;
+
 	i_assert(*key != '\0');
 	i_assert(strchr(key, '\t') == NULL &&
 		 strchr(key, '\n') == NULL);
 
-	if (str_len(reply->str) > 0)
-		str_append_c(reply->str, '\t');
-
-	str_append(reply->str, key);
-	if (value != NULL) {
-		str_append_c(reply->str, '=');
-		/* escape dangerous characters in the value */
-		str_append_tabescaped(reply->str, value);
+	if (!auth_stream_reply_find_idx(reply, key, &idx)) {
+		field = array_append_space(&reply->fields);
+		field->key = p_strdup(reply->pool, key);
+	} else {
+		field = array_idx_modifiable(&reply->fields, idx);
 	}
-}
-
-static bool
-auth_stream_reply_find_area(struct auth_stream_reply *reply, const char *key,
-			    unsigned int *idx_r, unsigned int *len_r)


More information about the dovecot-cvs mailing list