Encapsulate FastCGI implementation details.

Previously fcgi_request defined in main/fastcgi.h might be treated differently in different files, because of different behavior of #ifdef TCP_NODELAY. This leaded to stack memory corruption and unpredictable crashes.

Author: dstogov

main/fastcgi.c

@@ -132,6 +132,106 @@ static int is_impersonate = 0;
 
 #include "fastcgi.h"
 
+typedef struct _fcgi_header {
+	unsigned char version;
+	unsigned char type;
+	unsigned char requestIdB1;
+	unsigned char requestIdB0;
+	unsigned char contentLengthB1;
+	unsigned char contentLengthB0;
+	unsigned char paddingLength;
+	unsigned char reserved;
+} fcgi_header;
+
+typedef struct _fcgi_begin_request {
+	unsigned char roleB1;
+	unsigned char roleB0;
+	unsigned char flags;
+	unsigned char reserved[5];
+} fcgi_begin_request;
+
+typedef struct _fcgi_begin_request_rec {
+	fcgi_header hdr;
+	fcgi_begin_request body;
+} fcgi_begin_request_rec;
+
+typedef struct _fcgi_end_request {
+    unsigned char appStatusB3;
+    unsigned char appStatusB2;
+    unsigned char appStatusB1;
+    unsigned char appStatusB0;
+    unsigned char protocolStatus;
+    unsigned char reserved[3];
+} fcgi_end_request;
+
+typedef struct _fcgi_end_request_rec {
+	fcgi_header hdr;
+	fcgi_end_request body;
+} fcgi_end_request_rec;
+
+typedef struct _fcgi_hash_bucket {
+	unsigned int              hash_value;
+	unsigned int              var_len;
+	char                     *var;
+	unsigned int              val_len;
+	char                     *val;
+	struct _fcgi_hash_bucket *next;
+	struct _fcgi_hash_bucket *list_next;
+} fcgi_hash_bucket;
+
+typedef struct _fcgi_hash_buckets {
+	unsigned int	           idx;
+	struct _fcgi_hash_buckets *next;
+	struct _fcgi_hash_bucket   data[FCGI_HASH_TABLE_SIZE];
+} fcgi_hash_buckets;
+
+typedef struct _fcgi_data_seg {
+	char                  *pos;
+	char                  *end;
+	struct _fcgi_data_seg *next;
+	char                   data[1];
+} fcgi_data_seg;
+
+typedef struct _fcgi_hash {
+	fcgi_hash_bucket  *hash_table[FCGI_HASH_TABLE_SIZE];
+	fcgi_hash_bucket  *list;
+	fcgi_hash_buckets *buckets;
+	fcgi_data_seg     *data;
+} fcgi_hash;
+
+typedef struct _fcgi_req_hook 	fcgi_req_hook;
+
+struct _fcgi_req_hook {
+	void(*on_accept)();
+	void(*on_read)();
+	void(*on_close)();
+};
+
+struct _fcgi_request {
+	int            listen_socket;
+	int            tcp;
+	int            fd;
+	int            id;
+	int            keep;
+#ifdef TCP_NODELAY
+	int            nodelay;
+#endif
+	int            closed;
+	int            in_len;
+	int            in_pad;
+
+	fcgi_header   *out_hdr;
+
+	unsigned char *out_pos;
+	unsigned char  out_buf[1024*8];
+	unsigned char  reserved[sizeof(fcgi_end_request_rec)];
+
+	fcgi_req_hook  hook;
+
+	int            has_env;
+	fcgi_hash      env;
+};
+
 /* maybe it's better to use weak name instead */
 #ifndef HAVE_ATTRIBUTE_WEAK
 static fcgi_logger fcgi_log;
@@ -772,9 +872,9 @@ static void fcgi_hook_dummy() {
 	return;
 }
 
-fcgi_request *fcgi_init_request(fcgi_request *req, int listen_socket)
+fcgi_request *fcgi_init_request(int listen_socket, void(*on_accept)(), void(*on_read)(), void(*on_close)())
 {
-	memset(req, 0, sizeof(fcgi_request));
+	fcgi_request *req = calloc(1, sizeof(fcgi_request));
 	req->listen_socket = listen_socket;
 	req->fd = -1;
 	req->id = -1;
@@ -794,9 +894,9 @@ fcgi_request *fcgi_init_request(fcgi_request *req, int listen_socket)
 
 	*/
 	req->out_pos = req->out_buf;
-	req->hook.on_accept = fcgi_hook_dummy;
-	req->hook.on_read = fcgi_hook_dummy;
-	req->hook.on_close = fcgi_hook_dummy;
+	req->hook.on_accept = on_accept ? on_accept : fcgi_hook_dummy;
+	req->hook.on_read = on_read ? on_read : fcgi_hook_dummy;
+	req->hook.on_close = on_close ? on_close : fcgi_hook_dummy;
 
 #ifdef _WIN32
 	req->tcp = !GetNamedPipeInfo((HANDLE)_get_osfhandle(req->listen_socket), NULL, NULL, NULL, NULL);
@@ -809,6 +909,7 @@ fcgi_request *fcgi_init_request(fcgi_request *req, int listen_socket)
 
 void fcgi_destroy_request(fcgi_request *req) {
 	fcgi_hash_destroy(&req->env);
+	free(req);
 }
 
 static inline ssize_t safe_write(fcgi_request *req, const void *buf, size_t count)
@@ -1553,6 +1654,11 @@ int fcgi_finish_request(fcgi_request *req, int force_close)
 	return ret;
 }
 
+int fcgi_has_env(fcgi_request *req)
+{
+	return req && req->has_env;
+}
+
 char* fcgi_getenv(fcgi_request *req, const char* var, int var_len)
 {
 	unsigned int val_len;

main/fastcgi.h

@@ -77,43 +77,6 @@ typedef enum _fcgi_protocol_status {
 	FCGI_UNKNOWN_ROLE		= 3
 } dcgi_protocol_status;
 
-typedef struct _fcgi_header {
-	unsigned char version;
-	unsigned char type;
-	unsigned char requestIdB1;
-	unsigned char requestIdB0;
-	unsigned char contentLengthB1;
-	unsigned char contentLengthB0;
-	unsigned char paddingLength;
-	unsigned char reserved;
-} fcgi_header;
-
-typedef struct _fcgi_begin_request {
-	unsigned char roleB1;
-	unsigned char roleB0;
-	unsigned char flags;
-	unsigned char reserved[5];
-} fcgi_begin_request;
-
-typedef struct _fcgi_begin_request_rec {
-	fcgi_header hdr;
-	fcgi_begin_request body;
-} fcgi_begin_request_rec;
-
-typedef struct _fcgi_end_request {
-    unsigned char appStatusB3;
-    unsigned char appStatusB2;
-    unsigned char appStatusB1;
-    unsigned char appStatusB0;
-    unsigned char protocolStatus;
-    unsigned char reserved[3];
-} fcgi_end_request;
-
-typedef struct _fcgi_end_request_rec {
-	fcgi_header hdr;
-	fcgi_end_request body;
-} fcgi_end_request_rec;
-
 /* FastCGI client API */
 
 typedef void (*fcgi_apply_func)(char *var, unsigned int var_len, char *val, unsigned int val_len, void *arg);
@@ -122,69 +85,7 @@ typedef void (*fcgi_apply_func)(char *var, unsigned int var_len, char *val, unsi
 #define FCGI_HASH_TABLE_MASK (FCGI_HASH_TABLE_SIZE - 1)
 #define FCGI_HASH_SEG_SIZE   4096
 
-typedef struct _fcgi_hash_bucket {
-	unsigned int              hash_value;
-	unsigned int              var_len;
-	char                     *var;
-	unsigned int              val_len;
-	char                     *val;
-	struct _fcgi_hash_bucket *next;
-	struct _fcgi_hash_bucket *list_next;
-} fcgi_hash_bucket;
-
-typedef struct _fcgi_hash_buckets {
-	unsigned int	           idx;
-	struct _fcgi_hash_buckets *next;
-	struct _fcgi_hash_bucket   data[FCGI_HASH_TABLE_SIZE];
-} fcgi_hash_buckets;
-
-typedef struct _fcgi_data_seg {
-	char                  *pos;
-	char                  *end;
-	struct _fcgi_data_seg *next;
-	char                   data[1];
-} fcgi_data_seg;
-
-typedef struct _fcgi_hash {
-	fcgi_hash_bucket  *hash_table[FCGI_HASH_TABLE_SIZE];
-	fcgi_hash_bucket  *list;
-	fcgi_hash_buckets *buckets;
-	fcgi_data_seg     *data;
-} fcgi_hash;
-
-typedef struct _fcgi_request 	fcgi_request;
-typedef struct _fcgi_req_hook 	fcgi_req_hook;
-
-struct _fcgi_req_hook {
-	void(*on_accept)();
-	void(*on_read)();
-	void(*on_close)();
-};
-
-struct _fcgi_request {
-	int            listen_socket;
-	int            tcp;
-	int            fd;
-	int            id;
-	int            keep;
-#ifdef TCP_NODELAY
-	int            nodelay;
-#endif
-	int            closed;
-	int            in_len;
-	int            in_pad;
-
-	fcgi_header   *out_hdr;
-
-	unsigned char *out_pos;
-	unsigned char  out_buf[1024*8];
-	unsigned char  reserved[sizeof(fcgi_end_request_rec)];
-
-	fcgi_req_hook  hook;
-
-	int            has_env;
-	fcgi_hash      env;
-};
+typedef struct _fcgi_request fcgi_request;
 
 int fcgi_init(void);
 void fcgi_shutdown(void);
@@ -194,7 +95,7 @@ void fcgi_close(fcgi_request *req, int force, int destroy);
 int fcgi_in_shutdown(void);
 void fcgi_terminate(void);
 int fcgi_listen(const char *path, int backlog);
-fcgi_request* fcgi_init_request(fcgi_request *request, int listen_socket);
+fcgi_request* fcgi_init_request(int listen_socket, void(*on_accept)(), void(*on_read)(), void(*on_close)());
 void fcgi_destroy_request(fcgi_request *req);
 void fcgi_set_allowed_clients(char *ip);
 int fcgi_accept_request(fcgi_request *req);
@@ -207,6 +108,7 @@ typedef void (*fcgi_logger)(int type, const char *fmt, ...);
 void fcgi_set_logger(fcgi_logger lg);
 #endif
 
+int   fcgi_has_env(fcgi_request *req);
 char* fcgi_getenv(fcgi_request *req, const char* var, int var_len);
 char* fcgi_putenv(fcgi_request *req, char* var, int var_len, char* val);
 char* fcgi_quick_getenv(fcgi_request *req, const char* var, int var_len, unsigned int hash_value);

sapi/cgi/cgi_main.c

@@ -1036,12 +1036,12 @@ static int is_valid_path(const char *path)
 /* }}} */
 
 #define CGI_GETENV(name) \
-	((request->has_env) ? \
+	((has_env) ? \
 		FCGI_GETENV(request, name) : \
     	getenv(name))
 
 #define CGI_PUTENV(name, value) \
-	((request->has_env) ? \
+	((has_env) ? \
 		FCGI_PUTENV(request, name, value) : \
 		_sapi_cgi_putenv(name, sizeof(name)-1, value))
 
@@ -1113,6 +1113,7 @@ static int is_valid_path(const char *path)
  */
 static void init_request_info(fcgi_request *request)
 {
+	int has_env = fcgi_has_env(request);
 	char *env_script_filename = CGI_GETENV("SCRIPT_FILENAME");
 	char *env_path_translated = CGI_GETENV("PATH_TRANSLATED");
 	char *script_path_translated = env_script_filename;
@@ -1734,7 +1735,7 @@ int main(int argc, char *argv[])
 	int fastcgi;
 	char *bindpath = NULL;
 	int fcgi_fd = 0;
-	fcgi_request request = {0};
+	fcgi_request *request = NULL;
 	int warmup_repeats = 0;
 	int repeats = 1;
 	int benchmark = 0;
@@ -1972,7 +1973,7 @@ consult the installation file that came with this distribution, or visit \n\
 		php_import_environment_variables = cgi_php_import_environment_variables;
 
 		/* library is already initialized, now init our request */
-		fcgi_init_request(&request, fcgi_fd);
+		request = fcgi_init_request(fcgi_fd, NULL, NULL, NULL);
 
 #ifndef PHP_WIN32
 		/* Pre-fork, if required */
@@ -2101,8 +2102,8 @@ consult the installation file that came with this distribution, or visit \n\
 					break;
 				case 'h':
 				case '?':
-					if (request.listen_socket) {
-						fcgi_destroy_request(&request);
+					if (request) {
+						fcgi_destroy_request(request);
 					}
 					fcgi_shutdown();
 					no_headers = 1;
@@ -2125,9 +2126,9 @@ consult the installation file that came with this distribution, or visit \n\
 			fcgi_impersonate();
 		}
 #endif
-		while (!fastcgi || fcgi_accept_request(&request) >= 0) {
-			SG(server_context) = fastcgi ? (void *)&request : (void *) 1;
-			init_request_info(&request);
+		while (!fastcgi || fcgi_accept_request(request) >= 0) {
+			SG(server_context) = fastcgi ? (void *)request : (void *) 1;
+			init_request_info(request);
 
 			if (!cgi && !fastcgi) {
 				while ((c = php_getopt(argc, argv, OPTIONS, &php_optarg, &php_optind, 0, 2)) != -1) {
@@ -2312,7 +2313,7 @@ consult the installation file that came with this distribution, or visit \n\
 			 * get path_translated */
 			if (php_request_startup() == FAILURE) {
 				if (fastcgi) {
-					fcgi_finish_request(&request, 1);
+					fcgi_finish_request(request, 1);
 				}
 				SG(server_context) = NULL;
 				php_module_shutdown();
@@ -2523,7 +2524,7 @@ consult the installation file that came with this distribution, or visit \n\
 			/* only fastcgi will get here */
 			requests++;
 			if (max_requests && (requests == max_requests)) {
-				fcgi_finish_request(&request, 1);
+				fcgi_finish_request(request, 1);
 				if (bindpath) {
 					free(bindpath);
 				}
@@ -2536,8 +2537,8 @@ consult the installation file that came with this distribution, or visit \n\
 			/* end of fastcgi loop */
 		}
 
-		if (request.listen_socket) {
-			fcgi_destroy_request(&request);
+		if (request) {
+			fcgi_destroy_request(request);
 		}
 		fcgi_shutdown();