Download raw body.
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);
+}
relayd: move HTTP start-line logic to a dedicated function