Fix known "concurrent c_str()s" violations of SBuf API.

The second c_str() call destroys the buffer still being used by the
first c_str() result, leading to many "Invalid read of size N" errors.

IMO, we must instead fix SBuf to make similar violations unlikely, but
there is currently no squid-dev consensus on whether and how to do that.
See "[RFC] Support concurrent SBuf::c_str() calls" thread on squid-dev.

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index a94b61f117241d97dd6acf13bf1f2b12da55e44e..d6cfc7eb626c85df3c20b45bb82825569e620db2 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -1787,8 +1787,9 @@ ClientHttpRequest::doCallouts()
 
     if (calloutContext->error) {
         // XXX: prformance regression. c_str() reallocates
-        SBuf storeUri(request->storeId());
-        StoreEntry *e = storeCreateEntry(storeUri.c_str(), storeUri.c_str(), request->flags, request->method);
+        SBuf storeUriBuf(request->storeId());
+        const char *storeUri = storeUriBuf.c_str();
+        StoreEntry *e = storeCreateEntry(storeUri, storeUri, request->flags, request->method);
 #if USE_OPENSSL
         if (sslBumpNeeded()) {
             // We have to serve an error, so bump the client first.

diff --git a/src/ssl/ServerBump.cc b/src/ssl/ServerBump.cc
index c16dbdb8d9ecb7ae6d2b1cebaf3c1a0d9519df73..5de903eae1349ab2ec4f2926f4ff6245e0a7745d 100644 (file)
--- a/src/ssl/ServerBump.cc
+++ b/src/ssl/ServerBump.cc
@@ -33,8 +33,9 @@ Ssl::ServerBump::ServerBump(HttpRequest *fakeRequest, StoreEntry *e, Ssl::BumpMo
         entry->lock("Ssl::ServerBump");
     } else {
         // XXX: Performance regression. c_str() reallocates
-        SBuf uri(request->effectiveRequestUri());
-        entry = storeCreateEntry(uri.c_str(), uri.c_str(), request->flags, request->method);
+        SBuf uriBuf(request->effectiveRequestUri());
+        const char *uri = uriBuf.c_str();
+        entry = storeCreateEntry(uri, uri, request->flags, request->method);
     }
     // We do not need to be a client because the error contents will be used
     // later, but an entry without any client will trim all its contents away.