httpd shrink and logging update, part 2 of 7

   text    data     bss     dec     hex filename
   9836       0       0    9836    266c busybox.t1/networking/httpd.o.orig
   9724       0       0    9724    25fc busybox.t2/networking/httpd.o
   9657       0       0    9657    25b9 busybox.t3/networking/httpd.o
   9342       0       0    9342    247e busybox.t4/networking/httpd.o
   9342       0       0    9342    247e busybox.t5/networking/httpd.o
   9262       0       0    9262    242e busybox.t6/networking/httpd.o
   9283       0       0    9283    2443 busybox.t7/networking/httpd.o
   9334       0       0    9334    2476 busybox.t8/networking/httpd.o

1 files changed, 82 insertions, 92 deletions

diff --git a/networking/httpd.c b/networking/httpd.c
index b44beaa..2ac19cf 100644
--- a/networking/httpd.c
+++ b/networking/httpd.c
@@ -107,9 +107,9 @@ static const char httpd_conf[] ALIGN1 = "httpd.conf";
 #define TIMEOUT 60
 
 // Note: busybox xfuncs are not used because we want the server to keep running
-//       if something bad happens due to a malformed user request.
-//       As a result, all memory allocation after daemonize
-//       is checked rigorously
+// if something bad happens due to a malformed user request.
+// As a result, all memory allocations after daemonize
+// are checked rigorously
 
 //#define DEBUG 1
 #define DEBUG 0
@@ -875,7 +875,7 @@ static int sendHeaders(HttpResponseNum responseNum)
 				found_mime_type : "text/html";
 
 	if (verbose)
-		write(2, iobuf, sprintf(iobuf, "%s response:%u\n", rmt_ip_str, responseNum));
+		bb_error_msg("response:%u", responseNum);
 
 	/* emit the current date */
 	strftime(tmp_str, sizeof(tmp_str), RFC1123FMT, gmtime(&timer));
@@ -946,34 +946,34 @@ static int get_line(void)
 }
 
 #if ENABLE_FEATURE_HTTPD_CGI
-/****************************************************************************
- *
- > $Function: sendCgi()
- *
- * $Description: Execute a CGI script and send it's stdout back
- *
- *   Environment variables are set up and the script is invoked with pipes
- *   for stdin/stdout.  If a post is being done the script is fed the POST
- *   data in addition to setting the QUERY_STRING variable (for GETs or POSTs).
- *
- * $Parameters:
- *      (const char *) url . . . . . . The requested URL (with leading /).
- *      (int bodyLen)  . . . . . . . . Length of the post body.
- *      (const char *cookie) . . . . . For set HTTP_COOKIE.
- *      (const char *content_type) . . For set CONTENT_TYPE.
- *
- * $Return: (char *)  . . . . A pointer to the decoded string (same as input).
+/*
+ * Spawn CGI script, forward CGI's stdin/out <=> network
  *
- * $Errors: None
+ * Environment variables are set up and the script is invoked with pipes
+ * for stdin/stdout.  If a post is being done the script is fed the POST
+ * data in addition to setting the QUERY_STRING variable (for GETs or POSTs).
  *
- ****************************************************************************/
-static int sendCgi(const char *url,
-		const char *request, int bodyLen, const char *cookie,
+ * Parameters:
+ * const char *url              The requested URL (with leading /).
+ * int bodyLen                  Length of the post body.
+ * const char *cookie           For set HTTP_COOKIE.
+ * const char *content_type     For set CONTENT_TYPE.
+ */
+static void send_cgi_and_exit(
+		const char *url,
+		const char *request,
+		int bodyLen,
+		const char *cookie,
+		const char *content_type) ATTRIBUTE_NORETURN;
+static void send_cgi_and_exit(
+		const char *url,
+		const char *request,
+		int bodyLen,
+		const char *cookie,
 		const char *content_type)
 {
 	struct { int rd; int wr; } fromCgi;  /* CGI -> httpd pipe */
 	struct { int rd; int wr; } toCgi;    /* httpd -> CGI pipe */
-	char *fullpath;
 	char *argp[] = { NULL, NULL };
 	int pid = 0;
 	int inFd;
@@ -982,10 +982,8 @@ static int sendCgi(const char *url,
 	int status;
 	size_t post_read_size, post_read_idx;
 
-	if (pipe(&fromCgi.rd) != 0)
-		return 0;
-	if (pipe(&toCgi.rd) != 0)
-		return 0;
+	xpipe(&fromCgi.rd);
+	xpipe(&toCgi.rd);
 
 /*
  * Note: We can use vfork() here in the no-mmu case, although
@@ -995,27 +993,26 @@ static int sendCgi(const char *url,
  *    exits. This happens instantly after the child finishes,
  *    since httpd is run from inetd (and it can't run standalone
  *    in uClinux).
+ * TODO: we can muck with environment _first_ and then fork/exec,
+ * that will be more understandable, and safer wrt vfork!
  */
 
-// FIXME: setenv leaks memory! (old values of env vars are leaked)
-// Thus we have a bug on NOMMU.
-// Need to use this instead:
-// [malloc +] putenv + (in child: exec) + (in parent: unsetenv [+ free])
-
 #if !BB_MMU
-	fullpath = NULL;
 	pid = vfork();
 #else
 	pid = fork();
 #endif
 	if (pid < 0)
-		return 0;
+		_exit(0);
 
 	if (!pid) {
 		/* child process */
+		char *fullpath;
 		char *script;
 		char *purl;
 
+		xfunc_error_retval = 242;
+
 		if (accepted_socket > 1)
 			close(accepted_socket);
 		if (server_socket > 1)
@@ -1026,13 +1023,12 @@ static int sendCgi(const char *url,
 		close(fromCgi.rd);
 		close(toCgi.wr);
 		/* Huh? User seeing stderr can be a security problem.
-		 * If CGI really wants that, it can always dup2(1,2). */
+		 * If CGI really wants that, it can always do dup itself. */
 		/* dup2(1, 2); */
 
 		/*
 		 * Find PATH_INFO.
 		 */
-		xfunc_error_retval = 242;
 		purl = xstrdup(url);
 		script = purl;
 		while ((script = strchr(script + 1, '/')) != NULL) {
@@ -1040,7 +1036,7 @@ static int sendCgi(const char *url,
 			struct stat sb;
 
 			*script = '\0';
-			if (is_directory(purl + 1, 1, &sb) == 0) {
+			if (!is_directory(purl + 1, 1, &sb)) {
 				/* not directory, found script.cgi/PATH_INFO */
 				*script = '/';
 				break;
@@ -1050,10 +1046,7 @@ static int sendCgi(const char *url,
 		setenv1("PATH_INFO", script);   /* set /PATH_INFO or "" */
 		setenv1("REQUEST_METHOD", request);
 		if (g_query) {
-			char *uri = alloca(strlen(purl) + 2 + strlen(g_query));
-			if (uri)
-				sprintf(uri, "%s?%s", purl, g_query);
-			setenv1("REQUEST_URI", uri);
+			putenv(xasprintf("%s=%s?%s", "REQUEST_URI", purl, g_query));
 		} else {
 			setenv1("REQUEST_URI", purl);
 		}
@@ -1082,7 +1075,7 @@ static int sendCgi(const char *url,
 		 * IOW - REMOTE_PEER="1.2.3.4:56" makes much more sense.
 		 * Oh well... */
 		{
-			char *p = rmt_ip_str ? : (char*)"";
+			char *p = rmt_ip_str ? rmt_ip_str : (char*)"";
 			char *cp = strrchr(p, ':');
 			if (ENABLE_FEATURE_IPV6 && cp && strchr(cp, ']'))
 				cp = NULL;
@@ -1150,10 +1143,6 @@ static int sendCgi(const char *url,
 	} /* end child */
 
 	/* parent process */
-#if !BB_MMU
-	free(fullpath);
-#endif
-
 	buf_count = 0;
 	post_read_size = 0;
 	post_read_idx = 0; /* for gcc */
@@ -1306,23 +1295,15 @@ static int sendCgi(const char *url,
 				fprintf(stderr, "cgi read %d bytes: '%.*s'\n", count, count, rbuf);
 		} /* if (FD_ISSET(inFd)) */
 	} /* while (1) */
-	return 0;
+	_exit(0);
 }
 #endif          /* FEATURE_HTTPD_CGI */
 
-/****************************************************************************
- *
- > $Function: sendFile()
- *
- * $Description: Send a file response to a HTTP request
- *
- * $Parameter:
- *      (const char *) url . . The URL requested.
- *
- * $Return: (int)  . . . . . . Always 0.
- *
- ****************************************************************************/
-static int sendFile(const char *url)
+/*
+ * Send a file response to a HTTP request, and exit
+ */
+static void send_file_and_exit(const char *url) ATTRIBUTE_NORETURN;
+static void send_file_and_exit(const char *url)
 {
 	char *suffix;
 	int f;
@@ -1365,13 +1346,18 @@ static int sendFile(const char *url)
 		if (DEBUG)
 			bb_perror_msg("cannot open '%s'", url);
 		sendHeaders(HTTP_NOT_FOUND);
-		return 0;
+		_exit(0);
 	}
 
 	sendHeaders(HTTP_OK);
 	fd = accepted_socket;
 	if (fd == 0)
 		fd++; /* write to fd #1 in inetd mode */
+
+	/* If you want to know about EPIPEs below
+	 * (happen if you abort downloads from local httpd) */
+	/*signal(SIGPIPE, SIG_IGN);*/
+
 #if ENABLE_FEATURE_HTTPD_USE_SENDFILE
 	do {
 		/* byte count is rounded down to 64k */
@@ -1379,20 +1365,26 @@ static int sendFile(const char *url)
 		if (count < 0) {
 			if (offset == 0)
 				goto fallback;
-			bb_perror_msg("sendfile '%s'", url);
+			goto fin;
 		}
 	} while (count > 0);
-	close(f);
-	return 0;
+	_exit(0);
 
  fallback:
 #endif
+	/* TODO: why full_read? safe_read maybe? */
 	while ((count = full_read(f, iobuf, MAX_MEMORY_BUF)) > 0) {
-		if (full_write(fd, iobuf, count) != count)
+		ssize_t n = count;
+		count = full_write(fd, iobuf, count);
+		if (count != n)
 			break;
 	}
-	close(f);
-	return 0;
+#if ENABLE_FEATURE_HTTPD_USE_SENDFILE
+ fin:
+#endif
+	if (count < 0)
+		bb_perror_msg("error:%u", errno);
+	_exit(0);
 }
 
 static int checkPermIP(void)
@@ -1645,11 +1637,8 @@ static void handle_incoming_and_exit(void)
 				found_moved_temporarily = url;
 			}
 		}
-		if (verbose > 1) {
-			/* Hopefully does it with single write[v] */
-			/* (uclibc does, glibc: ?) */
-			fdprintf(2, "%s url:%s\n", rmt_ip_str, url);
-		}
+		if (verbose > 1)
+			bb_error_msg("url:%s", url);
 
 		test = url;
 		ip_allowed = checkPermIP();
@@ -1671,8 +1660,7 @@ static void handle_incoming_and_exit(void)
 					break; /* EOF or error or empty line */
 
 				if (DEBUG)
-					fprintf(stderr, "header: '%s'\n", iobuf);
-
+					bb_error_msg("header: '%s'", iobuf);
 #if ENABLE_FEATURE_HTTPD_CGI
 				/* try and do our best to parse more lines */
 				if ((STRNCASECMP(iobuf, "Content-length:") == 0)) {
@@ -1700,7 +1688,6 @@ static void handle_incoming_and_exit(void)
 					user_agent = strdup(skip_whitespace(iobuf + sizeof("User-Agent:")-1));
 				}
 #endif
-
 #if ENABLE_FEATURE_HTTPD_BASIC_AUTH
 				if (STRNCASECMP(iobuf, "Authorization:") == 0) {
 					/* We only allow Basic credentials.
@@ -1716,7 +1703,6 @@ static void handle_incoming_and_exit(void)
 					credentials = checkPerm(url, test);
 				}
 #endif          /* FEATURE_HTTPD_BASIC_AUTH */
-
 			} /* while extra header reading */
 		}
 		alarm(0);
@@ -1752,8 +1738,7 @@ static void handle_incoming_and_exit(void)
 		if (strncmp(test, "cgi-bin", 7) == 0) {
 			if (test[7] == '/' && test[8] == '\0')
 				goto FORBIDDEN;     /* protect listing cgi-bin/ */
-			sendCgi(url, prequest, length, cookie, content_type);
-			break;
+			send_cgi_and_exit(url, prequest, length, cookie, content_type);
 		}
 #if ENABLE_FEATURE_HTTPD_CONFIG_WITH_SCRIPT_INTERPR
 		{
@@ -1762,8 +1747,7 @@ static void handle_incoming_and_exit(void)
 				Htaccess *cur;
 				for (cur = script_i; cur; cur = cur->next) {
 					if (strcmp(cur->before_colon + 1, suffix) == 0) {
-						sendCgi(url, prequest, length, cookie, content_type);
-						goto bail_out;
+						send_cgi_and_exit(url, prequest, length, cookie, content_type);
 					}
 				}
 			}
@@ -1788,20 +1772,18 @@ static void handle_incoming_and_exit(void)
 			if (access("/cgi-bin/index.cgi"+1, X_OK) == 0) {
 				purl[0] = '\0';
 				g_query = url;
-				sendCgi("/cgi-bin/index.cgi", prequest, length, cookie, content_type);
-				break;
+				send_cgi_and_exit("/cgi-bin/index.cgi", prequest, length, cookie, content_type);
 			}
 		}
 #endif  /* FEATURE_HTTPD_CGI */
-		sendFile(test);
-		ContentLength = -1;
+		send_file_and_exit(test);
 	} while (0);
 
 #if ENABLE_FEATURE_HTTPD_CGI
  bail_out:
 #endif
 
-	exit(0);
+	_exit(0);
 
 #if 0 /* Is this needed? Why? */
 	if (DEBUG)
@@ -1830,7 +1812,7 @@ static void handle_incoming_and_exit(void)
 	} while (retval > 0 && read(accepted_socket, iobuf, sizeof(iobuf) > 0));
 	shutdown(accepted_socket, SHUT_RD);
 	close(accepted_socket);
-	exit(0);
+	_exit(0);
 #endif
 }
 
@@ -1844,6 +1826,7 @@ static void handle_incoming_and_exit(void)
 static void mini_httpd(int server) ATTRIBUTE_NORETURN;
 static void mini_httpd(int server)
 {
+// TODO: use accept WITHOUT select, it will just block there
 	fd_set readfd, portfd;
 
 	FD_ZERO(&portfd);
@@ -1857,9 +1840,10 @@ static void mini_httpd(int server)
 			struct sockaddr_in sin;
 			USE_FEATURE_IPV6(struct sockaddr_in6 sin6;)
 		} fromAddr;
+// TODO: this looks like lsa to me
 		socklen_t fromAddrLen = sizeof(fromAddr);
 
-		/* Now wait INDEFINITELY on the set of sockets! */
+		/* Now wait INDEFINITELY on the set of sockets */
 		readfd = portfd;
 		if (select(server + 1, &readfd, 0, 0, 0) <= 0)
 			continue;
@@ -1871,7 +1855,7 @@ static void mini_httpd(int server)
 		accepted_socket = s;
 		rmt_ip = 0;
 		tcp_port = 0;
-		if (verbose || ENABLE_FEATURE_HTTPD_CGI || DEBUG) {
+		if (ENABLE_FEATURE_HTTPD_CGI || DEBUG || verbose) {
 			free(rmt_ip_str);
 			rmt_ip_str = xmalloc_sockaddr2dotted(&fromAddr.sa, fromAddrLen);
 			if (DEBUG)
@@ -1879,6 +1863,7 @@ static void mini_httpd(int server)
 		}
 		if (fromAddr.sa.sa_family == AF_INET) {
 			rmt_ip = ntohl(fromAddr.sin.sin_addr.s_addr);
+// TODO: use get_nport?
 			tcp_port = ntohs(fromAddr.sin.sin_port);
 		}
 #if ENABLE_FEATURE_IPV6
@@ -1897,11 +1882,15 @@ static void mini_httpd(int server)
 			/* Do not reload config on HUP */
 			signal(SIGHUP, SIG_IGN);
 #endif
+			if (verbose) {
+				/* this trick makes -v logging much simpler */
+				applet_name = rmt_ip_str;
+			}
 			handle_incoming_and_exit();
 		}
 		close(s);
 	} /* while (1) */
-	/* return 0; - never reached */
+	/* never reached */
 }
 #endif
 
@@ -1914,6 +1903,7 @@ static void mini_httpd_inetd(void)
 		struct sockaddr_in sin;
 		USE_FEATURE_IPV6(struct sockaddr_in6 sin6;)
 	} fromAddr;
+// TODO: this looks like lsa to me
 	socklen_t fromAddrLen = sizeof(fromAddr);
 
 	getpeername(0, &fromAddr.sa, &fromAddrLen);