Improve AnyP::Uri::port_ and related port storage types (#1255)

The old "unsigned short" type is used for various things all over the
code and cannot distinguish a parsed port zero from an absent port. In
theory, it could also hold values exceeding 65535, getting out of sync
with various (poorly duplicated) maximum port value checks.

No significant runtime changes are expected, but some transactions with
unknown request URI ports may now be reported correctly, showing "-"
instead of "0" for the missing port number. In some very special/rare
on_unsupported_protocol cases, Squid might no longer attempt to tunnel
non-HTTP requests to a zero port. Such attempts were failing anyway.

This change leaves many "unsigned short" port use cases untouched. Most
of them should be converted, but most (if not all) of those conversions
should be done in dedicated projects. Here are a few TODO highlights:

* ACLIntRange: Stores integers, is usable for integers, but assumes it
  is being configured with port numbers. We probably should enhance that
  class to handle "all" integers instead of expanding/deepening that
  rather limiting "input is just port numbers" assumption.

* CachePeer and internalRemoteUri(): Likely runtime changes related to
  squid.conf validity.

* Ftp::Server::listenForDataConnection() and friends: Triggers a few
  changes with runtime effects related to comm_local_port() failures.

* Ip::Address, Snmp::Session, fde: Too many low-level changes, some of
  which would be difficult to test.

* Config.Port.icp: Possible runtime changes related to squid.conf and
  command-line arguments validity.

Also used "%hu" instead of "%u" or "%d" to printf() ports. These changes
arguably improve code and might make some pedantic compiler happier, but
they do not fix any "real" bugs because variadic printf() arguments are
automatically promoted from short to int and, hence, printf()
implementations never get and never expect shorts.

diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc
index 7cf26b089ef586089560ff8f0db6f10a4d471e0f..7c484fb850d96df4e4133ade51003776bf9a618e 100644 (file)
--- a/src/HttpRequest.cc
+++ b/src/HttpRequest.cc
@@ -872,11 +872,16 @@ FindListeningPortAddress(const HttpRequest *callerRequest, const AccessLogEntry
     });
 }
 
-unsigned short
+AnyP::Port
 FindListeningPortNumber(const HttpRequest *callerRequest, const AccessLogEntry *ale)
 {
     const auto ip = FindGoodListeningPortAddress(callerRequest, ale, [](const Ip::Address &address) {
         return address.port() > 0;
     });
-    return ip ? ip->port() : 0;
+
+    if (!ip)
+        return std::nullopt;
+
+    Assure(ip->port() > 0);
+    return ip->port();
 }

diff --git a/src/HttpRequest.h b/src/HttpRequest.h
index c9fb92d86dd19c3832af5f75811321b17236a032..550f191d5e75a58e92a8f4f3249831ad59b82813 100644 (file)
--- a/src/HttpRequest.h
+++ b/src/HttpRequest.h
@@ -285,9 +285,9 @@ void UpdateRequestNotes(ConnStateData *csd, HttpRequest &request, NotePairs cons
 /// nil parameter(s) indicate missing caller information and are handled safely
 const Ip::Address *FindListeningPortAddress(const HttpRequest *, const AccessLogEntry *);
 
-/// \returns listening/*_port port number used by the client connection (or zero)
+/// \returns listening/*_port port number used by the client connection (or nothing)
 /// nil parameter(s) indicate missing caller information and are handled safely
-unsigned short FindListeningPortNumber(const HttpRequest *, const AccessLogEntry *);
+AnyP::Port FindListeningPortNumber(const HttpRequest *, const AccessLogEntry *);
 
 #endif /* SQUID_HTTPREQUEST_H */
 

diff --git a/src/acl/UrlPort.cc b/src/acl/UrlPort.cc
index 53453ed0bcdbe938d7d0c222e770ec3a79a8cc72..2b01f5a8ec5fbda019dd2d9e5a3ca4edd4df3bc6 100644 (file)
--- a/src/acl/UrlPort.cc
+++ b/src/acl/UrlPort.cc
@@ -14,6 +14,6 @@
 int
 ACLUrlPortStrategy::match(ACLData<MatchType> * &data, ACLFilledChecklist *checklist)
 {
-    return data->match(checklist->request->url.port());
+    return data->match(checklist->request->url.port().value_or(0));
 }
 

diff --git a/src/anyp/Uri.cc b/src/anyp/Uri.cc
index e372939968b0458ff98a27f92b88e0ad5111aa6a..cc8516593e62d1edb22ebc1200b6853c9cf2e332 100644 (file)
--- a/src/anyp/Uri.cc
+++ b/src/anyp/Uri.cc
@@ -362,7 +362,11 @@ AnyP::Uri::parse(const HttpRequestMethod& method, const SBuf &rawUrl)
                 return false;
             *dst = '\0';
 
-            foundPort = scheme.defaultPort(); // may be reset later
+            // If the parsed scheme has no (known) default port, and there is no
+            // explicit port, then we will reject the zero port during foundPort
+            // validation, often resulting in a misleading 400/ERR_INVALID_URL.
+            // TODO: Remove this hack when switching to Tokenizer-based parsing.
+            foundPort = scheme.defaultPort().value_or(0); // may be reset later
 
             /* Is there any login information? (we should eventually parse it above) */
             t = strrchr(foundHost, '@');
@@ -571,10 +575,14 @@ AnyP::Uri::authority(bool requirePort) const
         authorityWithPort_.append(host());
         authorityHttp_ = authorityWithPort_;
 
-        // authorityForm_ only has :port if it is non-default
-        authorityWithPort_.appendf(":%u",port());
-        if (port() != getScheme().defaultPort())
-            authorityHttp_ = authorityWithPort_;
+        if (port().has_value()) {
+            authorityWithPort_.appendf(":%hu", *port());
+            // authorityHttp_ only has :port for known non-default ports
+            if (port() != getScheme().defaultPort())
+                authorityHttp_ = authorityWithPort_;
+        }
+        // else XXX: We made authorityWithPort_ that does not have a port.
+        // TODO: Audit callers and refuse to give out broken authorityWithPort_.
     }
 
     return requirePort ? authorityWithPort_ : authorityHttp_;
@@ -898,8 +906,7 @@ urlCheckRequest(const HttpRequest * r)
 
 AnyP::Uri::Uri(AnyP::UriScheme const &aScheme) :
     scheme_(aScheme),
-    hostIsNumeric_(false),
-    port_(0)
+    hostIsNumeric_(false)
 {
     *host_=0;
 }

diff --git a/src/anyp/Uri.h b/src/anyp/Uri.h
index 01cc1e7498d7c9bbea4d79eda07cc5dc05bb109b..1db3c971f0c20846c7e885d0898061fb953832cb 100644 (file)
--- a/src/anyp/Uri.h
+++ b/src/anyp/Uri.h
@@ -32,7 +32,7 @@ class Uri
     MEMPROXY_CLASS(Uri);
 
 public:
-    Uri() : hostIsNumeric_(false), port_(0) {*host_=0;}
+    Uri(): hostIsNumeric_(false) { *host_ = 0; }
     Uri(AnyP::UriScheme const &aScheme);
     Uri(const Uri &other) {
         this->operator =(other);
@@ -54,7 +54,7 @@ public:
         hostIsNumeric_ = false;
         *host_ = 0;
         hostAddr_.setEmpty();
-        port_ = 0;
+        port_ = std::nullopt;
         touch();
     }
     void touch(); ///< clear the cached URI display forms
@@ -91,8 +91,10 @@ public:
     /// [brackets]. See RFC 3986 Section 3.2.2.
     SBuf hostOrIp() const;
 
-    void port(unsigned short p) {port_=p; touch();}
-    unsigned short port() const {return port_;}
+    /// reset authority port subcomponent
+    void port(const Port p) { port_ = p; touch(); }
+    /// \copydoc port_
+    Port port() const { return port_; }
     /// reset the port to the default port number for the current scheme
     void defaultPort() { port(getScheme().defaultPort()); }
 
@@ -175,7 +177,7 @@ private:
     bool hostIsNumeric_;            ///< whether the authority 'host' is a raw-IP
     Ip::Address hostAddr_;          ///< binary representation of the URI authority if it is a raw-IP
 
-    unsigned short port_;   ///< URL port
+    Port port_; ///< authority port subcomponent
 
     // XXX: for now includes query-string.
     SBuf path_;     ///< URI path segment

diff --git a/src/anyp/UriScheme.cc b/src/anyp/UriScheme.cc
index 2ac9e8fd6ff68e8a7be6dc7e09499c9211a374b4..4a13f707123e5ed14c5bd9e632274c5f6e5d3035 100644 (file)
--- a/src/anyp/UriScheme.cc
+++ b/src/anyp/UriScheme.cc
@@ -67,7 +67,7 @@ AnyP::UriScheme::FindProtocolType(const SBuf &scheme)
     return AnyP::PROTO_UNKNOWN;
 }
 
-unsigned short
+AnyP::Port
 AnyP::UriScheme::defaultPort() const
 {
     switch (theScheme_) {
@@ -97,7 +97,7 @@ AnyP::UriScheme::defaultPort() const
         return 43;
 
     default:
-        return 0;
+        return std::nullopt;
     }
 }
 

diff --git a/src/anyp/UriScheme.h b/src/anyp/UriScheme.h
index 492e98b80cc5ed84ed23bb821c3904116848ff72..08d2cde6d543ee6e88002b1dce69febd8faf785c 100644 (file)
--- a/src/anyp/UriScheme.h
+++ b/src/anyp/UriScheme.h
@@ -13,11 +13,18 @@
 #include "sbuf/SBuf.h"
 
 #include <iosfwd>
+#include <optional>
 #include <vector>
 
 namespace AnyP
 {
 
+/// validated/supported port number; these values are never zero
+using KnownPort = uint16_t;
+
+/// validated/supported port number (if any)
+using Port = std::optional<KnownPort>;
+
 /** This class represents a URI Scheme such as http:// https://, wais://, urn: etc.
  * It does not represent the PROTOCOL that such schemes refer to.
  */
@@ -49,7 +56,7 @@ public:
      */
     SBuf image() const {return image_;}
 
-    unsigned short defaultPort() const;
+    Port defaultPort() const;
 
     /// initializes down-cased protocol scheme names array
     static void Init();

diff --git a/src/carp.cc b/src/carp.cc
index d500d6b02c31d52aa47beaff6a6246fd85f32fb4..126e443b09ef384a5670d31c16908a01486d1dd8 100644 (file)
--- a/src/carp.cc
+++ b/src/carp.cc
@@ -179,7 +179,7 @@ carpSelectParent(PeerSelector *ps)
                 key.append(request->url.host());
             }
             if (tp->options.carp_key.port) {
-                key.appendf(":%u", request->url.port());
+                key.appendf(":%hu", request->url.port().value_or(0));
             }
             if (tp->options.carp_key.path) {
                 // XXX: fix when path and query are separate

diff --git a/src/cf.data.pre b/src/cf.data.pre
index 33087f70ff6fa749e52b329f0e907575e8bb1291..71555d237aa2807cf6b149493ab6b9b07d0ce672 100644 (file)
--- a/src/cf.data.pre
+++ b/src/cf.data.pre
@@ -1219,8 +1219,12 @@ endif
        acl aclname urlpath_regex [-i] \.gif$ ...
          # regex matching on URL path [fast]
 
-       acl aclname port 80 70 21 0-1024...   # destination TCP port [fast]
-                                             # ranges are allowed
+       acl aclname port 80 70 21 0-1024 ...
+         # destination TCP port (or port range) of the request [fast]
+         #
+         # Port 0 matches requests that have no explicit and no default destination
+         # ports (e.g., HTTP requests with URN targets)
+
        acl aclname localport 3128 ...        # TCP port the client connected to [fast]
                                              # NP: for interception mode this is usually '80'
 

diff --git a/src/client_side.cc b/src/client_side.cc
index 0df55abfe7cf34146fb603aa8a075c810c52958e..00d49faab943e542edb32182a472a28a56deb5a4 100644 (file)
--- a/src/client_side.cc
+++ b/src/client_side.cc
@@ -1258,10 +1258,10 @@ ConnStateData::prepareTlsSwitchingURL(const Http1::RequestParserPointer &hp)
         const SBuf &scheme = AnyP::UriScheme(transferProtocol.protocol).image();
         const int url_sz = scheme.length() + useHost.length() + hp->requestUri().length() + 32;
         uri = static_cast<char *>(xcalloc(url_sz, 1));
-        snprintf(uri, url_sz, SQUIDSBUFPH "://" SQUIDSBUFPH ":%d" SQUIDSBUFPH,
+        snprintf(uri, url_sz, SQUIDSBUFPH "://" SQUIDSBUFPH ":%hu" SQUIDSBUFPH,
                  SQUIDSBUFPRINT(scheme),
                  SQUIDSBUFPRINT(useHost),
-                 tlsConnectPort,
+                 *tlsConnectPort,
                  SQUIDSBUFPRINT(hp->requestUri()));
     }
 #endif
@@ -3164,12 +3164,13 @@ ConnStateData::initiateTunneledRequest(HttpRequest::Pointer const &cause, const
 {
     // fake a CONNECT request to force connState to tunnel
     SBuf connectHost;
-    unsigned short connectPort = 0;
+    AnyP::Port connectPort;
 
     if (pinning.serverConnection != nullptr) {
         static char ip[MAX_IPSTRLEN];
         connectHost = pinning.serverConnection->remote.toStr(ip, sizeof(ip));
-        connectPort = pinning.serverConnection->remote.port();
+        if (const auto remotePort = pinning.serverConnection->remote.port())
+            connectPort = remotePort;
     } else if (cause) {
         connectHost = cause->url.hostOrIp();
         connectPort = cause->url.port();
@@ -3182,7 +3183,9 @@ ConnStateData::initiateTunneledRequest(HttpRequest::Pointer const &cause, const
         static char ip[MAX_IPSTRLEN];
         connectHost = clientConnection->local.toStr(ip, sizeof(ip));
         connectPort = clientConnection->local.port();
-    } else {
+    }
+
+    if (!connectPort) {
         // Typical cases are malformed HTTP requests on http_port and malformed
         // TLS handshakes on non-bumping https_port. TODO: Discover these
         // problems earlier so that they can be classified/detailed better.
@@ -3194,7 +3197,7 @@ ConnStateData::initiateTunneledRequest(HttpRequest::Pointer const &cause, const
     }
 
     debugs(33, 2, "Request tunneling for " << reason);
-    ClientHttpRequest *http = buildFakeRequest(connectHost, connectPort, payload);
+    const auto http = buildFakeRequest(connectHost, *connectPort, payload);
     HttpRequest::Pointer request = http->request;
     request->flags.forceTunnel = true;
     http->calloutContext = new ClientRequestContext(http);
@@ -3233,7 +3236,7 @@ ConnStateData::fakeAConnectRequest(const char *reason, const SBuf &payload)
 }
 
 ClientHttpRequest *
-ConnStateData::buildFakeRequest(SBuf &useHost, unsigned short usePort, const SBuf &payload)
+ConnStateData::buildFakeRequest(SBuf &useHost, const AnyP::KnownPort usePort, const SBuf &payload)
 {
     ClientHttpRequest *http = new ClientHttpRequest(this);
     Http::Stream *stream = new Http::Stream(clientConnection, http);

diff --git a/src/client_side.h b/src/client_side.h
index 6027b318b67cbd0feb1befdbb1b2f08014547378..e37ab27da1a275a0b97eba1706f350646960e383 100644 (file)
--- a/src/client_side.h
+++ b/src/client_side.h
@@ -143,7 +143,7 @@ public:
     struct {
         Comm::ConnectionPointer serverConnection; /* pinned server side connection */
         char *host = nullptr; ///< host name of pinned connection
-        int port = -1; ///< port of pinned connection
+        AnyP::Port port; ///< destination port of the request that caused serverConnection
         bool pinned = false; ///< this connection was pinned
         bool auth = false; ///< pinned for www authentication
         bool reading = false; ///< we are monitoring for peer connection closure
@@ -342,7 +342,7 @@ public:
     bool shouldPreserveClientData() const;
 
     /// build a fake http request
-    ClientHttpRequest *buildFakeRequest(SBuf &useHost, unsigned short usePort, const SBuf &payload);
+    ClientHttpRequest *buildFakeRequest(SBuf &useHost, AnyP::KnownPort usePort, const SBuf &payload);
 
     /// From-client handshake bytes (including bytes at the beginning of a
     /// CONNECT tunnel) which we may need to forward as-is if their syntax does
@@ -480,9 +480,10 @@ private:
     /// The number of parsed HTTP requests headers on a bumped client connection
     uint64_t parsedBumpedRequestCount = 0;
 
+    // TODO: Replace tlsConnectHostOrIp and tlsConnectPort with CONNECT request AnyP::Uri
     /// The TLS server host name appears in CONNECT request or the server ip address for the intercepted requests
     SBuf tlsConnectHostOrIp; ///< The TLS server host name as passed in the CONNECT request
-    unsigned short tlsConnectPort = 0; ///< The TLS server port number as passed in the CONNECT request
+    AnyP::Port tlsConnectPort; ///< The TLS server port number as passed in the CONNECT request
     SBuf sslCommonName_; ///< CN name for SSL certificate generation
 
     /// TLS client delivered SNI value. Empty string if none has been received.

diff --git a/src/client_side_request.cc b/src/client_side_request.cc
index dc2c3d1a48e66c6c965ce6fb34fbed5c06359d48..5f7b8bdc56a26711a995939b57d19342c22162b1 100644 (file)
--- a/src/client_side_request.cc
+++ b/src/client_side_request.cc
@@ -560,6 +560,7 @@ ClientRequestContext::hostHeaderVerify()
         return;
     }
 
+    // TODO: Unify Host value parsing below with AnyP::Uri authority parsing
     // Locate if there is a port attached, strip ready for IP lookup
     char *portStr = nullptr;
     char *hostB = xstrdup(host);
@@ -614,14 +615,17 @@ ClientRequestContext::hostHeaderVerify()
         // Verify forward-proxy requested URL domain matches the Host: header
         debugs(85, 3, "FAIL on validate URL domain " << http->request->url.host() << " matches Host: " << host);
         hostHeaderVerifyFailed(host, http->request->url.host());
-    } else if (portStr && port != http->request->url.port()) {
+    } else if (portStr && !http->request->url.port()) {
+        debugs(85, 3, "FAIL on validate portless URI matches Host: " << portStr);
+        hostHeaderVerifyFailed("portless URI", portStr);
+    } else if (portStr && port != *http->request->url.port()) {
         // Verify forward-proxy requested URL domain matches the Host: header
-        debugs(85, 3, "FAIL on validate URL port " << http->request->url.port() << " matches Host: port " << portStr);
+        debugs(85, 3, "FAIL on validate URL port " << *http->request->url.port() << " matches Host: port " << portStr);
         hostHeaderVerifyFailed("URL port", portStr);
     } else if (!portStr && http->request->method != Http::METHOD_CONNECT && http->request->url.port() != http->request->url.getScheme().defaultPort()) {
         // Verify forward-proxy requested URL domain matches the Host: header
         // Special case: we don't have a default-port to check for CONNECT. Assume URL is correct.
-        debugs(85, 3, "FAIL on validate URL port " << http->request->url.port() << " matches Host: default port " << http->request->url.getScheme().defaultPort());
+        debugs(85, 3, "FAIL on validate URL port " << http->request->url.port().value_or(0) << " matches Host: default port " << http->request->url.getScheme().defaultPort().value_or(0));
         hostHeaderVerifyFailed("URL port", "default port");
     } else {
         // Okay no problem.

diff --git a/src/clients/FtpGateway.cc b/src/clients/FtpGateway.cc
index 0f0d0ddcc3d4b3c7022aed45bfaa549a6be1ab7e..c3c7b21d0720cd4ed171ae516cf13f4392ec76cb 100644 (file)
--- a/src/clients/FtpGateway.cc
+++ b/src/clients/FtpGateway.cc
@@ -1266,8 +1266,9 @@ Ftp::Gateway::ftpRealm()
         realm.append("unknown", 7);
     else {
         realm.append(request->url.host());
-        if (request->url.port() != 21)
-            realm.appendf(" port %d", request->url.port());
+        const auto &rport = request->url.port();
+        if (rport && *rport != 21)
+            realm.appendf(" port %hu", *rport);
     }
     return realm;
 }

diff --git a/src/errorpage.cc b/src/errorpage.cc
index c4287f4140db0047f1a3a937cb8b45965d8e8094..b23f17a710db42c7670d33cf7daf814eeefaaa79 100644 (file)
--- a/src/errorpage.cc
+++ b/src/errorpage.cc
@@ -1086,8 +1086,8 @@ ErrorState::compileLegacyCode(Build &build)
         break;
 
     case 'p':
-        if (request) {
-            mb.appendf("%u", request->url.port());
+        if (request && request->url.port()) {
+            mb.appendf("%hu", *request->url.port());
         } else if (!building_deny_info_url) {
             p = "[unknown port]";
         }

diff --git a/src/errorpage.h b/src/errorpage.h
index 1d3b53045173c763856365dee39d6248c74c647e..79dea859092c8d4d337741cae9d34a9e696dfdbe 100644 (file)
--- a/src/errorpage.h
+++ b/src/errorpage.h
@@ -177,7 +177,6 @@ public:
     HttpRequestPointer request;
     char *url = nullptr;
     int xerrno = 0;
-    unsigned short port = 0;
     std::optional<SBuf> dnsError; ///< DNS lookup error message
     time_t ttl = 0;
 

diff --git a/src/format/Format.cc b/src/format/Format.cc
index ce8d59701bf070b38edd4ead3a04618ea2b9473c..e7e689d381f9757df9798e9d70fa08c134ea7d4c 100644 (file)
--- a/src/format/Format.cc
+++ b/src/format/Format.cc
@@ -508,7 +508,7 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS
 
         case LFT_LOCAL_LISTENING_PORT:
             if (const auto port = FindListeningPortNumber(nullptr, al.getRaw())) {
-                outint = port;
+                outint = *port;
                 doint = 1;
             }
             break;
@@ -1058,8 +1058,8 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS
             break;
 
         case LFT_CLIENT_REQ_URLPORT:
-            if (al->request) {
-                outint = al->request->url.port();
+            if (al->request && al->request->url.port()) {
+                outint = *al->request->url.port();
                 doint = 1;
             }
             break;
@@ -1134,8 +1134,8 @@ Format::Format::assemble(MemBuf &mb, const AccessLogEntry::Pointer &al, int logS
             break;
 
         case LFT_SERVER_REQ_URLPORT:
-            if (al->adapted_request) {
-                outint = al->adapted_request->url.port();
+            if (al->adapted_request && al->adapted_request->url.port()) {
+                outint = *al->adapted_request->url.port();
                 doint = 1;
             }
             break;

diff --git a/src/peer_select.cc b/src/peer_select.cc
index 886e136852cbd66c8f4f85bcbaf7a7a1a1d8f1de..834fb77502f0e8f9f24fe2254ff335433e985d95 100644 (file)
--- a/src/peer_select.cc
+++ b/src/peer_select.cc
@@ -530,7 +530,10 @@ PeerSelector::noteIp(const Ip::Address &ip)
 
     Comm::ConnectionPointer p = new Comm::Connection();
     p->remote = ip;
-    p->remote.port(peer ? peer->http_port : request->url.port());
+    // XXX: We return a (non-peer) destination with a zero port if the selection
+    // initiator supplied a request target without a port. If there are no valid
+    // use cases for this behavior, stop _selecting_ such destinations.
+    p->remote.port(peer ? peer->http_port : request->url.port().value_or(0));
     handlePath(p, *servers);
 }
 

diff --git a/src/tests/testHttpRequest.cc b/src/tests/testHttpRequest.cc
index fb4b689cf30ebcd9b6a1484e45ed16de3204819c..446993ac25406c0a8abae84bd9002267e66f29c6 100644 (file)
--- a/src/tests/testHttpRequest.cc
+++ b/src/tests/testHttpRequest.cc
@@ -44,13 +44,12 @@ void
 TestHttpRequest::testCreateFromUrl()
 {
     /* vanilla url, implicit method */
-    unsigned short expected_port;
     SBuf url("http://foo:90/bar");
     const auto mx = MasterXaction::MakePortless<XactionInitiator::initHtcp>();
     HttpRequest *aRequest = HttpRequest::FromUrl(url, mx);
-    expected_port = 90;
+    AnyP::KnownPort expected_port = 90;
     CPPUNIT_ASSERT(aRequest != nullptr);
-    CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port());
+    CPPUNIT_ASSERT_EQUAL(expected_port, *aRequest->url.port());
     CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET);
     CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->url.host()));
     CPPUNIT_ASSERT_EQUAL(SBuf("/bar"), aRequest->url.path());
@@ -61,7 +60,7 @@ TestHttpRequest::testCreateFromUrl()
     aRequest = HttpRequest::FromUrl(url, mx, Http::METHOD_GET);
     expected_port = 90;
     CPPUNIT_ASSERT(aRequest != nullptr);
-    CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port());
+    CPPUNIT_ASSERT_EQUAL(expected_port, *aRequest->url.port());
     CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET);
     CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->url.host()));
     CPPUNIT_ASSERT_EQUAL(SBuf("/bar"), aRequest->url.path());
@@ -72,7 +71,7 @@ TestHttpRequest::testCreateFromUrl()
     aRequest = HttpRequest::FromUrl(url, mx, Http::METHOD_PUT);
     expected_port = 80;
     CPPUNIT_ASSERT(aRequest != nullptr);
-    CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port());
+    CPPUNIT_ASSERT_EQUAL(expected_port, *aRequest->url.port());
     CPPUNIT_ASSERT(aRequest->method == Http::METHOD_PUT);
     CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->url.host()));
     CPPUNIT_ASSERT_EQUAL(SBuf("/bar"), aRequest->url.path());
@@ -89,7 +88,7 @@ TestHttpRequest::testCreateFromUrl()
     aRequest = HttpRequest::FromUrl(url, mx, Http::METHOD_CONNECT);
     expected_port = 45;
     CPPUNIT_ASSERT(aRequest != nullptr);
-    CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port());
+    CPPUNIT_ASSERT_EQUAL(expected_port, *aRequest->url.port());
     CPPUNIT_ASSERT(aRequest->method == Http::METHOD_CONNECT);
     CPPUNIT_ASSERT_EQUAL(String("foo"), String(aRequest->url.host()));
     CPPUNIT_ASSERT_EQUAL(SBuf(), aRequest->url.path());
@@ -104,15 +103,14 @@ TestHttpRequest::testCreateFromUrl()
 void
 TestHttpRequest::testIPv6HostColonBug()
 {
-    unsigned short expected_port;
     HttpRequest *aRequest = nullptr;
 
     /* valid IPv6 address without port */
     SBuf url("http://[2000:800::45]/foo");
     const auto mx = MasterXaction::MakePortless<XactionInitiator::initHtcp>();
     aRequest = HttpRequest::FromUrl(url, mx, Http::METHOD_GET);
-    expected_port = 80;
-    CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port());
+    AnyP::KnownPort expected_port = 80;
+    CPPUNIT_ASSERT_EQUAL(expected_port, *aRequest->url.port());
     CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET);
     CPPUNIT_ASSERT_EQUAL(String("[2000:800::45]"), String(aRequest->url.host()));
     CPPUNIT_ASSERT_EQUAL(SBuf("/foo"), aRequest->url.path());
@@ -122,7 +120,7 @@ TestHttpRequest::testIPv6HostColonBug()
     url = "http://[2000:800::45]:90/foo";
     aRequest = HttpRequest::FromUrl(url, mx, Http::METHOD_GET);
     expected_port = 90;
-    CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port());
+    CPPUNIT_ASSERT_EQUAL(expected_port, *aRequest->url.port());
     CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET);
     CPPUNIT_ASSERT_EQUAL(String("[2000:800::45]"), String(aRequest->url.host()));
     CPPUNIT_ASSERT_EQUAL(SBuf("/foo"), aRequest->url.path());
@@ -132,7 +130,7 @@ TestHttpRequest::testIPv6HostColonBug()
     url = "http://2000:800::45/foo";
     aRequest = HttpRequest::FromUrl(url, mx, Http::METHOD_GET);
     expected_port = 80;
-    CPPUNIT_ASSERT_EQUAL(expected_port, aRequest->url.port());
+    CPPUNIT_ASSERT_EQUAL(expected_port, *aRequest->url.port());
     CPPUNIT_ASSERT(aRequest->method == Http::METHOD_GET);
     CPPUNIT_ASSERT_EQUAL(String("[2000:800::45]"), String(aRequest->url.host()));
     CPPUNIT_ASSERT_EQUAL(SBuf("/foo"), aRequest->url.path());

diff --git a/src/wccp2.cc b/src/wccp2.cc
index 862f23d8ae9b763afcc4e8ea57d4e1f912d4aa81..df307dcabc9db3d6c9082e0de4af3738c91433d6 100644 (file)
--- a/src/wccp2.cc
+++ b/src/wccp2.cc
@@ -1669,8 +1669,6 @@ wccp2AssignBuckets(void *)
     int router_len;
     int bucket_counter;
     uint32_t service_flags;
-    unsigned short port = WCCP_PORT;
-
     /* Packet segments */
 
     struct wccp2_message_header_t *main_header;
@@ -1703,7 +1701,7 @@ wccp2AssignBuckets(void *)
     router_len = sizeof(router);
     memset(&router, '\0', router_len);
     router.sin_family = AF_INET;
-    router.sin_port = htons(port);
+    router.sin_port = htons(WCCP_PORT);
 
     /* Start main header - fill in length later */
     offset = 0;