Centralized and documented FTP protocol "version" (1.1) definition.

Moved AnyP::PortCfg::setTransport() to the cache_cf.cc parsing code where it
currently belongs. AnyP code should not have protocol-specific pieces as it is
code shared among (or used by) many (ideally, all transfer) protocols.

TODO: Convert Http::ProtocolVersion from class into a function.

diff --git a/src/anyp/PortCfg.cc b/src/anyp/PortCfg.cc
index 76a5fec9fe72a85c77c0a2a3d0d44caa17d769ea..23a13028dcb27fce94fd04575634ca001a59d6dc 100644 (file)
--- a/src/anyp/PortCfg.cc
+++ b/src/anyp/PortCfg.cc
@@ -186,22 +186,3 @@ AnyP::PortCfg::configureSslServerContext()
     }
 }
 #endif
-
-void
-AnyP::PortCfg::setTransport(const char *aProtocol)
-{
-    // HTTP/1.0 not supported because we are version 1.1 which contains a superset of 1.0
-    // and RFC 2616 requires us to upgrade 1.0 to 1.1
-
-    if (strcasecmp("http", aProtocol) == 0 || strcmp("HTTP/1.1", aProtocol) == 0)
-        transport = AnyP::ProtocolVersion(AnyP::PROTO_HTTP, 1,1);
-
-    else if (strcasecmp("https", aProtocol) == 0 || strcmp("HTTPS/1.1", aProtocol) == 0)
-        transport = AnyP::ProtocolVersion(AnyP::PROTO_HTTPS, 1,1);
-
-    else if (strcasecmp("ftp", aProtocol) == 0)
-        transport = AnyP::ProtocolVersion(AnyP::PROTO_FTP, 1,1);
-
-    else
-        fatalf("http(s)_port protocol=%s is not supported\n", aProtocol);
-}

diff --git a/src/anyp/PortCfg.h b/src/anyp/PortCfg.h
index bbabd18ad5582abfc1a6ffcb4d0fe285d9b87223..7f3440133c4306ac7ba58c97bd5627f892af809b 100644 (file)
--- a/src/anyp/PortCfg.h
+++ b/src/anyp/PortCfg.h
@@ -24,13 +24,6 @@ public:
     void configureSslServerContext();
 #endif
 
-    /**
-     * Set this ports transport type from a string representation.
-     * Unknown transport type representations will halt Squid.
-     * Supports: HTTP, HTTP/1.1, HTTPS, HTTPS/1.1, and FTP.
-     */
-    void setTransport(const char *aProtocol);
-
     PortCfgPointer next;
 
     Ip::Address s;

diff --git a/src/cache_cf.cc b/src/cache_cf.cc
index 17835cf12c6a5c642d0b8a003a1160a4700c3934..988b8785516a7ed7d25608c0451f31aaa87d825a 100644 (file)
--- a/src/cache_cf.cc
+++ b/src/cache_cf.cc
@@ -52,6 +52,7 @@
 #include "eui/Config.h"
 #include "ExternalACL.h"
 #include "format/Format.h"
+#include "ftp/Elements.h"
 #include "globals.h"
 #include "HttpHeaderTools.h"
 #include "HttpRequestMethod.h"
@@ -3566,6 +3567,27 @@ parsePortSpecification(const AnyP::PortCfgPointer &s, char *token)
     }
 }
 
+/// parses the protocol= option of the *_port directive, returning parsed value
+/// unsupported option values result in a fatal error message
+static AnyP::ProtocolVersion
+parsePortProtocol(const char *value)
+{
+    // HTTP/1.0 not supported because we are version 1.1 which contains a superset of 1.0
+    // and RFC 2616 requires us to upgrade 1.0 to 1.1
+    if (strcasecmp("http", value) == 0 || strcmp("HTTP/1.1", value) == 0)
+        return AnyP::ProtocolVersion(AnyP::PROTO_HTTP, 1,1);
+
+    if (strcasecmp("https", value) == 0 || strcmp("HTTPS/1.1", value) == 0)
+        return AnyP::ProtocolVersion(AnyP::PROTO_HTTPS, 1,1);
+
+    if (strcasecmp("ftp", value) == 0)
+        return AnyP::ProtocolVersion(AnyP::PROTO_FTP,
+            Ftp::ProtocolVersion().major, Ftp::ProtocolVersion().minor);
+
+    fatalf("%s directive does not support protocol=%s\n", cfg_directive, value);
+    return AnyP::ProtocolVersion(); // not reached
+}
+
 static void
 parse_port_option(AnyP::PortCfgPointer &s, char *token)
 {
@@ -3638,7 +3660,7 @@ parse_port_option(AnyP::PortCfgPointer &s, char *token)
             debugs(3, DBG_CRITICAL, "FATAL: http(s)_port: protocol option requires Acceleration mode flag.");
             self_destruct();
         }
-        s->setTransport(token + 9);
+        s->transport = parsePortProtocol(token + 9);
     } else if (strcmp(token, "allow-direct") == 0) {
         if (!s->flags.accelSurrogate) {
             debugs(3, DBG_CRITICAL, "FATAL: http(s)_port: allow-direct option requires Acceleration mode flag.");
@@ -3766,7 +3788,7 @@ void
 add_http_port(char *portspec)
 {
     AnyP::PortCfgPointer s = new AnyP::PortCfg();
-    s->setTransport("HTTP");
+    s->transport = parsePortProtocol("HTTP");
     parsePortSpecification(s, portspec);
     // we may need to merge better if the above returns a list with clones
     assert(s->next == NULL);
@@ -3798,7 +3820,7 @@ parsePortCfg(AnyP::PortCfgPointer *head, const char *optionName)
     }
 
     AnyP::PortCfgPointer s = new AnyP::PortCfg();
-    s->setTransport(protocol);
+    s->transport = parsePortProtocol(protocol); // default; protocol=... overwrites
     parsePortSpecification(s, token);
 
     /* parse options ... */

diff --git a/src/ftp/Elements.cc b/src/ftp/Elements.cc
index 8570d99ceec7a1124dcb82584b258956f7aaa720..0921108bb8cc09a132ec8c0286603c59aae1106c 100644 (file)
--- a/src/ftp/Elements.cc
+++ b/src/ftp/Elements.cc
@@ -8,11 +8,27 @@
 #include "HttpReply.h"
 #include "SBuf.h"
 
+// FTP does not have a notion of a "protocol version" but we need something for
+// compatibility with the current HttpMsg wrapping layer. We use version 1.1:
+// * some ICAP services probably expect /1.0 or /1.1 when parsing HTTP headers;
+// * FTP commands are sent on a "persistent by default" connection, just like
+//   HTTP/1.1. Using 1.1 leads to fewer exceptions in current code shared by
+//   HTTP and FTP.
+AnyP::ProtocolVersion
+Ftp::ProtocolVersion()
+{
+    return AnyP::ProtocolVersion(AnyP::PROTO_FTP, 1, 1);
+}
+
 HttpReply *
 Ftp::HttpReplyWrapper(const int ftpStatus, const char *ftpReason, const Http::StatusCode httpStatus, const int64_t clen)
 {
     HttpReply *const reply = new HttpReply;
-    reply->sline.set(Http::ProtocolVersion(1, 1), httpStatus);
+
+    Http::ProtocolVersion httpVersion = Http::ProtocolVersion(
+        Ftp::ProtocolVersion().major, Ftp::ProtocolVersion().minor);
+    reply->sline.set(httpVersion, httpStatus);
+
     HttpHeader &header = reply->header;
     header.putTime(HDR_DATE, squid_curtime);
     {

diff --git a/src/ftp/Elements.h b/src/ftp/Elements.h
index 769de2f4331f7f59b9f51ce8d1950e6e858c3ff0..70f90a803af4f6b4baa4758a741b10683887c66d 100644 (file)
--- a/src/ftp/Elements.h
+++ b/src/ftp/Elements.h
@@ -6,9 +6,17 @@
 class SBuf;
 class HttpReply;
 
+namespace AnyP
+{
+class ProtocolVersion;
+}
+
 namespace Ftp
 {
 
+/// Protocol version to use in HttpMsg structures wrapping FTP messages.
+AnyP::ProtocolVersion ProtocolVersion();
+
 /// Create an internal HttpReply structure to house FTP control response info.
 HttpReply *HttpReplyWrapper(const int ftpStatus, const char *ftpReason, const Http::StatusCode httpStatus, const int64_t clen);
 

diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc
index e17761ab0fa9122a12063fcb568b4367355b685b..0a5100d497b2f78339d1a04da14de0b18314e76d 100644 (file)
--- a/src/servers/FtpServer.cc
+++ b/src/servers/FtpServer.cc
@@ -53,7 +53,6 @@ Ftp::Server::Server(const MasterXaction::Pointer &xact):
         connector(),
         reader()
 {
-    assert(xact->squidPort->transport.protocol == AnyP::PROTO_FTP);
     flags.readMore = false; // we need to announce ourselves first
 }
 
@@ -633,7 +632,7 @@ Ftp::Server::parseOneRequest(Http::ProtocolVersion &ver)
         return NULL;
     }
 
-    ver = Http::ProtocolVersion(1, 1);
+    ver = Http::ProtocolVersion(Ftp::ProtocolVersion().major, Ftp::ProtocolVersion().minor);
     request->flags.ftpNative = true;
     request->http_ver = ver;