From: Amos Jeffries <yadij@users.noreply.github.com>
Date: Fri, 27 Jul 2018 00:12:11 +0000 (+0000)
Subject: Use AnyP::Uri to generate ASN request URI (#258)
X-Git-Tag: M-staged-PR258
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=6f160e6fe209011cba1e8b9929302f2da82285d1;p=thirdparty%2Fsquid.git

Use AnyP::Uri to generate ASN request URI (#258)

Removes the fixed size asres c-string buffer the URL was
using by replacing with an SBuf and dynamic xstrdup().

Also, make AS_REQBUF_SZ configurable at build time with
-DAS_REQBUF_SZ=nn.

TODO: convert the store API to accept AnyP::Uri instead of
c-string will remove 1-2 memory reallocations.
---

diff --git a/src/acl/Asn.cc b/src/acl/Asn.cc
index fdeca2834f..0781446479 100644
--- a/src/acl/Asn.cc
+++ b/src/acl/Asn.cc
@@ -29,7 +29,9 @@
 #include "StoreClient.h"
 
 #define WHOIS_PORT 43
+#ifndef AS_REQBUF_SZ
 #define AS_REQBUF_SZ    4096
+#endif
 
 /* BEGIN of definitions for radix tree entries */
 
@@ -70,40 +72,30 @@ class ASState
     CBDATA_CLASS(ASState);
 
 public:
-    ASState();
-    ~ASState();
+    ASState() {
+        memset(reqbuf, 0, sizeof(reqbuf));
+    }
+    ~ASState() {
+        if (entry) {
+            debugs(53, 3, entry->url());
+            storeUnregister(sc, entry, this);
+            entry->unlock("~ASState");
+        }
+    }
 
-    StoreEntry *entry;
-    store_client *sc;
+public:
+    StoreEntry *entry = nullptr;
+    store_client *sc = nullptr;
     HttpRequest::Pointer request;
-    int as_number;
-    int64_t offset;
-    int reqofs;
+    int as_number = 0;
+    int64_t offset = 0;
+    int reqofs = 0;
     char reqbuf[AS_REQBUF_SZ];
-    bool dataRead;
+    bool dataRead = false;
 };
 
 CBDATA_CLASS_INIT(ASState);
 
-ASState::ASState() :
-    entry(NULL),
-    sc(NULL),
-    request(NULL),
-    as_number(0),
-    offset(0),
-    reqofs(0),
-    dataRead(false)
-{
-    memset(reqbuf, 0, AS_REQBUF_SZ);
-}
-
-ASState::~ASState()
-{
-    debugs(53, 3, entry->url());
-    storeUnregister(sc, entry, this);
-    entry->unlock("~ASState");
-}
-
 /** entry into the radix tree */
 struct rtentry_t {
     struct squid_radix_node e_nodes[2];
@@ -235,19 +227,27 @@ asnStats(StoreEntry * sentry)
 static void
 asnCacheStart(int as)
 {
-    // TODO: use class AnyP::Uri instead of generating a string and re-parsing
-    LOCAL_ARRAY(char, asres, 4096);
-    StoreEntry *e;
-    ASState *asState = new ASState;
+    AnyP::Uri whoisUrl(AnyP::PROTO_WHOIS);
+    whoisUrl.host(Config.as_whois_server);
+
+    SBuf asPath("/!gAS");
+    asPath.appendf("%d", as);
+    whoisUrl.path(asPath);
+
     debugs(53, 3, "AS " << as);
-    snprintf(asres, 4096, "whois://%s/!gAS%d", Config.as_whois_server, as);
+    ASState *asState = new ASState;
     asState->as_number = as;
     const MasterXaction::Pointer mx = new MasterXaction(XactionInitiator::initAsn);
-    asState->request = HttpRequest::FromUrl(asres, mx);
-    assert(asState->request != NULL);
+    asState->request = new HttpRequest(mx);
+    asState->request->url = whoisUrl;
+    asState->request->method = Http::METHOD_GET;
+
+    // XXX: performance regression, c_str() reallocates
+    const auto asres = xstrdup(whoisUrl.absolute().c_str());
 
     // XXX: Missing a hittingRequiresCollapsing() && startCollapsingOn() check.
-    if ((e = storeGetPublic(asres, Http::METHOD_GET)) == NULL) {
+    auto e = storeGetPublic(asres, Http::METHOD_GET);
+    if (!e) {
         e = storeCreateEntry(asres, asres, RequestFlags(), Http::METHOD_GET);
         asState->sc = storeClientListAdd(e, asState);
         FwdState::fwdStart(Comm::ConnectionPointer(), e, asState->request.getRaw());
@@ -255,6 +255,7 @@ asnCacheStart(int as)
         e->lock("Asn");
         asState->sc = storeClientListAdd(e, asState);
     }
+    xfree(asres);
 
     asState->entry = e;
     StoreIOBuffer readBuffer (AS_REQBUF_SZ, asState->offset, asState->reqbuf);