Commit Diff

Commit:
35744950aa0953bee3035400e8035af844a675ba
From:
Omar Polo <op@omarpolo.com>
Date:
Message:
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).
Actions:
Patch | Tree
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: