From ffabfba815b1d2d3dfab2f96dbfedd1f8179c2da Mon Sep 17 00:00:00 2001 From: Stephane Bortzmeyer Date: Thu, 29 Mar 2007 20:19:55 +0000 Subject: [PATCH] Fix (properly, this time), #1688940. Use only sNprintf and strNcpy --- SRC/http.c | 37 +++++++++++++++++++------------------ SRC/misc/test-long-urls | 11 +++++++++++ 2 files changed, 30 insertions(+), 18 deletions(-) create mode 100755 SRC/misc/test-long-urls diff --git a/SRC/http.c b/SRC/http.c index 81c3e3d..0559f7b 100644 --- a/SRC/http.c +++ b/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 - * should work, although there is no hard - * limit. */ + int size = 350; /* Enough? RFC 2616, section 3.2.1 says 255 + * should be enough, although there is no + * 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, - "GET %s HTTP/1.0\r\nUser-Agent: Echoping/%s\r\n%s\r\n", - url, VERSION, cache_directive); + * * * * -- + * HTTP/1.0" */ + result = snprintf(sendline, size, + "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 - * URL includes no - * host name *and* - * is very long, the - * hostname buffer - * overflows. */ + * * strNcpy. */ if (!strcmp(hostname, "")) - sprintf(hostname, "%s:%d", host, sport); - sprintf(sendline, - "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); + snprintf(hostname, size, "%s:%d", host, sport); + 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", + 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; diff --git a/SRC/misc/test-long-urls b/SRC/misc/test-long-urls new file mode 100755 index 0000000..187ac2c --- /dev/null +++ b/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