commit 35744950aa0953bee3035400e8035af844a675ba from: Omar Polo date: Mon Feb 01 22:04:51 2021 UTC simplify handle_cgi Now that I got rid of the enum+switch, adding more state is easier. Before, we used an hack to remember if we had read the CGI reply or not (c->code = -1). This introduces a new state, handle_cgi_reply that reads the CGI script reply, logs it, and only then switches to handle_cgi. handle_cgi itself is cleaner, now it only reads into c->sbuf and send what it had red. We even get, almost for free, the 42 error. If read exists with -1 or 0 from in handle_cgi_reply, we return a proper error to the client. We can extend this further in the future and also try to validate the CGI reply (for now we're only looking for a \n). commit - b06f80cdf43be684bd57f9674917d2479bc0d317 commit + 35744950aa0953bee3035400e8035af844a675ba blob - 4cf6266ffe374eb56bd65c42e3af47a237b0fe5d blob + 1ba4a9e90b1b681d79146270694f5d845b642855 --- gmid.h +++ gmid.h @@ -42,6 +42,7 @@ #define SUCCESS 20 #define TEMP_REDIRECT 30 #define TEMP_FAILURE 40 +#define CGI_ERROR 42 #define NOT_FOUND 51 #define PROXY_REFUSED 53 #define BAD_REQUEST 59 @@ -134,11 +135,14 @@ typedef void (*statefn)(struct pollfd*, struct client* * handle_handshake -> handle_open_conn * handle_handshake -> close_conn // on err * - * handle_open_conn -> handle_cgi // via open_file/dir/... + * handle_open_conn -> handle_cgi_reply // via open_file/dir/... * handle_open_conn -> send_directory_listing // ...same * handle_open_conn -> send_file // ...same - * handle_open_conn -> close_conn // on error + * handle_open_conn -> start_reply // on error * + * handle_cgi_reply -> handle_cgi // after logging the CGI reply + * handle_cgi_reply -> start_reply // on error + * * handle_cgi -> close_conn * * send_directory_listing -> close_conn @@ -229,6 +233,7 @@ int read_next_dir_entry(struct client*); void send_directory_listing(struct pollfd*, struct client*); void cgi_poll_on_child(struct pollfd*, struct client*); void cgi_poll_on_client(struct pollfd*, struct client*); +void handle_cgi_reply(struct pollfd*, struct client*); void handle_cgi(struct pollfd*, struct client*); void close_conn(struct pollfd*, struct client*); void do_accept(int, struct tls*, struct pollfd*, struct client*); blob - cd35b70d9cdb34405b104054ef52a5d4bacf3329 blob + 5ffd83fbb081331521f87cd015054fe9521eb30d --- regress/runtime +++ regress/runtime @@ -163,7 +163,7 @@ eq "$(head /slow)" "20 text/gemini" "Unexpected head eq "$(get /slow)" "# hello world$ln" "Unexpected body for /slow" echo OK GET /slow with cgi -eq "$(head /err)" "" "Unexpected head for /err" +eq "$(head /err)" "42 CGI error" "Unexpected head for /err" eq "$(get /err)" "" "Unexpected body for /err" echo OK GET /err with cgi blob - 40b9c7356223916bfc402bc86cf90e13a6bb6116 blob + 9292e0c3b60a1ba10fc9c0ef463d9c3cb4c955fc --- server.c +++ server.c @@ -444,10 +444,9 @@ start_cgi(const char *spath, const char *relpath, start_reply(fds, c, TEMP_FAILURE, "internal server error"); return; } - c->state = handle_cgi; + cgi_poll_on_child(fds, c); - c->code = -1; - /* handle_cgi(fds, c); */ + c->state = handle_cgi_reply; return; err: @@ -671,41 +670,37 @@ cgi_poll_on_client(struct pollfd *fds, struct client * c->fd = fd; } -/* handle the read from the child process. Return like read(2) */ -static ssize_t -read_from_cgi(struct client *c) +/* accumulate the meta line from the cgi script. */ +void +handle_cgi_reply(struct pollfd *fds, struct client *c) { - void *buf; + void *buf, *e; size_t len; ssize_t r; - /* if we haven't read a whole response line, we want to - * continue reading. */ + buf = c->sbuf + c->len; + len = sizeof(c->sbuf) - c->len; - if (c->code == -1) { - buf = c->sbuf + c->len; - len = sizeof(c->sbuf) - c->len; - } else { - buf = c->sbuf; - len = sizeof(c->sbuf); + /* we're polling on the child! */ + r = read(fds->fd, buf, len); + if (r == 0 || r == -1) { + cgi_poll_on_client(fds, c); + start_reply(fds, c, CGI_ERROR, "CGI error"); + return; } - - r = read(c->fd, buf, len); - if (r == 0 || r == -1) - return r; c->len += r; - c->off = 0; - if (c->code != -1) - return r; - - if (strchr(c->sbuf, '\n') || c->len == sizeof(c->sbuf)) { - c->code = 0; + /* TODO: error if the CGI script don't reply correctly */ + e = strchr(c->sbuf, '\n'); + if (e != NULL || c->len == sizeof(c->sbuf)) { log_request(c, c->sbuf, c->len); - } - return r; + c->off = 0; + c->state = handle_cgi; + c->state(fds, c); + return; + } } void @@ -717,25 +712,22 @@ handle_cgi(struct pollfd *fds, struct client *c) cgi_poll_on_client(fds, c); while (1) { - if (c->code == -1 || c->len == 0) { - switch (r = read_from_cgi(c)) { + if (c->len == 0) { + switch (r = read(c->fd, c->sbuf, sizeof(c->sbuf))) { case 0: goto end; - case -1: if (errno == EAGAIN || errno == EWOULDBLOCK) { cgi_poll_on_child(fds, c); return; } goto end; + default: + c->len = r; + c->off = 0; } } - if (c->code == -1) { - cgi_poll_on_child(fds, c); - return; - } - while (c->len > 0) { switch (r = tls_write(c->ctx, c->sbuf + c->off, c->len)) { case -1: