some fastcgi improvements:

- DPRINTF instead of log_info for internal debugging.
- submit QUERY_STRING, if it exists
- use a proper function to create an HTTP header.
- use server_file_error() to detect EOF and fastcgi stream errors.
- disable keep-alive/persist for now until we have a reliable way to
get the content length from the cgi response or support chunked
encoding.

"Cool, jep" florian@

4 files changed, 91 insertions, 44 deletions

diff --git a/usr.sbin/httpd/httpd.h b/usr.sbin/httpd/httpd.h
index fe6e0eb51db..2605a068621 100644
--- a/usr.sbin/httpd/httpd.h
+++ b/usr.sbin/httpd/httpd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: httpd.h,v 1.24 2014/07/31 14:25:14 reyk Exp $	*/
+/*	$OpenBSD: httpd.h,v 1.25 2014/07/31 17:55:09 reyk Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2014 Reyk Floeter <reyk@openbsd.org>
@@ -432,9 +432,11 @@ void	 server_close_http(struct client *);
 int	 server_response(struct httpd *, struct client *);
 const char *
 	 server_http_host(struct sockaddr_storage *, char *, size_t);
+void	 server_http_date(char *, size_t);
 
 /* server_file.c */
 int	 server_file(struct httpd *, struct client *);
+void	 server_file_error(struct bufferevent *, short, void *);
 
 /* server_fcgi.c */
 int	 server_fcgi(struct httpd *, struct client *);
diff --git a/usr.sbin/httpd/server_fcgi.c b/usr.sbin/httpd/server_fcgi.c
index 391d0d4908b..bc8d25176c7 100644
--- a/usr.sbin/httpd/server_fcgi.c
+++ b/usr.sbin/httpd/server_fcgi.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: server_fcgi.c,v 1.3 2014/07/31 14:25:14 reyk Exp $	*/
+/*	$OpenBSD: server_fcgi.c,v 1.4 2014/07/31 17:55:09 reyk Exp $	*/
 
 /*
  * Copyright (c) 2014 Florian Obser <florian@openbsd.org>
@@ -81,8 +81,8 @@ struct fcgi_begin_request_body {
 	uint8_t		reserved[5];
 } __packed;
 
+int	server_fcgi_header(struct client *, u_int);
 void	server_fcgi_read(struct bufferevent *, void *);
-void	server_fcgi_error(struct bufferevent *, short, void *);
 int	fcgi_add_param(uint8_t *, char *, char *, int);
 
 int
@@ -99,7 +99,6 @@ server_fcgi(struct httpd *env, struct client *clt)
 	uint8_t				 buf[FCGI_RECORD_SIZE];
 	uint8_t				*params;
 
-	log_info("server_fcgi");
 	if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1)
 		goto fail;
 
@@ -112,21 +111,18 @@ server_fcgi(struct httpd *env, struct client *clt)
 	}
 	sun.sun_len = len;
 
-	log_info("path: %s", sun.sun_path);
 	if (connect(fd, (struct sockaddr *)&sun, sizeof(sun)) == -1)
 		goto fail;
 
 	if (clt->clt_srvbev != NULL)
 		bufferevent_free(clt->clt_srvbev);
+
 	clt->clt_srvbev = bufferevent_new(fd, server_fcgi_read,
-	    NULL, server_fcgi_error, clt);
+	    NULL, server_file_error, clt);
 	if (clt->clt_srvbev == NULL) {
 		errstr = "failed to allocate fcgi buffer event";
 		goto fail;
 	}
-	bufferevent_settimeout(clt->clt_srvbev,
-	    srv_conf->timeout.tv_sec, srv_conf->timeout.tv_sec);
-	bufferevent_enable(clt->clt_srvbev, EV_READ);
 
 	bzero(&buf, sizeof(buf));
 
@@ -155,7 +151,14 @@ server_fcgi(struct httpd *env, struct client *clt)
 	    FCGI_CONTENT_SIZE);
 	params += len;
 	total_len += len;
-	
+
+	if (desc->http_query) {
+		len = fcgi_add_param(params, "QUERY_STRING", desc->http_query,
+		    FCGI_CONTENT_SIZE);
+		params += len;
+		total_len += len;
+	}
+
 	h->content_len = htons(total_len);
 
 	bufferevent_write(clt->clt_srvbev, &buf,
@@ -172,6 +175,18 @@ server_fcgi(struct httpd *env, struct client *clt)
 	bufferevent_write(clt->clt_srvbev, &buf,
 	    sizeof(struct fcgi_record_header));
 
+	bufferevent_settimeout(clt->clt_srvbev,
+	    srv_conf->timeout.tv_sec, srv_conf->timeout.tv_sec);
+	bufferevent_enable(clt->clt_srvbev, EV_READ|EV_WRITE);
+	bufferevent_disable(clt->clt_bev, EV_READ);
+
+	/*
+	 * persist is not supported yet because we don't get the
+	 * Content-Length from slowcgi and don't support chunked encoding.
+	 */
+	clt->clt_persist = 0;
+	clt->clt_done = 0;
+
 	return (0);
  fail:
 	if (errstr == NULL)
@@ -184,7 +199,7 @@ int
 fcgi_add_param(uint8_t *buf, char *key, char *val, int size)
 {
 	int len = 0;
-	log_info("%s => %s", key, val);
+	DPRINTF("%s: %s => %s", __func__, key, val);
 	buf[0] = strlen(key);
 	len++;
 	buf[1] = strlen(val);
@@ -204,27 +219,63 @@ server_fcgi_read(struct bufferevent *bev, void *arg)
 	size_t	 len;
 
 	len = bufferevent_read(bev, &buf, FCGI_RECORD_SIZE);
-
-	log_info("server_fcgi_read: %lu", len);
+	DPRINTF("%s: %lu", __func__, len);
 	
 	h = (struct fcgi_record_header *) &buf;
-	log_info("h->version: %d", h->version);
-	log_info("h->type: %d", h->type);
-	log_info("h->id: %d", ntohs(h->id));
-	log_info("h->content_len: %d", ntohs(h->content_len));
-	
+	DPRINTF("%s: record header: version %d type %d id %d content len %d",
+	    __func__, h->version, h->type, ntohs(h->id),
+	    ntohs(h->content_len));
+
 	if (h->type == FCGI_STDOUT && ntohs(h->content_len) > 0) {
-		log_info("%s", (char*) &buf +
-		     sizeof(struct fcgi_record_header));
-		server_bufferevent_print(clt, "HTTP/1.1 200 OK\r\n");
-		server_bufferevent_print(clt, (char*) &buf +
-		     sizeof(struct fcgi_record_header));
-		
+		DPRINTF("%s", (char *) &buf +
+                    sizeof(struct fcgi_record_header));
+
+		server_fcgi_header(clt, 200);
+		server_bufferevent_write(clt, (char *)&buf +
+		    sizeof(struct fcgi_record_header),
+		    len - sizeof(struct fcgi_record_header));
 	}
 }
 
-void
-server_fcgi_error(struct bufferevent *bev, short error, void *arg)
+int
+server_fcgi_header(struct client *clt, u_int code)
 {
-	log_info("server_fcgi_error: %d", error);
+	struct http_descriptor	*desc = clt->clt_desc;
+	const char		*error;
+	char			 tmbuf[32];
+
+	if (desc == NULL || (error = server_httperror_byid(code)) == NULL)
+		return (-1);
+
+	kv_purge(&desc->http_headers);
+
+	/* Add error codes */
+	if (kv_setkey(&desc->http_pathquery, "%lu", code) == -1 ||
+	    kv_set(&desc->http_pathquery, "%s", error) == -1)
+		return (-1);
+
+	/* Add headers */
+	if (kv_add(&desc->http_headers, "Server", HTTPD_SERVERNAME) == NULL)
+		return (-1);
+
+	/* Is it a persistent connection? */
+	if (clt->clt_persist) {
+		if (kv_add(&desc->http_headers,
+		    "Connection", "keep-alive") == NULL)
+			return (-1);
+	} else if (kv_add(&desc->http_headers, "Connection", "close") == NULL)
+		return (-1);
+
+	/* Date header is mandatory and should be added last */
+	server_http_date(tmbuf, sizeof(tmbuf));
+	if (kv_add(&desc->http_headers, "Date", tmbuf) == NULL)
+		return (-1);
+
+	/* Write initial header (fcgi might append more) */
+	if (server_writeresponse_http(clt) == -1 ||
+	    server_bufferevent_print(clt, "\r\n") == -1 ||
+	    server_writeheader_http(clt) == -1)
+		return (-1);
+
+	return (0);
 }
diff --git a/usr.sbin/httpd/server_file.c b/usr.sbin/httpd/server_file.c
index 035c7088b7d..7084c9f3d77 100644
--- a/usr.sbin/httpd/server_file.c
+++ b/usr.sbin/httpd/server_file.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: server_file.c,v 1.21 2014/07/31 14:25:14 reyk Exp $	*/
+/*	$OpenBSD: server_file.c,v 1.22 2014/07/31 17:55:09 reyk Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2014 Reyk Floeter <reyk@openbsd.org>
@@ -51,7 +51,6 @@
 int	 server_file_access(struct client *, char *, size_t,
 	    struct stat *);
 int	 server_file_index(struct httpd *, struct client *);
-void	 server_file_error(struct bufferevent *, short, void *);
 
 int
 server_file_access(struct client *clt, char *path, size_t len,
@@ -178,7 +177,7 @@ server_file(struct httpd *env, struct client *clt)
 	case 0:
 		/* Connection is already finished */
 		close(fd);
-		return (0);
+		goto done;
 	default:
 		break;
 	}
@@ -199,6 +198,8 @@ server_file(struct httpd *env, struct client *clt)
 	bufferevent_enable(clt->clt_srvbev, EV_READ);
 	bufferevent_disable(clt->clt_bev, EV_READ);
 
+ done:
+	server_reset_http(clt);
 	return (0);
  fail:
 	if (errstr == NULL)
@@ -309,7 +310,7 @@ server_file_index(struct httpd *env, struct client *clt)
 	case 0:
 		/* Connection is already finished */
 		evbuffer_free(evb);
-		return (0);
+		goto done;
 	default:
 		break;
 	}
@@ -325,6 +326,8 @@ server_file_index(struct httpd *env, struct client *clt)
 		clt->clt_toread = TOREAD_HTTP_NONE;
 	clt->clt_done = 0;
 
+ done:
+	server_reset_http(clt);
 	return (0);
 
  fail:
diff --git a/usr.sbin/httpd/server_http.c b/usr.sbin/httpd/server_http.c
index 3eff410c891..4b29da4faa8 100644
--- a/usr.sbin/httpd/server_http.c
+++ b/usr.sbin/httpd/server_http.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: server_http.c,v 1.23 2014/07/31 09:34:57 reyk Exp $	*/
+/*	$OpenBSD: server_http.c,v 1.24 2014/07/31 17:55:09 reyk Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2014 Reyk Floeter <reyk@openbsd.org>
@@ -48,7 +48,6 @@
 #include "httpd.h"
 #include "http.h"
 
-static void	 server_http_date(char *, size_t);
 static int	 server_httpmethod_cmp(const void *, const void *);
 static int	 server_httperror_cmp(const void *, const void *);
 void		 server_httpdesc_free(struct http_descriptor *);
@@ -518,7 +517,7 @@ server_reset_http(struct client *clt)
 	clt->clt_srv_conf = &srv->srv_conf;
 }
 
-static void
+void
 server_http_date(char *tmbuf, size_t len)
 {
 	time_t			 t;
@@ -673,7 +672,6 @@ server_response(struct httpd *httpd, struct client *clt)
 	struct server		*srv = clt->clt_srv;
 	struct server_config	*srv_conf = &srv->srv_conf, *location;
 	struct kv		*kv, key, *host;
-	int			 ret;
 
 	/* Canonicalize the request path */
 	if (desc->http_path == NULL ||
@@ -754,15 +752,8 @@ server_response(struct httpd *httpd, struct client *clt)
 	}
 
 	if (srv_conf->flags & SRVFLAG_FCGI)
-		ret = server_fcgi(httpd, clt);
-	else
-		ret = server_file(httpd, clt);
-	if (ret == -1)
-		return (ret);
-
-	server_reset_http(clt);
-
-	return (0);
+		return (server_fcgi(httpd, clt));
+	return (server_file(httpd, clt));
  fail:
 	server_abort_http(clt, 400, "bad request");
 	return (-1);