Bug 4671 pt3: various GCC 7 compile errors

Also, remove limit on FTP realm strings

Convert ftpRealm() from generating char* to SBuf. This fixes issues identified
by GCC 7 where the realm string may be longer than the available buffer and
gets truncated.
The size of the buffer was making the occurance rather rare, but it is still
possible.

diff --git a/src/DiskIO/DiskThreads/aiops.cc b/src/DiskIO/DiskThreads/aiops.cc
index b44adfa8e23868b3d4079e984ca4d071c0d67958..e11f94871eb531a3b649146e1bb8f375f8a84177 100644 (file)
--- a/src/DiskIO/DiskThreads/aiops.cc
+++ b/src/DiskIO/DiskThreads/aiops.cc
@@ -290,7 +290,7 @@ squidaio_init(void)
     /* Create threads and get them to sit in their wait loop */
     squidaio_thread_pool = memPoolCreate("aio_thread", sizeof(squidaio_thread_t));
 
-    assert(NUMTHREADS);
+    assert(NUMTHREADS != 0);
 
     for (i = 0; i < NUMTHREADS; ++i) {
         threadp = (squidaio_thread_t *)squidaio_thread_pool->alloc();

diff --git a/src/clients/FtpGateway.cc b/src/clients/FtpGateway.cc
index cd27d86582f1fa0f0f23df6e508460b157d9df4e..628fb7e62cbe29a118e0ce427b893d409b776843 100644 (file)
--- a/src/clients/FtpGateway.cc
+++ b/src/clients/FtpGateway.cc
@@ -153,8 +153,8 @@ public:
     virtual void timeout(const CommTimeoutCbParams &io);
     void ftpAcceptDataConnection(const CommAcceptCbParams &io);
 
-    static HttpReply *ftpAuthRequired(HttpRequest * request, const char *realm);
-    const char *ftpRealm(void);
+    static HttpReply *ftpAuthRequired(HttpRequest * request, SBuf &realm);
+    SBuf ftpRealm();
     void loginFailed(void);
 
     virtual void haveParsedReplyHeaders();
@@ -1189,7 +1189,8 @@ Ftp::Gateway::start()
 {
     if (!checkAuth(&request->header)) {
         /* create appropriate reply */
-        HttpReply *reply = ftpAuthRequired(request, ftpRealm());
+        SBuf realm(ftpRealm()); // local copy so SBuf wont disappear too early
+        HttpReply *reply = ftpAuthRequired(request, realm);
         entry->replaceHttpReply(reply);
         serverComplete();
         return;
@@ -1290,7 +1291,9 @@ Ftp::Gateway::loginFailed()
 
 #if HAVE_AUTH_MODULE_BASIC
     /* add Authenticate header */
-    newrep->header.putAuth("Basic", ftpRealm());
+    // XXX: performance regression. c_str() may reallocate
+    SBuf realm(ftpRealm()); // local copy so SBuf wont disappear too early
+    newrep->header.putAuth("Basic", realm.c_str());
 #endif
 
     // add it to the store entry for response....
@@ -1298,18 +1301,19 @@ Ftp::Gateway::loginFailed()
     serverComplete();
 }
 
-const char *
+SBuf
 Ftp::Gateway::ftpRealm()
 {
-    static char realm[8192];
+    SBuf realm;
 
     /* This request is not fully authenticated */
-    if (!request) {
-        snprintf(realm, 8192, "FTP %s unknown", user);
-    } else if (request->port == 21) {
-        snprintf(realm, 8192, "FTP %s %s", user, request->GetHost());
-    } else {
-        snprintf(realm, 8192, "FTP %s %s port %d", user, request->GetHost(), request->port);
+    realm.appendf("FTP %s ", user);
+    if (!request)
+        realm.append("unknown", 7);
+    else {
+        realm.append(request->GetHost());
+        if (request->port != 21)
+            realm.appendf(" port %d", request->port);
     }
     return realm;
 }
@@ -2673,13 +2677,14 @@ Ftp::Gateway::haveParsedReplyHeaders()
 }
 
 HttpReply *
-Ftp::Gateway::ftpAuthRequired(HttpRequest * request, const char *realm)
+Ftp::Gateway::ftpAuthRequired(HttpRequest * request, SBuf &realm)
 {
     ErrorState err(ERR_CACHE_ACCESS_DENIED, Http::scUnauthorized, request);
     HttpReply *newrep = err.BuildHttpReply();
 #if HAVE_AUTH_MODULE_BASIC
     /* add Authenticate header */
-    newrep->header.putAuth("Basic", realm);
+    // XXX: performance regression. c_str() may reallocate
+    newrep->header.putAuth("Basic", realm.c_str());
 #endif
     return newrep;
 }

diff --git a/src/fde.cc b/src/fde.cc
index 6c6633b51435ab5c95707dba3842b756d827436b..4d1989b71324b391c029ea56b2d8333708ceb91b 100644 (file)
--- a/src/fde.cc
+++ b/src/fde.cc
@@ -85,15 +85,15 @@ fde::DumpStats (StoreEntry *dumpEntry)
 char const *
 fde::remoteAddr() const
 {
-    LOCAL_ARRAY(char, buf, MAX_IPSTRLEN );
+    static char buf[MAX_IPSTRLEN+7]; // 7 = length of ':port' strings
 
     if (type != FD_SOCKET)
         return null_string;
 
     if ( *ipaddr )
-        snprintf( buf, MAX_IPSTRLEN, "%s:%d", ipaddr, (int)remote_port);
+        snprintf(buf, sizeof(buf), "%s:%u", ipaddr, remote_port);
     else
-        local_addr.toUrl(buf,MAX_IPSTRLEN); // toHostStr does not include port.
+        local_addr.toUrl(buf, sizeof(buf)); // toHostStr does not include port.
 
     return buf;
 }

diff --git a/src/fs/ufs/RebuildState.cc b/src/fs/ufs/RebuildState.cc
index 679c4e15dbd0afa4d30eec4a36853db1584f921e..a4c06aa67216e0f48e28ecaf63bbbaad83697529 100644 (file)
--- a/src/fs/ufs/RebuildState.cc
+++ b/src/fs/ufs/RebuildState.cc
@@ -444,7 +444,7 @@ Fs::Ufs::RebuildState::getNextFile(sfileno * filn_p, int *size)
         }
 
         if (0 == in_dir) {  /* we need to read in a new directory */
-            snprintf(fullpath, MAXPATHLEN, "%s/%02X/%02X",
+            snprintf(fullpath, sizeof(fullpath), "%s/%02X/%02X",
                      sd->path,
                      curlvl1, curlvl2);
 
@@ -489,7 +489,7 @@ Fs::Ufs::RebuildState::getNextFile(sfileno * filn_p, int *size)
                 continue;
             }
 
-            snprintf(fullfilename, MAXPATHLEN, "%s/%s",
+            snprintf(fullfilename, sizeof(fullfilename), "%s/%s",
                      fullpath, entry->d_name);
             debugs(47, 3, HERE << "Opening " << fullfilename);
             fd = file_open(fullfilename, O_RDONLY | O_BINARY);

diff --git a/src/fs/ufs/RebuildState.h b/src/fs/ufs/RebuildState.h
index 42b1bee4b3e41aa3890f280b2433e5f523d779ee..44c3830ac3534787e9486cde9938a28f74c0c3d1 100644 (file)
--- a/src/fs/ufs/RebuildState.h
+++ b/src/fs/ufs/RebuildState.h
@@ -54,7 +54,7 @@ public:
     dirent_t *entry;
     DIR *td;
     char fullpath[MAXPATHLEN];
-    char fullfilename[MAXPATHLEN];
+    char fullfilename[MAXPATHLEN*2];
 
     StoreRebuildData counts;
 

diff --git a/src/gopher.cc b/src/gopher.cc
index 03b69a228677b0bbc6cb18231d0d8f6d50bf3b86..6588dde5d7f8a4173f12ca758c55d44a456c0697 100644 (file)
--- a/src/gopher.cc
+++ b/src/gopher.cc
@@ -820,7 +820,7 @@ gopherReadReply(const Comm::ConnectionPointer &conn, char *buf, size_t len, Comm
  * This will be called when request write is complete. Schedule read of reply.
  */
 static void
-gopherSendComplete(const Comm::ConnectionPointer &conn, char *buf, size_t size, Comm::Flag errflag, int xerrno, void *data)
+gopherSendComplete(const Comm::ConnectionPointer &conn, char *, size_t size, Comm::Flag errflag, int xerrno, void *data)
 {
     GopherStateData *gopherState = (GopherStateData *) data;
     StoreEntry *entry = gopherState->entry;
@@ -840,10 +840,6 @@ gopherSendComplete(const Comm::ConnectionPointer &conn, char *buf, size_t size,
         err->url = xstrdup(entry->url());
         gopherState->fwd->fail(err);
         gopherState->serverConn->close();
-
-        if (buf)
-            memFree(buf, MEM_4K_BUF);   /* Allocated by gopherSendRequest. */
-
         return;
     }
 
@@ -885,9 +881,6 @@ gopherSendComplete(const Comm::ConnectionPointer &conn, char *buf, size_t size,
     AsyncCall::Pointer call =  commCbCall(5,5, "gopherReadReply",
                                           CommIoCbPtrFun(gopherReadReply, gopherState));
     entry->delayAwareRead(conn, gopherState->replybuf, BUFSIZ, call);
-
-    if (buf)
-        memFree(buf, MEM_4K_BUF);   /* Allocated by gopherSendRequest. */
 }
 
 /**
@@ -898,32 +891,31 @@ static void
 gopherSendRequest(int fd, void *data)
 {
     GopherStateData *gopherState = (GopherStateData *)data;
-    char *buf = (char *)memAllocate(MEM_4K_BUF);
+    MemBuf mb;
+    mb.init();
 
     if (gopherState->type_id == GOPHER_CSO) {
         const char *t = strchr(gopherState->request, '?');
 
-        if (t != NULL)
+        if (t)
             ++t;        /* skip the ? */
         else
             t = "";
 
-        snprintf(buf, 4096, "query %s\r\nquit\r\n", t);
-    } else if (gopherState->type_id == GOPHER_INDEX) {
-        char *t = strchr(gopherState->request, '?');
-
-        if (t != NULL)
-            *t = '\t';
-
-        snprintf(buf, 4096, "%s\r\n", gopherState->request);
+        mb.Printf("query %s\r\nquit", t);
     } else {
-        snprintf(buf, 4096, "%s\r\n", gopherState->request);
+        if (gopherState->type_id == GOPHER_INDEX) {
+            if (char *t = strchr(gopherState->request, '?'))
+                *t = '\t';
+        }
+        mb.append(gopherState->request, strlen(gopherState->request));
     }
+    mb.append("\r\n", 2);
 
-    debugs(10, 5, HERE << gopherState->serverConn);
+    debugs(10, 5, gopherState->serverConn);
     AsyncCall::Pointer call = commCbCall(5,5, "gopherSendComplete",
                                          CommIoCbPtrFun(gopherSendComplete, gopherState));
-    Comm::Write(gopherState->serverConn, buf, strlen(buf), call, NULL);
+    Comm::Write(gopherState->serverConn, &mb, call);
 
     gopherState->entry->makePublic();
 }

diff --git a/src/icmp/Makefile.am b/src/icmp/Makefile.am
index a8faa29e9d07f9a430dbecf2b71687b9e000a4a0..fa1a0b96e342979bb0a3fc6183dac36888856cf3 100644 (file)
--- a/src/icmp/Makefile.am
+++ b/src/icmp/Makefile.am
@@ -59,7 +59,8 @@ nodist_pinger_SOURCES = $(COPIED_SOURCE)
 pinger_LDFLAGS = $(LIBADD_DL)
 pinger_LDADD=\
        libicmp-core.la \
-       ../ip/libip.la \
+       $(top_builddir)/src/ip/libip.la \
+       $(top_builddir)/src/base/libbase.la \
        $(COMPAT_LIB) \
        $(XTRA_LIBS)