From cebbf23c9fbb7df6bcc4715bf4c9221520b00cd2 Mon Sep 17 00:00:00 2001 From: reyk Date: Thu, 31 Jul 2014 17:55:09 +0000 Subject: [PATCH] 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@ --- usr.sbin/httpd/httpd.h | 4 +- usr.sbin/httpd/server_fcgi.c | 103 ++++++++++++++++++++++++++--------- usr.sbin/httpd/server_file.c | 11 ++-- usr.sbin/httpd/server_http.c | 17 ++---- 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 @@ -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 @@ -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 @@ -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 @@ -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); -- 2.20.1