changeset 15684:cc4472f02f70

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.
author Timo Sirainen <tss@iki.fi>
date Wed, 30 Jan 2013 19:46:58 +0200
parents f11ae64365b5
children 17f5257d60c1
files src/auth/auth-master-connection.c src/auth/auth-request-handler.c src/auth/auth-request.c src/auth/auth-request.h src/auth/auth-stream.c src/auth/auth-stream.h src/auth/auth-worker-client.c src/auth/db-checkpassword.c src/auth/userdb-blocking.c
diffstat 9 files changed, 153 insertions(+), 146 deletions(-) [+]
line wrap: on
line diff
--- 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:
--- 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,
--- 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])) {
--- 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;
--- 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)
-{
-	const char *str = str_c(reply->str);
-	unsigned int i, start, key_len = strlen(key);
-
-	i = 0;
-	while (str[i] != '\0') {
-		start = i;
-		for (; str[i] != '\0'; i++) {
-			if (str[i] == '\t')
-				break;
-		}
-
-		if (strncmp(str+start, key, key_len) == 0 &&
-		    (str[start+key_len] == '=' ||
-		     str[start+key_len] == '\t' ||
-		     str[start+key_len] == '\0')) {
-			*idx_r = start;
-			*len_r = i - start;
-			return TRUE;
-		}
-		if (str[i] == '\t')
-			i++;
-	}
-	return FALSE;
+	field->value = p_strdup_empty(reply->pool, value);
+	field->flags = flags;
 }
 
 void auth_stream_reply_remove(struct auth_stream_reply *reply, const char *key)
 {
-	unsigned int idx, len;
-
-	if (!auth_stream_reply_find_area(reply, key, &idx, &len))
-		return;
+	unsigned int idx;
 
-	if (idx == 0 && str_len(reply->str) == len) {
-		/* removing the only item */
-	} else if (str_len(reply->str) == idx + len) {
-		/* removing the last item -> remove the preceding tab */
-		len++;
-		idx--;
-	} else {
-		/* remove the trailing tab */
-		len++;
-	}
-	str_delete(reply->str, idx, len);
+	if (auth_stream_reply_find_idx(reply, key, &idx))
+		array_delete(&reply->fields, idx, 1);
 }
 
 const char *auth_stream_reply_find(struct auth_stream_reply *reply,
 				   const char *key)
 {
-	unsigned int idx, len, keylen;
+	const struct auth_stream_field *field;
+	unsigned int idx;
 
-	if (!auth_stream_reply_find_area(reply, key, &idx, &len))
+	if (!auth_stream_reply_find_idx(reply, key, &idx))
 		return NULL;
-	else {
-		keylen = strlen(key);
-		if (len == keylen) {
-			/* key without =value */
-			return "";
-		}
-		i_assert(len > keylen);
-		idx += keylen + 1;
-		len -= keylen + 1;
-		return t_strndup(str_c(reply->str) + idx, len);
-	}
+
+	field = array_idx(&reply->fields, idx);
+	return field->value == NULL ? "" : field->value;
 }
 
 bool auth_stream_reply_exists(struct auth_stream_reply *reply, const char *key)
 {
-	unsigned int idx, len;
-
-	return auth_stream_reply_find_area(reply, key, &idx, &len);
+	return auth_stream_reply_find(reply, key) != NULL;
 }
 
 void auth_stream_reply_reset(struct auth_stream_reply *reply)
 {
-	str_truncate(reply->str, 0);
+	array_clear(&reply->fields);
 }
 
-void auth_stream_reply_import(struct auth_stream_reply *reply, const char *str)
+void auth_stream_reply_import(struct auth_stream_reply *reply, const char *str,
+			      enum auth_stream_field_flags flags)
 {
-	if (str_len(reply->str) > 0)
-		str_append_c(reply->str, '\t');
-	str_append(reply->str, str);
+	T_BEGIN {
+		const char *const *arg = t_strsplit_tab(str);
+		const char *key, *value;
+
+		for (; *arg != NULL; arg++) {
+			value = strchr(*arg, '=');
+			if (value == NULL) {
+				key = *arg;
+				value = NULL;
+			} else {
+				key = t_strdup_until(*arg, value++);
+			}
+			auth_stream_reply_add(reply, key, value, flags);
+		}
+	} T_END;
 }
 
-const char *auth_stream_reply_export(struct auth_stream_reply *reply)
+const ARRAY_TYPE(auth_stream_field) *
+auth_stream_reply_export(struct auth_stream_reply *reply)
 {
-	return str_c(reply->str);
+	return &reply->fields;
 }
 
-void auth_stream_reply_append(struct auth_stream_reply *reply,
-			      string_t *dest)
+void auth_stream_reply_append(struct auth_stream_reply *reply, string_t *dest,
+			      bool include_hidden)
 {
-	str_append_str(dest, reply->str);
+	const struct auth_stream_field *fields;
+	unsigned int i, count;
+	bool first = TRUE;
+
+	fields = array_get(&reply->fields, &count);
+	for (i = 0; i < count; i++) {
+		if (!include_hidden &&
+		    (fields[i].flags & AUTH_STREAM_FIELD_FLAG_HIDDEN) != 0)
+			continue;
+
+		if (first)
+			first = FALSE;
+		else
+			str_append_c(dest, '\t');
+		str_append(dest, fields[i].key);
+		if (fields[i].value != NULL) {
+			str_append_c(dest, '=');
+			str_append_tabescaped(dest, fields[i].value);
+		}
+	}
 }
 
 bool auth_stream_is_empty(struct auth_stream_reply *reply)
 {
-	return reply == NULL || str_len(reply->str) == 0;
+	return reply == NULL || array_count(&reply->fields) == 0;
 }
--- a/src/auth/auth-stream.h	Wed Jan 30 19:11:30 2013 +0200
+++ b/src/auth/auth-stream.h	Wed Jan 30 19:46:58 2013 +0200
@@ -3,9 +3,21 @@
 
 struct auth_request;
 
+enum auth_stream_field_flags {
+	/* This field is internal to auth process and won't be sent to client */
+	AUTH_STREAM_FIELD_FLAG_HIDDEN	= 0x01
+};
+
+struct auth_stream_field {
+	const char *key, *value;
+	enum auth_stream_field_flags flags;
+};
+ARRAY_DEFINE_TYPE(auth_stream_field, struct auth_stream_field);
+
 struct auth_stream_reply *auth_stream_reply_init(pool_t pool);
 void auth_stream_reply_add(struct auth_stream_reply *reply,
-			   const char *key, const char *value) ATTR_NULL(3);
+			   const char *key, const char *value,
+			   enum auth_stream_field_flags flags) ATTR_NULL(3);
 void auth_stream_reply_reset(struct auth_stream_reply *reply);
 void auth_stream_reply_remove(struct auth_stream_reply *reply, const char *key);
 
@@ -13,10 +25,12 @@
 				   const char *key);
 bool auth_stream_reply_exists(struct auth_stream_reply *reply, const char *key);
 
-void auth_stream_reply_import(struct auth_stream_reply *reply, const char *str);
-const char *auth_stream_reply_export(struct auth_stream_reply *reply);
-void auth_stream_reply_append(struct auth_stream_reply *reply,
-			      string_t *dest);
+void auth_stream_reply_import(struct auth_stream_reply *reply, const char *str,
+			      enum auth_stream_field_flags flags);
+const ARRAY_TYPE(auth_stream_field) *
+auth_stream_reply_export(struct auth_stream_reply *reply);
+void auth_stream_reply_append(struct auth_stream_reply *reply, string_t *dest,
+			      bool include_hidden);
 bool auth_stream_is_empty(struct auth_stream_reply *reply);
 
 #endif
--- a/src/auth/auth-worker-client.c	Wed Jan 30 19:11:30 2013 +0200
+++ b/src/auth/auth-worker-client.c	Wed Jan 30 19:46:58 2013 +0200
@@ -131,11 +131,7 @@
 			str_append_tabescaped(str, request->passdb_password);
 		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);
 		}
 	}
 	str_append_c(str, '\n');
@@ -220,11 +216,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);
 		}
 	}
 	str_append_c(str, '\n');
@@ -351,7 +343,7 @@
 		break;
 	case USERDB_RESULT_OK:
 		str_append(str, "OK\t");
-		auth_stream_reply_append(auth_request->userdb_reply, str);
+		auth_stream_reply_append(auth_request->userdb_reply, str, TRUE);
 		if (auth_request->userdb_lookup_failed)
 			str_append(str, "\ttempfail");
 		break;
--- a/src/auth/db-checkpassword.c	Wed Jan 30 19:11:30 2013 +0200
+++ b/src/auth/db-checkpassword.c	Wed Jan 30 19:46:58 2013 +0200
@@ -5,6 +5,7 @@
 #if defined(PASSDB_CHECKPASSWORD) || defined(USERDB_CHECKPASSWORD)
 
 #include "lib-signals.h"
+#include "array.h"
 #include "buffer.h"
 #include "str.h"
 #include "ioloop.h"
@@ -49,18 +50,16 @@
 	struct child_wait *child_wait;
 };
 
-static void env_put_extra_fields(const char *extra_fields)
+static void
+env_put_extra_fields(const ARRAY_TYPE(auth_stream_field) *extra_fields)
 {
-	const char *const *tmp;
-	const char *key, *p;
+	const struct auth_stream_field *field;
+	const char *key, *value;
 
-	for (tmp = t_strsplit_tab(extra_fields); *tmp != NULL; tmp++) {
-		key = t_str_ucase(t_strcut(*tmp, '='));
-		p = strchr(*tmp, '=');
-		if (p == NULL)
-			env_put(t_strconcat(key, "=1", NULL));
-		else
-			env_put(t_strconcat(key, p, NULL));
+	array_foreach(extra_fields, field) {
+		key = t_str_ucase(field->key);
+		value = field->value != NULL ? field->value : "1";
+		env_put(t_strconcat(key, "=", value, NULL));
 	}
 }
 
@@ -271,7 +270,7 @@
 				    request->master_user, NULL));
 	}
 	if (request->extra_fields != NULL) {
-		const char *fields =
+		const ARRAY_TYPE(auth_stream_field) *fields =
 			auth_stream_reply_export(request->extra_fields);
 
 		/* extra fields could come from master db */
--- a/src/auth/userdb-blocking.c	Wed Jan 30 19:11:30 2013 +0200
+++ b/src/auth/userdb-blocking.c	Wed Jan 30 19:46:58 2013 +0200
@@ -38,7 +38,7 @@
 
 	if (*args != '\0') {
 		request->userdb_reply = auth_stream_reply_init(request->pool);
-		auth_stream_reply_import(request->userdb_reply, args);
+		auth_stream_reply_import(request->userdb_reply, args, 0);
 		if (auth_stream_reply_exists(request->userdb_reply, "tempfail"))
 			request->userdb_lookup_failed = TRUE;
 	}