Fix (properly, this time), #1688940. Use only sNprintf and strNcpy

SRC/http.c

@@ -12,21 +12,25 @@ char           *
 make_http_sendline(char *url, char *host, int port, int nocache)
 {
 	short           sport = (short) port;
-	int             size = 255;	/* Enough? RFC 2616, section 3.2.1 says it
+	int             size = 350;	/* Enough? RFC 2616, section 3.2.1 says 255
-					 * should work, although there is no hard
+					 * should be enough, although there is no
-					 * limit. */
+					 * hard limit. We reserve more because there 
+					 * * are the protocol elements, the HTTP
+					 * headers, etc */
 	char           *sendline = (char *) malloc(size);
 	char           *hostname = (char *) malloc(size);
 	char           *cache_directive = "";
+	int             result;
 #ifdef HTTP10
 	if (nocache)
 		cache_directive = "Pragma: no-cache\r\n";	/* RFC 1945,
 								 * "Hypertext
 								 * Transfer Protocol 
-								 * * -- HTTP/1.0" */
+								 * * * * --
-	sprintf(sendline,
+								 * HTTP/1.0" */
-		"GET %s HTTP/1.0\r\nUser-Agent: Echoping/%s\r\n%s\r\n",
+	result = snprintf(sendline, size,
-		url, VERSION, cache_directive);
+			  "GET %s HTTP/1.0\r\nUser-Agent: Echoping/%s\r\n%s\r\n",
+			  url, VERSION, cache_directive);
 #else
 	if (nocache) {
 		if (nocache == 1)
@@ -48,19 +52,16 @@ make_http_sendline(char *url, char *host, int port, int nocache)
 	}
 	strncpy(hostname, HTParse(url, "", PARSE_HOST), size);	/* See bug #1688940
 								 * to see why we use 
-								 * strNcpy . If the
+								 * * strNcpy. */
-								 * URL includes no
-								 * host name *and*
-								 * is very long, the 
-								 * hostname buffer
-								 * overflows. */
 	if (!strcmp(hostname, ""))
-		sprintf(hostname, "%s:%d", host, sport);
+		snprintf(hostname, size, "%s:%d", host, sport);
-	sprintf(sendline,
+	result = snprintf(sendline, size,
-		"GET %s HTTP/1.1\r\nUser-Agent: Echoping/%s\r\nHost: %s\r\nConnection: close\r\n%s\r\n",
+			  "GET %s HTTP/1.1\r\nUser-Agent: Echoping/%s\r\nHost: %s\r\nConnection: close\r\n%s\r\n",
-		url, VERSION, hostname, cache_directive);
+			  url, VERSION, hostname, cache_directive);
 	free(hostname);
 #endif
+	if (result >= size)
+		err_quit("URL and/or hostname too long(s)");
 	return sendline;
 }
 
@@ -148,7 +149,7 @@ read_from_server(CHANNEL fs, short ssl, boolean accept_redirects)
 	 */
 	if ((nr < 2) && (timeout_flag))	/* Probably a timeout */
 		return -1;
-	if (nr < 2)		/* Hmm, if the body is empty, we'll get a * *
+	if (nr < 2)		/* Hmm, if the body is empty, we'll get a * * * *
 				 * meaningless error message */
 		err_sys("Error reading HTTP body");
 	total = total + nr;

SRC/misc/test-long-urls

@@ -0,0 +1,11 @@
+#!/bin/sh
+
+# Bug 1688940
+
+# OK
+./echoping -v -h "/?query=0123456789656565864854129568977808708770878781672766762766742213542786502345617812784576590234567890123456784455644855856556697867566473432623422345678901234567890123478901234567565719787867155665376556472234516542568425446852434177664277876766" www.james.rcpt.to 
+
+echo ""
+
+# Too long
+./echoping -v -h "/?query=01234567896565658648541295689778087087708787816727667627667422135427865023456178127845765901234567890123456784455644855856556697867566473432623422345678901234567890123478901234567565719787867155665376556472234516542568425446852434177664277876766" www.james.rcpt.to