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 ac53ee1223cc730ee13e48c4a81a5ab46cbfeb9b..e1b3b584dc401a704cc614924e38f8ed2bdc5453 100644 (file)
--- a/src/HttpHeader.cc
+++ b/src/HttpHeader.cc
@@ -25,6 +25,7 @@
 #include "mime_header.h"
 #include "profiler/Profiler.h"
 #include "rfc1123.h"
+#include "sbuf/StringConvert.h"
 #include "SquidConfig.h"
 #include "StatHist.h"
 #include "Store.h"
@@ -1005,6 +1006,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 0a06802ad7b38c3a5a84898f6462d4b2ff253766..6947c728be7653d2e751dc030f8f9a1ebedc49cd 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 */
@@ -115,6 +116,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 f74b0d5980761c13a52d5d44ae1a99803dafaa53..909020ccd38116beb6f88e55491d3c89ef39fdbe 100644 (file)
--- a/src/SquidString.h
+++ b/src/SquidString.h
@@ -47,6 +47,11 @@ public:
         return buf_[aPos];
     }
 
+    /// 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_; }
+
     size_type size() const { return len_; }
 
     /// variant of size() suited to be used for printf-alikes.

diff --git a/src/client_side_reply.cc b/src/client_side_reply.cc
index 4d824fd04724adc314a86ac61d26c3c91a9d4e19..459a177f2a41df736609ddff15ad3425c9137b8b 100644 (file)
--- a/src/client_side_reply.cc
+++ b/src/client_side_reply.cc
@@ -1595,19 +1595,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 10273c0d13d8b4b315f5f7b9003ffd5b2bd3a30e..d3a351764056cb83f44ea3456ffde94478c32795 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 */