From d28dd3ec802849a6e22a06ef858c3d1771cbeb5b Mon Sep 17 00:00:00 2001
From: Nicolas Favre-Felix <n.favrefelix@gmail.com>
Date: Sat, 8 Jan 2022 13:48:02 -0800
Subject: [PATCH] Avoid responding to the wrong fd after client disconnection

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.
---
 src/client.c |  8 +++++++-
 src/client.h |  1 +
 src/cmd.c    | 11 +++++++++++
 src/conf.c   |  1 +
 src/http.c   |  7 ++++++-
 5 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/src/client.c b/src/client.c
index ed6f3f3..353b360 100644
--- a/src/client.c
+++ b/src/client.c
@@ -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);
diff --git a/src/client.h b/src/client.h
index c355bac..30468e6 100644
--- a/src/client.h
+++ b/src/client.h
@@ -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 */
 };
diff --git a/src/cmd.c b/src/cmd.c
index 6fcc528..937e6ca 100644
--- a/src/cmd.c
+++ b/src/cmd.c
@@ -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));
 
@@ -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))) {
diff --git a/src/conf.c b/src/conf.c
index 8a1cc76..904356e 100644
--- a/src/conf.c
+++ b/src/conf.c
@@ -6,6 +6,7 @@
 #include <unistd.h>
 #include <pwd.h>
 #include <grp.h>
+#include <errno.h>
 
 #include <jansson.h>
 #include <evhttp.h>
diff --git a/src/http.c b/src/http.c
index d750d6a..3e58aff 100644
--- a/src/http.c
+++ b/src/http.c
@@ -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);
 	}
@@ -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);