From: Rafael Sadowski Subject: relayd: move HTTP start-line logic to a dedicated function To: tech@openbsd.org Date: Sun, 1 Mar 2026 15:53:13 +0100 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); +}