Bug 4671 pt3: 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/clients/FtpGateway.cc b/src/clients/FtpGateway.cc
index da0d5d2a09014736870506f8d08fe96074cd6afa..aa4d97aa87c7025230a81562b6fe52be586ca063 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();
@@ -1162,7 +1162,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;
@@ -1263,7 +1264,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....
@@ -1271,18 +1274,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->url.port() == 21) {
-        snprintf(realm, 8192, "FTP %s %s", user, request->url.host());
-    } else {
-        snprintf(realm, 8192, "FTP %s %s port %d", user, request->url.host(), request->url.port());
+    realm.appendf("FTP %s ", user);
+    if (!request)
+        realm.append("unknown", 7);
+    else {
+        realm.append(request->url.host());
+        if (request->url.port() != 21)
+            realm.appendf(" port %d", request->url.port());
     }
     return realm;
 }
@@ -2642,13 +2646,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;
 }