Skip to content

Commit

Permalink
Avoid responding to the wrong fd after client disconnection
Browse files Browse the repository at this point in the history
Slightly adapted from a proposed change by @majklik on GitHub in
issue #212 (one invalid read fixed and a memory leak avoided).
This marks an inflight cmd's fd as -1 when the HTTP client disconnects,
which prevents the later response from Redis from being sent to a new
client which has connected in the meantime and been assigned the same
client fd.
  • Loading branch information
nicolasff committed Jan 8, 2022
1 parent 545c56c commit d28dd3e
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 2 deletions.
8 changes: 7 additions & 1 deletion src/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,13 @@ http_client_read(struct http_client *c) {
c->reused_cmd = NULL;

/* delete command object */
cmd_free(cmd);
cmd_free(cmd); /* this will also set c->last_cmd to null so we won't enter the `if` below */
}

if(c->last_cmd) { /* avoid fd reuse */
c->last_cmd->fd = -1;
c->last_cmd->http_client = NULL;
c->last_cmd = NULL;
}

close(c->fd);
Expand Down
1 change: 1 addition & 0 deletions src/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ struct http_client {
char *filename; /* content-disposition */

struct cmd *reused_cmd;
struct cmd *last_cmd; /* last command executed, might be in flight */

struct ws_client *ws; /* websocket client */
};
Expand Down
11 changes: 11 additions & 0 deletions src/cmd.c
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ cmd_new(struct http_client *client, int count) {
c->count = count;
c->http_client = client;

/* attach to client so that the cmd's fd can be unset
when the client is freed and its fd is closed */
if(client) {
client->last_cmd = c;
}

c->argv = calloc(count, sizeof(char*));
c->argv_len = calloc(count, sizeof(size_t));

Expand Down Expand Up @@ -56,6 +62,11 @@ cmd_free(struct cmd *c) {
free(c->if_none_match);
if(c->mime_free) free(c->mime);

/* detach last_cmd from http_client since the cmd is being freed */
if(c->http_client && c->http_client->last_cmd == c) {
c->http_client->last_cmd = NULL;
}

if (c->ac && /* we have a connection */
(c->database != c->w->s->cfg->database /* custom DB */
|| cmd_is_subscribe(c))) {
Expand Down
1 change: 1 addition & 0 deletions src/conf.c
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <unistd.h>
#include <pwd.h>
#include <grp.h>
#include <errno.h>

#include <jansson.h>
#include <evhttp.h>
Expand Down
7 changes: 6 additions & 1 deletion src/http.c
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ http_response_cleanup(struct http_response *r, int fd, int success) {

/* cleanup buffer */
free(r->out);
if(!r->keep_alive || !success) {
if((!r->keep_alive || !success) && fd > 0) {
/* Close fd is client doesn't support Keep-Alive. */
close(fd);
}
Expand Down Expand Up @@ -234,6 +234,11 @@ http_response_write(struct http_response *r, int fd) {
char *p;
int i, ret;

if(fd < 0) { /* http_client was freed, which set the inflight cmd's fd to -1 */
http_response_cleanup(r, fd, 0); /* we would have done this after the write */
return;
}

r->out_sz = sizeof("HTTP/1.x xxx ")-1 + strlen(r->msg) + 2;
r->out = calloc(r->out_sz + 1, 1);

Expand Down

0 comments on commit d28dd3e

Please sign in to comment.