From 398789b931dd95096b60e279a7117553fac0bfa1 Mon Sep 17 00:00:00 2001 From: Thuan Duong Date: Mon, 15 Jan 2018 17:09:12 +0700 Subject: [PATCH] Fix Bug: redis_auth don't work with redis_db https://github.com/twitter/twemproxy/pull/458 --- src/nc_message.c | 3 --- src/nc_message.h | 2 -- src/nc_request.c | 9 ------- src/proto/nc_memcache.c | 7 ----- src/proto/nc_proto.h | 2 -- src/proto/nc_redis.c | 59 ++++++++++++++++++++++++++++++----------- 6 files changed, 43 insertions(+), 39 deletions(-) diff --git a/src/nc_message.c b/src/nc_message.c index abfe8bc5..de3b6326 100644 --- a/src/nc_message.c +++ b/src/nc_message.c @@ -226,7 +226,6 @@ _msg_get(void) msg->token = NULL; msg->parser = NULL; - msg->add_auth = NULL; msg->result = MSG_PARSE_OK; msg->fragment = NULL; @@ -293,7 +292,6 @@ msg_get(struct conn *conn, bool request, bool redis) } else { msg->parser = redis_parse_rsp; } - msg->add_auth = redis_add_auth; msg->fragment = redis_fragment; msg->reply = redis_reply; msg->failure = redis_failure; @@ -305,7 +303,6 @@ msg_get(struct conn *conn, bool request, bool redis) } else { msg->parser = memcache_parse_rsp; } - msg->add_auth = memcache_add_auth; msg->fragment = memcache_fragment; msg->failure = memcache_failure; msg->pre_coalesce = memcache_pre_coalesce; diff --git a/src/nc_message.h b/src/nc_message.h index 2844c824..65032d6d 100644 --- a/src/nc_message.h +++ b/src/nc_message.h @@ -21,7 +21,6 @@ #include typedef void (*msg_parse_t)(struct msg *); -typedef rstatus_t (*msg_add_auth_t)(struct context *ctx, struct conn *c_conn, struct conn *s_conn); typedef rstatus_t (*msg_fragment_t)(struct msg *, uint32_t, struct msg_tqh *); typedef void (*msg_coalesce_t)(struct msg *r); typedef rstatus_t (*msg_reply_t)(struct msg *r); @@ -223,7 +222,6 @@ struct msg { msg_fragment_t fragment; /* message fragment */ msg_reply_t reply; /* generate message reply (example: ping) */ - msg_add_auth_t add_auth; /* add auth message when we forward msg */ msg_failure_t failure; /* transient failure response? */ msg_coalesce_t pre_coalesce; /* message pre-coalesce */ diff --git a/src/nc_request.c b/src/nc_request.c index 5fc4190a..341aaea3 100644 --- a/src/nc_request.c +++ b/src/nc_request.c @@ -593,15 +593,6 @@ req_forward(struct context *ctx, struct conn *c_conn, struct msg *msg) } } - if (!conn_authenticated(s_conn)) { - status = msg->add_auth(ctx, c_conn, s_conn); - if (status != NC_OK) { - req_forward_error(ctx, c_conn, msg); - s_conn->err = errno; - return; - } - } - s_conn->enqueue_inq(ctx, s_conn, msg); req_forward_stats(ctx, s_conn->owner, msg); diff --git a/src/proto/nc_memcache.c b/src/proto/nc_memcache.c index be3c5f54..7dbcd9ad 100644 --- a/src/proto/nc_memcache.c +++ b/src/proto/nc_memcache.c @@ -1545,13 +1545,6 @@ memcache_swallow_msg(struct conn *conn, struct msg *pmsg, struct msg *msg) { } -rstatus_t -memcache_add_auth(struct context *ctx, struct conn *c_conn, struct conn *s_conn) -{ - NOT_REACHED(); - return NC_OK; -} - rstatus_t memcache_reply(struct msg *r) { diff --git a/src/proto/nc_proto.h b/src/proto/nc_proto.h index 62370290..b6426dc6 100644 --- a/src/proto/nc_proto.h +++ b/src/proto/nc_proto.h @@ -143,7 +143,6 @@ void memcache_parse_rsp(struct msg *r); bool memcache_failure(struct msg *r); void memcache_pre_coalesce(struct msg *r); void memcache_post_coalesce(struct msg *r); -rstatus_t memcache_add_auth(struct context *ctx, struct conn *c_conn, struct conn *s_conn); rstatus_t memcache_fragment(struct msg *r, uint32_t ncontinuum, struct msg_tqh *frag_msgq); rstatus_t memcache_reply(struct msg *r); void memcache_post_connect(struct context *ctx, struct conn *conn, struct server *server); @@ -154,7 +153,6 @@ void redis_parse_rsp(struct msg *r); bool redis_failure(struct msg *r); void redis_pre_coalesce(struct msg *r); void redis_post_coalesce(struct msg *r); -rstatus_t redis_add_auth(struct context *ctx, struct conn *c_conn, struct conn *s_conn); rstatus_t redis_fragment(struct msg *r, uint32_t ncontinuum, struct msg_tqh *frag_msgq); rstatus_t redis_reply(struct msg *r); void redis_post_connect(struct context *ctx, struct conn *conn, struct server *server); diff --git a/src/proto/nc_redis.c b/src/proto/nc_redis.c index 2db88882..f24adb42 100644 --- a/src/proto/nc_redis.c +++ b/src/proto/nc_redis.c @@ -2839,20 +2839,23 @@ redis_handle_auth_req(struct msg *req, struct msg *rsp) } rstatus_t -redis_add_auth(struct context *ctx, struct conn *c_conn, struct conn *s_conn) +redis_add_auth(struct context *ctx, struct conn *conn, struct server *server, int *sended) { rstatus_t status; struct msg *msg; struct server_pool *pool; - ASSERT(!s_conn->client && !s_conn->proxy); - ASSERT(!conn_authenticated(s_conn)); + ASSERT(!conn->client && !conn->proxy && conn->connected && conn->redis); + ASSERT(sended); - pool = c_conn->owner; + pool = server->owner; + if (!pool->require_auth) { + return NC_OK; + } - msg = msg_get(c_conn, true, c_conn->redis); + msg = msg_get(conn, true, conn->redis); if (msg == NULL) { - c_conn->err = errno; + conn->err = errno; return NC_ENOMEM; } @@ -2863,23 +2866,33 @@ redis_add_auth(struct context *ctx, struct conn *c_conn, struct conn *s_conn) return status; } + msg->type = MSG_REQ_REDIS_AUTH; + msg->result = MSG_PARSE_OK; msg->swallow = 1; - s_conn->enqueue_inq(ctx, s_conn, msg); - s_conn->authenticated = 1; + msg->owner = NULL; + + conn->authenticated = 1; + + /* enqueue as head and send */ + req_server_enqueue_imsgq_head(ctx, conn, msg); + *sended = 1; + + log_debug(LOG_NOTICE, "sent 'AUTH %s' to %s | %s", pool->redis_auth.data, + pool->name.data, server->name.data); return NC_OK; } -void -redis_post_connect(struct context *ctx, struct conn *conn, struct server *server) +rstatus_t +redis_select_db(struct context *ctx, struct conn *conn, struct server *server, int *sended) { rstatus_t status; struct server_pool *pool = server->owner; struct msg *msg; int digits; - ASSERT(!conn->client && conn->connected); - ASSERT(conn->redis); + ASSERT(!conn->client && conn->connected && conn->redis); + ASSERT(sended); /* * By default, every connection to redis uses the database DB 0. You @@ -2888,7 +2901,7 @@ redis_post_connect(struct context *ctx, struct conn *conn, struct server *server * on a per pool basis in the configuration */ if (pool->redis_db <= 0) { - return; + return NC_OK; } /* @@ -2898,14 +2911,15 @@ redis_post_connect(struct context *ctx, struct conn *conn, struct server *server */ msg = msg_get(conn, true, conn->redis); if (msg == NULL) { - return; + conn->err = errno; + return NC_ENOMEM; } digits = (pool->redis_db >= 10) ? (int)log10(pool->redis_db) + 1 : 1; status = msg_prepend_format(msg, "*2\r\n$6\r\nSELECT\r\n$%d\r\n%d\r\n", digits, pool->redis_db); if (status != NC_OK) { msg_put(msg); - return; + return status; } msg->type = MSG_REQ_REDIS_SELECT; msg->result = MSG_PARSE_OK; @@ -2914,10 +2928,23 @@ redis_post_connect(struct context *ctx, struct conn *conn, struct server *server /* enqueue as head and send */ req_server_enqueue_imsgq_head(ctx, conn, msg); - msg_send(ctx, conn); + *sended = 1; log_debug(LOG_NOTICE, "sent 'SELECT %d' to %s | %s", pool->redis_db, pool->name.data, server->name.data); + + return NC_OK; +} + +void +redis_post_connect(struct context *ctx, struct conn *conn, struct server *server) +{ + int sended = 0; + redis_select_db(ctx, conn, server, &sended); + redis_add_auth(ctx, conn, server, &sended); + if (sended) { + msg_send(ctx, conn); + } } void