From: Eduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Date: Thu, 23 Mar 2017 12:55:36 +0000 (+1200)
Subject: Fix appending Http::HdrType::VIA code duplication
X-Git-Tag: M-staged-PR71~212
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=90be6ff518647d4980ad6b725b1279da082cc7ee;p=thirdparty%2Fsquid.git

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 ac53ee1223..e1b3b584dc 100644
--- 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 0a06802ad7..6947c728be 100644
--- 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 f74b0d5980..909020ccd3 100644
--- 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 4d824fd047..459a177f2a 100644
--- 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 10273c0d13..d3a3517640 100644
--- 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 */