[core] open fd when appending file to cq (fixes #2655)

http_chunk_append_file() opens fd when appending file to chunkqueue.
Defers calculation of content length until response is finished.

This reduces race conditions pertaining to stat() and then (later)
open(), when the result of the stat() was used for Content-Length
or to generate chunked headers.

Note: this does not change how lighttpd handles files that are modified
in-place by another process after having been opened by lighttpd --
don't do that.  This *does* improve handling of files that are
frequently modified via a temporary file and then atomically renamed
into place.

mod_fastcgi has been modified to use http_chunk_append_file_range() with
X-Sendfile2 and will open the target file multiple times if there are
multiple ranges.

Note: (future todo) not implemented for chunk.[ch] interfaces used by
range requests in mod_staticfile or by mod_ssi.  Those uses could lead
to too many open fds.  For mod_staticfile, limits should be put in place
for max number of ranges accepted by mod_staticfile.  For mod_ssi,
limits would need to be placed on the maximum number of includes, and
the primary SSI file split across lots of SSI directives should either
copy the pieces or perhaps chunk.h could be extended to allow for an
open fd to be shared across multiple chunks.  Doing either of these
would improve the performance of SSI since they would replace many file
opens on the pieces of the SSI file around the SSI directives.

x-ref:
  "Serving a file that is getting updated can cause an empty response or incorrect content-length error"
  https://redmine.lighttpd.net/issues/2655

github:
Closes #49

13 files changed, 150 insertions, 71 deletions

diff --git a/NEWS b/NEWS
index b0f3b06f..f1037220 100644
--- a/NEWS
+++ b/NEWS
@@ -78,6 +78,7 @@ NEWS
   * [mod_ssi] config ssi.exec (fixes #2051)
   * [mod_redirect,mod_rewrite] short-circuit if blank replacement (fixes #2085)
   * [mod_indexfile] save physical path to env (fixes #448, #892)
+  * [core] open fd when appending file to cq (fixes #2655)
 
 - 1.4.39 - 2016-01-02
   * [core] fix memset_s call (fixes #2698)
diff --git a/src/chunk.c b/src/chunk.c
index d4cebafe..3527152a 100644
--- a/src/chunk.c
+++ b/src/chunk.c
@@ -203,6 +203,27 @@ void chunkqueue_reset(chunkqueue *cq) {
 	cq->bytes_out = 0;
 }
 
+void chunkqueue_append_file_fd(chunkqueue *cq, buffer *fn, int fd, off_t offset, off_t len) {
+	chunk *c;
+
+	if (0 == len) {
+		close(fd);
+		return;
+	}
+
+	c = chunkqueue_get_unused_chunk(cq);
+
+	c->type = FILE_CHUNK;
+
+	buffer_copy_buffer(c->file.name, fn);
+	c->file.start = offset;
+	c->file.length = len;
+	c->file.fd = fd;
+	c->offset = 0;
+
+	chunkqueue_append_chunk(cq, c);
+}
+
 void chunkqueue_append_file(chunkqueue *cq, buffer *fn, off_t offset, off_t len) {
 	chunk *c;
 
diff --git a/src/chunk.h b/src/chunk.h
index d64345a9..95819589 100644
--- a/src/chunk.h
+++ b/src/chunk.h
@@ -51,6 +51,7 @@ typedef struct {
 chunkqueue *chunkqueue_init(void);
 void chunkqueue_set_tempdirs(chunkqueue *cq, array *tempdirs, unsigned int upload_temp_file_size);
 void chunkqueue_append_file(chunkqueue *cq, buffer *fn, off_t offset, off_t len); /* copies "fn" */
+void chunkqueue_append_file_fd(chunkqueue *cq, buffer *fn, int fd, off_t offset, off_t len); /* copies "fn" */
 void chunkqueue_append_mem(chunkqueue *cq, const char *mem, size_t len); /* copies memory */
 void chunkqueue_append_buffer(chunkqueue *cq, buffer *mem); /* may reset "mem" */
 void chunkqueue_prepend_buffer(chunkqueue *cq, buffer *mem); /* may reset "mem" */
diff --git a/src/connections.c b/src/connections.c
index 25925925..fd6cd07a 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -495,11 +495,11 @@ static int connection_handle_write_prepare(server *srv, connection *con) {
 			buffer_append_int(con->physical.path, con->http_status);
 			buffer_append_string_len(con->physical.path, CONST_STR_LEN(".html"));
 
-			if (HANDLER_ERROR != stat_cache_get_entry(srv, con, con->physical.path, &sce)) {
+			if (0 == http_chunk_append_file(srv, con, con->physical.path)) {
 				con->file_finished = 1;
-
-				http_chunk_append_file(srv, con, con->physical.path, 0, sce->st.st_size);
-				response_header_overwrite(srv, con, CONST_STR_LEN("Content-Type"), CONST_BUF_LEN(sce->content_type));
+				if (HANDLER_ERROR != stat_cache_get_entry(srv, con, con->physical.path, &sce)) {
+					response_header_overwrite(srv, con, CONST_STR_LEN("Content-Type"), CONST_BUF_LEN(sce->content_type));
+				}
 			}
 		}
 
diff --git a/src/http_chunk.c b/src/http_chunk.c
index 18b32b30..32d9eef1 100644
--- a/src/http_chunk.c
+++ b/src/http_chunk.c
@@ -9,6 +9,7 @@
 #include "server.h"
 #include "chunk.h"
 #include "http_chunk.h"
+#include "stat_cache.h"
 #include "log.h"
 
 #include <sys/types.h>
@@ -22,7 +23,7 @@
 #include <errno.h>
 #include <string.h>
 
-static void http_chunk_append_len(server *srv, connection *con, size_t len) {
+static void http_chunk_append_len(server *srv, connection *con, uintmax_t len) {
 	buffer *b;
 
 	force_assert(NULL != srv);
@@ -36,27 +37,63 @@ static void http_chunk_append_len(server *srv, connection *con, size_t len) {
 	chunkqueue_append_buffer(con->write_queue, b);
 }
 
+static int http_chunk_append_file_open_fstat(server *srv, connection *con, buffer *fn, struct stat *st) {
+	if (!con->conf.follow_symlink) {
+		/*(preserve existing stat_cache symlink checks)*/
+		stat_cache_entry *sce;
+		if (HANDLER_ERROR == stat_cache_get_entry(srv, con, fn, &sce)) return -1;
+	}
 
-void http_chunk_append_file(server *srv, connection *con, buffer *fn, off_t offset, off_t len) {
-	chunkqueue *cq;
-
-	force_assert(NULL != con);
-	if (0 == len) return;
-
-	cq = con->write_queue;
+	return stat_cache_open_rdonly_fstat(srv, con, fn, st);
+}
 
+static void http_chunk_append_file_fd_range(server *srv, connection *con, buffer *fn, int fd, off_t offset, off_t len) {
+	chunkqueue *cq = con->write_queue;
 
 	if (con->response.transfer_encoding & HTTP_TRANSFER_ENCODING_CHUNKED) {
-		http_chunk_append_len(srv, con, len);
+		http_chunk_append_len(srv, con, (uintmax_t)len);
 	}
 
-	chunkqueue_append_file(cq, fn, offset, len);
+	chunkqueue_append_file_fd(cq, fn, fd, offset, len);
 
 	if (con->response.transfer_encoding & HTTP_TRANSFER_ENCODING_CHUNKED) {
 		chunkqueue_append_mem(cq, CONST_STR_LEN("\r\n"));
 	}
 }
 
+int http_chunk_append_file_range(server *srv, connection *con, buffer *fn, off_t offset, off_t len) {
+	struct stat st;
+	const int fd = http_chunk_append_file_open_fstat(srv, con, fn, &st);
+	if (fd < 0) return -1;
+
+	if (-1 == len) {
+		if (offset >= st.st_size) {
+			close(fd);
+			return (offset == st.st_size) ? 0 : -1;
+		}
+		len = st.st_size - offset;
+	} else if (st.st_size - offset < len) {
+		close(fd);
+		return -1;
+	}
+
+	http_chunk_append_file_fd_range(srv, con, fn, fd, offset, len);
+	return 0;
+}
+
+int http_chunk_append_file(server *srv, connection *con, buffer *fn) {
+	struct stat st;
+	const int fd = http_chunk_append_file_open_fstat(srv, con, fn, &st);
+	if (fd < 0) return -1;
+
+	if (0 != st.st_size) {
+		http_chunk_append_file_fd_range(srv, con, fn, fd, 0, st.st_size);
+	} else {
+		close(fd);
+	}
+	return 0;
+}
+
 void http_chunk_append_buffer(server *srv, connection *con, buffer *mem) {
 	chunkqueue *cq;
 
diff --git a/src/http_chunk.h b/src/http_chunk.h
index eaa771a4..266e24ce 100644
--- a/src/http_chunk.h
+++ b/src/http_chunk.h
@@ -7,7 +7,8 @@
 
 void http_chunk_append_mem(server *srv, connection *con, const char * mem, size_t len); /* copies memory */
 void http_chunk_append_buffer(server *srv, connection *con, buffer *mem); /* may reset "mem" */
-void http_chunk_append_file(server *srv, connection *con, buffer *fn, off_t offset, off_t len); /* copies "fn" */
+int http_chunk_append_file(server *srv, connection *con, buffer *fn); /* copies "fn" */
+int http_chunk_append_file_range(server *srv, connection *con, buffer *fn, off_t offset, off_t len); /* copies "fn" */
 void http_chunk_close(server *srv, connection *con);
 
 #endif
diff --git a/src/mod_cml_lua.c b/src/mod_cml_lua.c
index 6a1ac504..519f9c89 100644
--- a/src/mod_cml_lua.c
+++ b/src/mod_cml_lua.c
@@ -239,11 +239,12 @@ int cache_parse_lua(server *srv, connection *con, plugin_data *p, buffer *fn) {
 
 		lua_pushnil(L);  /* first key */
 		while (lua_next(L, curelem) != 0) {
-			stat_cache_entry *sce = NULL;
 			/* key' is at index -2 and value' at index -1 */
 
 			if (lua_isstring(L, -1)) {
 				const char *s = lua_tostring(L, -1);
+				struct stat st;
+				int fd;
 
 				/* the file is relative, make it absolute */
 				if (s[0] != '/') {
@@ -253,7 +254,8 @@ int cache_parse_lua(server *srv, connection *con, plugin_data *p, buffer *fn) {
 					buffer_copy_string(b, lua_tostring(L, -1));
 				}
 
-				if (HANDLER_ERROR == stat_cache_get_entry(srv, con, b, &sce)) {
+				fd = stat_cache_open_rdonly_fstat(srv, con, b, &st);
+				if (fd < 0) {
 					/* stat failed */
 
 					switch(errno) {
@@ -280,8 +282,8 @@ int cache_parse_lua(server *srv, connection *con, plugin_data *p, buffer *fn) {
 						break;
 					}
 				} else {
-					chunkqueue_append_file(con->write_queue, b, 0, sce->st.st_size);
-					if (sce->st.st_mtime > mtime) mtime = sce->st.st_mtime;
+					chunkqueue_append_file_fd(con->write_queue, b, fd, 0, st.st_size);
+					if (st.st_mtime > mtime) mtime = st.st_mtime;
 				}
 			} else {
 				/* not a string */
diff --git a/src/mod_fastcgi.c b/src/mod_fastcgi.c
index 26f68385..0110d9ae 100644
--- a/src/mod_fastcgi.c
+++ b/src/mod_fastcgi.c
@@ -2258,7 +2258,9 @@ range_success: ;
 					range_len = end_range - begin_range + 1;
 					if (range_len < 0) return 502;
 					if (range_len != 0) {
-						http_chunk_append_file(srv, con, srv->tmp_buf, begin_range, range_len);
+						if (0 != http_chunk_append_file_range(srv, con, srv->tmp_buf, begin_range, range_len)) {
+							return 502;
+						}
 					}
 					sendfile2_content_length += range_len;
 
@@ -2507,24 +2509,14 @@ static int fcgi_demux_response(server *srv, handler_ctx *hctx) {
 				if (host->allow_xsendfile && hctx->send_content_body &&
 				    (NULL != (ds = (data_string *) array_get_element(con->response.headers, "X-LIGHTTPD-send-file"))
 					  || NULL != (ds = (data_string *) array_get_element(con->response.headers, "X-Sendfile")))) {
-					stat_cache_entry *sce;
-
-					if (HANDLER_ERROR != stat_cache_get_entry(srv, con, ds->value, &sce)) {
-						data_string *dcls;
-						if (NULL == (dcls = (data_string *)array_get_unused_element(con->response.headers, TYPE_STRING))) {
-							dcls = data_response_init();
-						}
+					if (0 == http_chunk_append_file(srv, con, ds->value)) {
 						/* found */
-						http_chunk_append_file(srv, con, ds->value, 0, sce->st.st_size);
+						data_string *dcls = (data_string *) array_get_element(con->response.headers, "Content-Length");
+						if (dcls) buffer_reset(dcls->value);
+						con->parsed_response &= ~HTTP_CONTENT_LENGTH;
+						con->response.content_length = -1;
 						hctx->send_content_body = 0; /* ignore the content */
 						joblist_append(srv, con);
-
-						buffer_copy_string_len(dcls->key, "Content-Length", sizeof("Content-Length")-1);
-						buffer_copy_int(dcls->value, sce->st.st_size);
-						array_replace(con->response.headers, (data_unset *)dcls);
-
-						con->parsed_response |= HTTP_CONTENT_LENGTH;
-						con->response.content_length = sce->st.st_size;
 					} else {
 						log_error_write(srv, __FILE__, __LINE__, "sb",
 							"send-file error: couldn't get stat_cache entry for:",
diff --git a/src/mod_flv_streaming.c b/src/mod_flv_streaming.c
index 2cf4cd2e..02eae375 100644
--- a/src/mod_flv_streaming.c
+++ b/src/mod_flv_streaming.c
@@ -210,7 +210,6 @@ URIHANDLER_FUNC(mod_flv_streaming_path_handler) {
 
 		if (0 == strncmp(con->physical.path->ptr + s_len - ct_len, ds->value->ptr, ct_len)) {
 			data_string *get_param;
-			stat_cache_entry *sce = NULL;
 			int start;
 			char *err = NULL;
 			/* if there is a start=[0-9]+ in the header use it as start,
@@ -235,18 +234,12 @@ URIHANDLER_FUNC(mod_flv_streaming_path_handler) {
 
 			if (start <= 0) return HANDLER_GO_ON;
 
-			/* check if start is > filesize */
-			if (HANDLER_GO_ON != stat_cache_get_entry(srv, con, con->physical.path, &sce)) {
-				return HANDLER_GO_ON;
-			}
-
-			if (start > sce->st.st_size) {
+			/* let's build a flv header */
+			http_chunk_append_mem(srv, con, CONST_STR_LEN("FLV\x1\x1\0\0\0\x9\0\0\0\x9"));
+			if (0 != http_chunk_append_file_range(srv, con, con->physical.path, start, -1)) {
+				chunkqueue_reset(con->write_queue);
 				return HANDLER_GO_ON;
 			}
-
-			/* we are safe now, let's build a flv header */
-			http_chunk_append_mem(srv, con, CONST_STR_LEN("FLV\x1\x1\0\0\0\x9\0\0\0\x9"));
-			http_chunk_append_file(srv, con, con->physical.path, start, sce->st.st_size - start);
 			http_chunk_close(srv, con);
 
 			response_header_overwrite(srv, con, CONST_STR_LEN("Content-Type"), CONST_STR_LEN("video/x-flv"));
diff --git a/src/mod_magnet.c b/src/mod_magnet.c
index 9afba892..260cf425 100644
--- a/src/mod_magnet.c
+++ b/src/mod_magnet.c
@@ -3,6 +3,7 @@
 #include "base.h"
 #include "log.h"
 #include "buffer.h"
+#include "http_chunk.h"
 
 #include "plugin.h"
 
@@ -737,29 +738,24 @@ static int magnet_attach_content(server *srv, connection *con, lua_State *L, int
 				lua_getfield(L, -3, "offset"); /* (0-based) start of range */
 
 				if (lua_isstring(L, -3)) { /* filename has to be a string */
-					buffer *fn;
-					stat_cache_entry *sce;
-					handler_t res;
-
-					fn = magnet_checkbuffer(L, -3);
-
-					res = stat_cache_get_entry(srv, con, fn, &sce);
-
-					if (HANDLER_GO_ON == res) {
-						off_t off = (off_t) luaL_optinteger(L, -1, 0);
-						off_t end = (off_t) luaL_optinteger(L, -2, (lua_Integer) sce->st.st_size);
-
-						if (off < 0) {
-							buffer_free(fn);
-							return luaL_error(L, "offset for '%s' is negative", lua_tostring(L, -3));
-						}
+					buffer *fn = magnet_checkbuffer(L, -3);
+					off_t off = (off_t) luaL_optinteger(L, -1, 0);
+					off_t len = (off_t) luaL_optinteger(L, -2, -1); /*(-1 to http_chunk_append_file_range() uses file size minus offset)*/
+					if (off < 0) {
+						buffer_free(fn);
+						return luaL_error(L, "offset for '%s' is negative", lua_tostring(L, -3));
+					}
 
-						if (end < off) {
-							buffer_free(fn);
-							return luaL_error(L, "offset > length for '%s'", lua_tostring(L, -3));
-						}
+					if (len >= off) {
+						len -= off;
+					} else if (-1 != len) {
+						buffer_free(fn);
+						return luaL_error(L, "offset > length for '%s'", lua_tostring(L, -3));
+					}
 
-						chunkqueue_append_file(con->write_queue, fn, off, end - off);
+					if (0 != len && 0 != http_chunk_append_file_range(srv, con, fn, off, len)) {
+						buffer_free(fn);
+						return luaL_error(L, "error opening file content '%s' at offset %lld", lua_tostring(L, -3), (long long)off);
 					}
 
 					buffer_free(fn);
diff --git a/src/mod_staticfile.c b/src/mod_staticfile.c
index 5bfa3b93..a0fa2933 100644
--- a/src/mod_staticfile.c
+++ b/src/mod_staticfile.c
@@ -540,10 +540,12 @@ URIHANDLER_FUNC(mod_staticfile_subrequest) {
 	/* we add it here for all requests
 	 * the HEAD request will drop it afterwards again
 	 */
-	http_chunk_append_file(srv, con, con->physical.path, 0, sce->st.st_size);
-
-	con->http_status = 200;
-	con->file_finished = 1;
+	if (0 == sce->st.st_size || 0 == http_chunk_append_file(srv, con, con->physical.path)) {
+		con->http_status = 200;
+		con->file_finished = 1;
+	} else {
+		con->http_status = 403;
+	}
 
 	return HANDLER_FINISHED;
 }
diff --git a/src/stat_cache.c b/src/stat_cache.c
index fa9b7cb9..9464625f 100644
--- a/src/stat_cache.c
+++ b/src/stat_cache.c
@@ -691,6 +691,38 @@ handler_t stat_cache_get_entry(server *srv, connection *con, buffer *name, stat_
 	return HANDLER_GO_ON;
 }
 
+int stat_cache_open_rdonly_fstat (server *srv, connection *con, buffer *name, struct stat *st) {
+	/*(Note: O_NOFOLLOW affects only the final path segment, the target file,
+	 * not any intermediate symlinks along the path)*/
+	#ifndef O_BINARY
+	#define O_BINARY 0
+	#endif
+	#ifndef O_LARGEFILE
+	#define O_LARGEFILE 0
+	#endif
+	#ifndef O_NOCTTY
+	#define O_NOCTTY 0
+	#endif
+	#ifndef O_NONBLOCK
+	#define O_NONBLOCK 0
+	#endif
+	#ifndef O_NOFOLLOW
+	#define O_NOFOLLOW 0
+	#endif
+	const int oflags = O_BINARY | O_LARGEFILE | O_NOCTTY | O_NONBLOCK
+			 | (con->conf.follow_symlink ? 0 : O_NOFOLLOW);
+	const int fd = open(name->ptr, O_RDONLY | oflags);
+	if (fd >= 0) {
+		if (0 == fstat(fd, st)) {
+			return fd;
+		} else {
+			close(fd);
+		}
+	}
+	UNUSED(srv); /*(might log_error_write(srv, ...) in the future)*/
+	return -1;
+}
+
 /**
  * remove stat() from cache which havn't been stat()ed for
  * more than 10 seconds
diff --git a/src/stat_cache.h b/src/stat_cache.h
index 5c22e49a..72c61f4d 100644
--- a/src/stat_cache.h
+++ b/src/stat_cache.h
@@ -9,6 +9,7 @@ void stat_cache_free(stat_cache *fc);
 
 handler_t stat_cache_get_entry(server *srv, connection *con, buffer *name, stat_cache_entry **fce);
 handler_t stat_cache_handle_fdevent(server *srv, void *_fce, int revent);
+int stat_cache_open_rdonly_fstat (server *srv, connection *con, buffer *name, struct stat *st);
 
 int stat_cache_trigger_cleanup(server *srv);
 #endif