zeekling
/
hiredis
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Auto merge of #379 - thomaslee:tom_test_race, r=badboy 

Fix potential race in 'invalid timeout' tests

It's possible for the call to connect() to succeed on the very first try, in which case the logic for checking for invalid timeout fields is never executed. When this happens, the tests fail because they expect a REDIS_ERR_IO but no such failure has occurred.

Tests aside, this is a potential source of irritating and hard-to-find intermittent bugs.

This patch forces the validation to occur early so that we get predictable behavior whenever an invalid timeout is specified.
This commit is contained in:
not-a-robot 2016-04-20 17:42:04 +02:00
parent d3c7df33a1 d4b715f0aa
commit a91afd746b
2 changed files with 30 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

39
net.c
View File

@ -178,19 +178,15 @@ static int redisSetTcpNoDelay(redisContext *c) {
#define __MAX_MSEC (((LONG_MAX) - 999) / 1000)
static int redisContextWaitReady(redisContext *c, const struct timeval *timeout) {
struct pollfd wfd[1];
long msec;
msec = -1;
wfd[0].fd = c->fd;
wfd[0].events = POLLOUT;
static int redisContextTimeoutMsec(redisContext *c, long *result)
{
const struct timeval *timeout = c->timeout;
long msec = -1;
/* Only use timeout when not NULL. */
if (timeout != NULL) {
if (timeout->tv_usec > 1000000 || timeout->tv_sec > __MAX_MSEC) {
__redisSetErrorFromErrno(c, REDIS_ERR_IO, NULL);
redisContextCloseFd(c);
*result = msec;
return REDIS_ERR;
}
@ -201,6 +197,16 @@ static int redisContextWaitReady(redisContext *c, const struct timeval *timeout)
}
}
*result = msec;
return REDIS_OK;
}
static int redisContextWaitReady(redisContext *c, long msec) {
struct pollfd wfd[1];
wfd[0].fd = c->fd;
wfd[0].events = POLLOUT;
if (errno == EINPROGRESS) {
int res;
@ -265,7 +271,9 @@ static int _redisContextConnectTcp(redisContext *c, const char *addr, int port,
int blocking = (c->flags & REDIS_BLOCK);
int reuseaddr = (c->flags & REDIS_REUSEADDR);
int reuses = 0;
long timeout_msec = -1;
servinfo = NULL;
c->connection_type = REDIS_CONN_TCP;
c->tcp.port = port;
@ -296,6 +304,11 @@ static int _redisContextConnectTcp(redisContext *c, const char *addr, int port,
c->timeout = NULL;
}
if (redisContextTimeoutMsec(c, &timeout_msec) != REDIS_OK) {
__redisSetError(c, REDIS_ERR_IO, "Invalid timeout specified");
goto error;
}
if (source_addr == NULL) {
free(c->tcp.source_addr);
c->tcp.source_addr = NULL;
@ -374,7 +387,7 @@ addrretry:
goto addrretry;
}
} else {
if (redisContextWaitReady(c,c->timeout) != REDIS_OK)
if (redisContextWaitReady(c,timeout_msec) != REDIS_OK)
goto error;
}
}
@ -415,6 +428,7 @@ int redisContextConnectBindTcp(redisContext *c, const char *addr, int port,
int redisContextConnectUnix(redisContext *c, const char *path, const struct timeval *timeout) {
int blocking = (c->flags & REDIS_BLOCK);
struct sockaddr_un sa;
long timeout_msec = -1;
if (redisCreateSocket(c,AF_LOCAL) < 0)
return REDIS_ERR;
@ -438,13 +452,16 @@ int redisContextConnectUnix(redisContext *c, const char *path, const struct time
c->timeout = NULL;
}
if (redisContextTimeoutMsec(c,&timeout_msec) != REDIS_OK)
return REDIS_ERR;
sa.sun_family = AF_LOCAL;
strncpy(sa.sun_path,path,sizeof(sa.sun_path)-1);
if (connect(c->fd, (struct sockaddr*)&sa, sizeof(sa)) == -1) {
if (errno == EINPROGRESS && !blocking) {
/* This is ok. */
} else {
if (redisContextWaitReady(c,c->timeout) != REDIS_OK)
if (redisContextWaitReady(c,timeout_msec) != REDIS_OK)
return REDIS_ERR;
}
}

4
test.c
View File

@ -552,7 +552,7 @@ static void test_invalid_timeout_errors(struct config config) {
c = redisConnectWithTimeout(config.tcp.host, config.tcp.port, config.tcp.timeout);
test_cond(c->err == REDIS_ERR_IO);
test_cond(c->err == REDIS_ERR_IO && strcmp(c->errstr, "Invalid timeout specified") == 0);
redisFree(c);
test("Set error when an invalid timeout sec value is given to redisConnectWithTimeout: ");
@ -562,7 +562,7 @@ static void test_invalid_timeout_errors(struct config config) {
c = redisConnectWithTimeout(config.tcp.host, config.tcp.port, config.tcp.timeout);
test_cond(c->err == REDIS_ERR_IO);
test_cond(c->err == REDIS_ERR_IO && strcmp(c->errstr, "Invalid timeout specified") == 0);
redisFree(c);
}