Index | Thread | Search

From:
Rafael Sadowski <rafael@sizeofvoid.org>
Subject:
relayd: move HTTP start-line logic to a dedicated function
To:
tech@openbsd.org
Date:
Sun, 1 Mar 2026 15:53:13 +0100

Download raw body.

Thread
  • Rafael Sadowski:

    relayd: move HTTP start-line logic to a dedicated function

Begin refactoring the oversized relay_read_http() aka 500+ lines  by
moving HTTP start-line logic into a separate function. This is the first
of several planned extractions to reduce the complexity of the main read
loop.

This is a structural refactoring with no functional change.

Feedback? OK?

diff --git a/usr.sbin/relayd/relay_http.c b/usr.sbin/relayd/relay_http.c
index e9b9c4449e5..4fafd4fdf6a 100644
--- a/usr.sbin/relayd/relay_http.c
+++ b/usr.sbin/relayd/relay_http.c
@@ -78,6 +78,8 @@ int		 relay_match_actions(struct ctl_relay_event *,
 		    struct relay_table **);
 void		 relay_httpdesc_free(struct http_descriptor *);
 char *		 server_root_strip(char *, int);
+int		 relay_http_parse_startline(struct ctl_relay_event *, char *,
+		    enum httpmethod *);
 
 static struct relayd	*env = NULL;
 
@@ -193,8 +195,6 @@ relay_read_http(struct bufferevent *bev, void *arg)
 	struct kv		*upgrade = NULL, *upgrade_ws = NULL;
 	struct kv		*connection_close = NULL;
 	int			 ws_response = 0;
-	struct http_method_node	*hmn;
-	struct http_session	*hs;
 	enum httpmethod		 request_method = HTTP_METHOD_NONE;
 
 	getmonotime(&con->se_tv_last);
@@ -248,137 +248,13 @@ relay_read_http(struct bufferevent *bev, void *arg)
 			relay_abort_http(con, 400, "malformed", 0);
 			goto abort;
 		}
-
-		hs = con->se_priv;
-		DPRINTF("%s: session %d http_session %p", __func__,
-			con->se_id, hs);
-
-		/*
-		 * The first line is the GET/POST/PUT/... request,
-		 * subsequent lines are HTTP headers.
-		 */
+		/* Process the HTTP start-line (request or response status) */
 		if (++cre->line == 1) {
-			key = line;
-			if ((value = strchr(key, ' ')) == NULL) {
-				relay_abort_http(con, 400, "malformed", 0);
-				goto abort;
-			}
-			*value++ = '\0';
-
-			if (cre->dir == RELAY_DIR_RESPONSE) {
-				desc->http_method = HTTP_METHOD_RESPONSE;
-				hmn = SIMPLEQ_FIRST(&hs->hs_methods);
-
-				/*
-				 * There is nothing preventing the relay from
-				 * sending an unbalanced response.  Be prepared.
-				 */
-				if (hmn == NULL) {
-					request_method = HTTP_METHOD_NONE;
-					DPRINTF("%s: session %d unbalanced "
-					    "response", __func__, con->se_id);
-				} else {
-					SIMPLEQ_REMOVE_HEAD(&hs->hs_methods,
-					    hmn_entry);
-					request_method = hmn->hmn_method;
-					DPRINTF("%s: session %d dequeuing %s",
-					    __func__, con->se_id,
-					    relay_httpmethod_byid(request_method));
-					free(hmn);
-				}
-
-				/*
-				 * Decode response path and query
-				 */
-				desc->http_version = strdup(key);
-				if (desc->http_version == NULL) {
-					free(line);
-					goto fail;
-				}
-				desc->http_rescode = strdup(value);
-				if (desc->http_rescode == NULL) {
-					free(line);
-					goto fail;
-				}
-				desc->http_resmesg = strchr(desc->http_rescode,
-				    ' ');
-				if (desc->http_resmesg == NULL) {
-					free(line);
-					goto fail;
-				}
-				*desc->http_resmesg++ = '\0';
-				desc->http_resmesg = strdup(desc->http_resmesg);
-				if (desc->http_resmesg == NULL) {
-					free(line);
-					goto fail;
-				}
-				desc->http_status = strtonum(desc->http_rescode,
-				    100, 599, &errstr);
-				if (errstr) {
-					DPRINTF(
-					    "%s: http_status %s: errno %d, %s",
-					    __func__, desc->http_rescode, errno,
-					    errstr);
-					free(line);
-					goto fail;
-				}
-				DPRINTF("http_version %s http_rescode %s "
-				    "http_resmesg %s", desc->http_version,
-				    desc->http_rescode, desc->http_resmesg);
-			} else if (cre->dir == RELAY_DIR_REQUEST) {
-				desc->http_method =
-				    relay_httpmethod_byname(key);
-				if (desc->http_method == HTTP_METHOD_NONE) {
-					free(line);
-					goto fail;
-				}
-				if ((hmn = calloc(1, sizeof *hmn)) == NULL) {
-					free(line);
-					goto fail;
-				}
-				hmn->hmn_method = desc->http_method;
-				DPRINTF("%s: session %d enqueuing %s",
-				    __func__, con->se_id,
-				    relay_httpmethod_byid(hmn->hmn_method));
-				SIMPLEQ_INSERT_TAIL(&hs->hs_methods, hmn,
-				    hmn_entry);
-				/*
-				 * Decode request path and query
-				 */
-				desc->http_path = strdup(value);
-				if (desc->http_path == NULL) {
-					free(line);
-					goto fail;
-				}
-				desc->http_version = strchr(desc->http_path,
-				    ' ');
-				if (desc->http_version == NULL) {
-					free(line);
-					goto fail;
-				}
-				*desc->http_version++ = '\0';
-				desc->http_query = strchr(desc->http_path, '?');
-				if (desc->http_query != NULL)
-					*desc->http_query++ = '\0';
-
-				/*
-				 * Have to allocate the strings because they
-				 * could be changed independently by the
-				 * filters later.
-				 */
-				if ((desc->http_version =
-				    strdup(desc->http_version)) == NULL) {
-					free(line);
-					goto fail;
-				}
-				if (desc->http_query != NULL &&
-				    (desc->http_query =
-				    strdup(desc->http_query)) == NULL) {
-					free(line);
-					goto fail;
-				}
+			if (relay_http_parse_startline(cre, line,
+			    &request_method) == -1) {
+				free(line);
+				return;
 			}
-
 			free(line);
 			continue;
 		}
@@ -2127,3 +2003,124 @@ server_root_strip(char *path, int n)
 	return (path);
 }
 
+/*
+ * Handle the first line of an HTTP message (Request- or Status-Line).
+ */
+int
+relay_http_parse_startline(struct ctl_relay_event *cre, char *line,
+    enum httpmethod *request_method)
+{
+	struct rsession		*con = cre->con;
+	struct http_descriptor	*desc = cre->desc;
+	struct http_session	*hs = con->se_priv;
+	struct http_method_node	*hmn;
+	char			*key, *value;
+	const char		*errstr;
+
+	DPRINTF("%s: session %d http_session %p", __func__, con->se_id, hs);
+
+	key = line;
+	if ((value = strchr(key, ' ')) == NULL) {
+		relay_abort_http(con, 400, "malformed", 0);
+		return (-1);
+	}
+	*value++ = '\0';
+
+	if (cre->dir == RELAY_DIR_RESPONSE) {
+		desc->http_method = HTTP_METHOD_RESPONSE;
+		hmn = SIMPLEQ_FIRST(&hs->hs_methods);
+
+		/*
+		 * There is nothing preventing the relay from
+		 * sending an unbalanced response.  Be prepared.
+		 */
+		if (hmn == NULL) {
+			*request_method = HTTP_METHOD_NONE;
+			DPRINTF("%s: session %d unbalanced response",
+			    __func__, con->se_id);
+		} else {
+			SIMPLEQ_REMOVE_HEAD(&hs->hs_methods, hmn_entry);
+			*request_method = hmn->hmn_method;
+			DPRINTF("%s: session %d dequeuing %s",
+			    __func__, con->se_id,
+			    relay_httpmethod_byid(*request_method));
+			free(hmn);
+		}
+
+		/*
+		 * Decode response path and query
+		 */
+		desc->http_version = strdup(key);
+		if (desc->http_version == NULL)
+			goto fail;
+
+		desc->http_rescode = strdup(value);
+		if (desc->http_rescode == NULL)
+			goto fail;
+
+		desc->http_resmesg = strchr(desc->http_rescode, ' ');
+		if (desc->http_resmesg == NULL)
+			goto fail;
+
+		*desc->http_resmesg++ = '\0';
+		desc->http_resmesg = strdup(desc->http_resmesg);
+		if (desc->http_resmesg == NULL)
+			goto fail;
+
+		desc->http_status = strtonum(desc->http_rescode, 100, 599,
+		    &errstr);
+		if (errstr) {
+			DPRINTF(
+			    "%s: http_status %s: errno %d, %s",
+			    __func__, desc->http_rescode, errno,
+			    errstr);
+			goto fail;
+		}
+		DPRINTF("http_version %s http_rescode %s http_resmesg %s",
+		    desc->http_version, desc->http_rescode,
+		    desc->http_resmesg);
+	} else if (cre->dir == RELAY_DIR_REQUEST) {
+		desc->http_method = relay_httpmethod_byname(key);
+
+		if (desc->http_method == HTTP_METHOD_NONE)
+			goto fail;
+
+		if ((hmn = calloc(1, sizeof *hmn)) == NULL)
+			goto fail;
+		hmn->hmn_method = desc->http_method;
+		DPRINTF("%s: session %d enqueuing %s", __func__, con->se_id,
+		    relay_httpmethod_byid(hmn->hmn_method));
+		SIMPLEQ_INSERT_TAIL(&hs->hs_methods, hmn, hmn_entry);
+		/*
+		 * Decode request path and query
+		 */
+		desc->http_path = strdup(value);
+		if (desc->http_path == NULL)
+			goto fail;
+
+		desc->http_version = strchr(desc->http_path, ' ');
+		if (desc->http_version == NULL)
+			goto fail;
+
+		*desc->http_version++ = '\0';
+		desc->http_query = strchr(desc->http_path, '?');
+		if (desc->http_query != NULL)
+			*desc->http_query++ = '\0';
+
+		/*
+		 * Have to allocate the strings because they
+		 * could be changed independently by the
+		 * filters later.
+		 */
+		if ((desc->http_version = strdup(desc->http_version)) == NULL)
+			goto fail;
+
+		if (desc->http_query != NULL && (desc->http_query =
+		    strdup(desc->http_query)) == NULL)
+			goto fail;
+	}
+	return (0);
+fail:
+	relay_abort_http(con, 500, strerror(errno), 0);
+	return (-1);
+}