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).
This commit is contained in:
Omar Polo 2021-02-01 22:04:51 +00:00
parent b06f80cdf4
commit 35744950aa
3 changed files with 35 additions and 38 deletions

9
gmid.h
View File

@ -42,6 +42,7 @@
#define SUCCESS 20 #define SUCCESS 20
#define TEMP_REDIRECT 30 #define TEMP_REDIRECT 30
#define TEMP_FAILURE 40 #define TEMP_FAILURE 40
#define CGI_ERROR 42
#define NOT_FOUND 51 #define NOT_FOUND 51
#define PROXY_REFUSED 53 #define PROXY_REFUSED 53
#define BAD_REQUEST 59 #define BAD_REQUEST 59
@ -134,10 +135,13 @@ typedef void (*statefn)(struct pollfd*, struct client*);
* handle_handshake -> handle_open_conn * handle_handshake -> handle_open_conn
* handle_handshake -> close_conn // on err * 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_directory_listing // ...same
* handle_open_conn -> send_file // ...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 * handle_cgi -> close_conn
* *
@ -229,6 +233,7 @@ int read_next_dir_entry(struct client*);
void send_directory_listing(struct pollfd*, struct client*); void send_directory_listing(struct pollfd*, struct client*);
void cgi_poll_on_child(struct pollfd*, struct client*); void cgi_poll_on_child(struct pollfd*, struct client*);
void cgi_poll_on_client(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 handle_cgi(struct pollfd*, struct client*);
void close_conn(struct pollfd*, struct client*); void close_conn(struct pollfd*, struct client*);
void do_accept(int, struct tls*, struct pollfd*, struct client*); void do_accept(int, struct tls*, struct pollfd*, struct client*);

View File

@ -163,7 +163,7 @@ eq "$(head /slow)" "20 text/gemini" "Unexpected head for /slow"
eq "$(get /slow)" "# hello world$ln" "Unexpected body for /slow" eq "$(get /slow)" "# hello world$ln" "Unexpected body for /slow"
echo OK GET /slow with cgi 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" eq "$(get /err)" "" "Unexpected body for /err"
echo OK GET /err with cgi echo OK GET /err with cgi

View File

@ -444,10 +444,9 @@ start_cgi(const char *spath, const char *relpath,
start_reply(fds, c, TEMP_FAILURE, "internal server error"); start_reply(fds, c, TEMP_FAILURE, "internal server error");
return; return;
} }
c->state = handle_cgi;
cgi_poll_on_child(fds, c); cgi_poll_on_child(fds, c);
c->code = -1; c->state = handle_cgi_reply;
/* handle_cgi(fds, c); */
return; return;
err: err:
@ -671,41 +670,37 @@ cgi_poll_on_client(struct pollfd *fds, struct client *c)
c->fd = fd; c->fd = fd;
} }
/* handle the read from the child process. Return like read(2) */ /* accumulate the meta line from the cgi script. */
static ssize_t void
read_from_cgi(struct client *c) handle_cgi_reply(struct pollfd *fds, struct client *c)
{ {
void *buf; void *buf, *e;
size_t len; size_t len;
ssize_t r; ssize_t r;
/* if we haven't read a whole response line, we want to buf = c->sbuf + c->len;
* continue reading. */ len = sizeof(c->sbuf) - c->len;
if (c->code == -1) { /* we're polling on the child! */
buf = c->sbuf + c->len; r = read(fds->fd, buf, len);
len = sizeof(c->sbuf) - c->len; if (r == 0 || r == -1) {
} else { cgi_poll_on_client(fds, c);
buf = c->sbuf; start_reply(fds, c, CGI_ERROR, "CGI error");
len = sizeof(c->sbuf); return;
} }
r = read(c->fd, buf, len);
if (r == 0 || r == -1)
return r;
c->len += r; c->len += r;
c->off = 0;
if (c->code != -1) /* TODO: error if the CGI script don't reply correctly */
return r; e = strchr(c->sbuf, '\n');
if (e != NULL || c->len == sizeof(c->sbuf)) {
if (strchr(c->sbuf, '\n') || c->len == sizeof(c->sbuf)) {
c->code = 0;
log_request(c, c->sbuf, c->len); log_request(c, c->sbuf, c->len);
}
return r; c->off = 0;
c->state = handle_cgi;
c->state(fds, c);
return;
}
} }
void void
@ -717,25 +712,22 @@ handle_cgi(struct pollfd *fds, struct client *c)
cgi_poll_on_client(fds, c); cgi_poll_on_client(fds, c);
while (1) { while (1) {
if (c->code == -1 || c->len == 0) { if (c->len == 0) {
switch (r = read_from_cgi(c)) { switch (r = read(c->fd, c->sbuf, sizeof(c->sbuf))) {
case 0: case 0:
goto end; goto end;
case -1: case -1:
if (errno == EAGAIN || errno == EWOULDBLOCK) { if (errno == EAGAIN || errno == EWOULDBLOCK) {
cgi_poll_on_child(fds, c); cgi_poll_on_child(fds, c);
return; return;
} }
goto end; goto end;
default:
c->len = r;
c->off = 0;
} }
} }
if (c->code == -1) {
cgi_poll_on_child(fds, c);
return;
}
while (c->len > 0) { while (c->len > 0) {
switch (r = tls_write(c->ctx, c->sbuf + c->off, c->len)) { switch (r = tls_write(c->ctx, c->sbuf + c->off, c->len)) {
case -1: case -1: