Fix appending Http::HdrType::VIA code duplication

... resolving Via header truncation at 1024 bytes.

Also fixes the generated Via values for non-HTTP protocols.

diff --git a/src/HttpHeader.cc b/src/HttpHeader.cc
index 1733ab6490134b43d31d8e8ee677d8e8e976af3b..d22c3fe775baa61ba4dfa5da5e72a6d032875d23 100644 (file)
--- a/src/HttpHeader.cc
+++ b/src/HttpHeader.cc
@@ -24,6 +24,7 @@
 #include "mgr/Registration.h"
 #include "profiler/Profiler.h"
 #include "rfc1123.h"
+#include "sbuf/StringConvert.h"
 #include "SquidConfig.h"
 #include "StatHist.h"
 #include "Store.h"
@@ -956,6 +957,32 @@ HttpHeader::has(Http::HdrType id) const
     return CBIT_TEST(mask, id);
 }
 
+void
+HttpHeader::addVia(const AnyP::ProtocolVersion &ver, const HttpHeader *from)
+{
+    // TODO: do not add Via header for messages where Squid itself
+    // generated the message (i.e., Downloader or ESI) there should be no Via header added at all.
+
+    if (Config.onoff.via) {
+        SBuf buf;
+        // RFC 7230 section 5.7.1.: protocol-name is omitted when
+        // the received protocol is HTTP.
+        if (ver.protocol > AnyP::PROTO_NONE && ver.protocol < AnyP::PROTO_UNKNOWN &&
+                ver.protocol != AnyP::PROTO_HTTP && ver.protocol != AnyP::PROTO_HTTPS)
+            buf.appendf("%s/", AnyP::ProtocolType_str[ver.protocol]);
+        buf.appendf("%d.%d %s", ver.major, ver.minor, ThisCache);
+        const HttpHeader *hdr = from ? from : this;
+        SBuf strVia = StringToSBuf(hdr->getList(Http::HdrType::VIA));
+        if (!strVia.isEmpty())
+            strVia.append(", ", 2);
+        strVia.append(buf);
+        // XXX: putStr() still suffers from String size limits
+        Must(strVia.length() < String::SizeMaxXXX());
+        delById(Http::HdrType::VIA);
+        putStr(Http::HdrType::VIA, strVia.c_str());
+    }
+}
+
 void
 HttpHeader::putInt(Http::HdrType id, int number)
 {

diff --git a/src/HttpHeader.h b/src/HttpHeader.h
index 4af5498270a160051536429b2d9a3e2288b8a0a9..0f3dd93192d2aff8cd9fcc3cdbb5788f0c3387f1 100644 (file)
--- a/src/HttpHeader.h
+++ b/src/HttpHeader.h
@@ -9,6 +9,7 @@
 #ifndef SQUID_HTTPHEADER_H
 #define SQUID_HTTPHEADER_H
 
+#include "anyp/ProtocolVersion.h"
 #include "base/LookupTable.h"
 #include "http/RegisteredHeaders.h"
 /* because we pass a spec by value */
@@ -108,6 +109,9 @@ public:
     String getByNameListMember(const char *name, const char *member, const char separator) const;
     String getListMember(Http::HdrType id, const char *member, const char separator) const;
     int has(Http::HdrType id) const;
+    /// Appends "this cache" information to VIA header field.
+    /// Takes the initial VIA value from "from" parameter, if provided.
+    void addVia(const AnyP::ProtocolVersion &ver, const HttpHeader *from = 0);
     void putInt(Http::HdrType id, int number);
     void putInt64(Http::HdrType id, int64_t number);
     void putTime(Http::HdrType id, time_t htime);

diff --git a/src/SquidString.h b/src/SquidString.h
index bbe0c74bf4e8c99383e7f13c9088036be7336844..6f3a816da77bd381ca4946c884fd04d0a963f2c1 100644 (file)
--- a/src/SquidString.h
+++ b/src/SquidString.h
@@ -42,6 +42,11 @@ public:
      */
     _SQUID_INLINE_ char operator [](unsigned int pos) const;
 
+    /// The absolute size limit on data held in a String.
+    /// Since Strings can be nil-terminated implicitly it is best to ensure
+    /// the useful content length is strictly less than this limit.
+    static const size_type SizeMaxXXX() { return SizeMax_; }
+
     _SQUID_INLINE_ size_type size() const;
     /// variant of size() suited to be used for printf-alikes.
     /// throws when size() > MAXINT

diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index ca7e99d91dfa2dd5725cad632ce05bdd03be8c59..146ea1c3e16075d227bdbeaa903831ee8eca914d 100644 (file)
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -1594,19 +1594,8 @@ clientReplyContext::buildReplyHeader()
         hdr->putStr(Http::HdrType::TRANSFER_ENCODING, "chunked");
     }
 
-    /* Append VIA */
-    if (Config.onoff.via) {
-        LOCAL_ARRAY(char, bbuf, MAX_URL + 32);
-        String strVia;
-        hdr->getList(Http::HdrType::VIA, &strVia);
-        snprintf(bbuf, MAX_URL + 32, "%d.%d %s",
-                 reply->sline.version.major,
-                 reply->sline.version.minor,
-                 ThisCache);
-        strListAdd(&strVia, bbuf, ',');
-        hdr->delById(Http::HdrType::VIA);
-        hdr->putStr(Http::HdrType::VIA, strVia.termedBuf());
-    }
+    hdr->addVia(reply->sline.version);
+
     /* Signal keep-alive or close explicitly */
     hdr->putStr(Http::HdrType::CONNECTION, request->flags.proxyKeepalive ? "keep-alive" : "close");
 

diff --git a/src/http.cc b/src/http.cc
index 4600df0c562e9ec2b3adfafd0919be2dca07fd42..8f026c5e7fabfb1306597a92204339c8d0243eb9 100644 (file)
--- a/src/http.cc
+++ b/src/http.cc
@@ -1795,17 +1795,7 @@ HttpStateData::httpBuildRequestHeader(HttpRequest * request,
         request->flags.isRanged = false;
     }
 
-    /* append Via */
-    if (Config.onoff.via) {
-        String strVia;
-        strVia = hdr_in->getList(Http::HdrType::VIA);
-        snprintf(bbuf, BBUF_SZ, "%d.%d %s",
-                 request->http_ver.major,
-                 request->http_ver.minor, ThisCache);
-        strListAdd(&strVia, bbuf, ',');
-        hdr_out->putStr(Http::HdrType::VIA, strVia.termedBuf());
-        strVia.clean();
-    }
+    hdr_out->addVia(request->http_ver, hdr_in);
 
     if (request->flags.accelerated) {
         /* Append Surrogate-Capabilities */