altsvc: cap the list at 5,000 entries

Avoid never-ending growth.

When adding more entries, it now deletes the first entry in the list,
which is the oldest added entry still held in memory. I decided to avoid
a Least Recently Used concept as I suspect with a list with this many
entries most entries have not been used, and we don't save the timestamp
of recent use anyway.

The net effect might (no matter what) be that the removed entry might
feel a bit "random" in the eyes of the user.

Verify with test 1669.

Reported-by: Geeknik Labs
Fixes #21183
Closes #21189

diff --git a/lib/altsvc.c b/lib/altsvc.c
index eb072bb49d55da42b8ae1ff1eb25c16c2198b494..e0834cb66357a0ce1403426bba9675358eff5548 100644 (file)
--- a/lib/altsvc.c
+++ b/lib/altsvc.c
@@ -144,6 +144,20 @@ static struct altsvc *altsvc_create(struct Curl_str *srchost,
                          srcport, dstport);
 }
 
+/* append the new entry to the list after possibly removing an old entry
+   first */
+static void altsvc_append(struct altsvcinfo *asi, struct altsvc *as)
+{
+  while(Curl_llist_count(&asi->list) >= MAX_ALTSVC_ENTRIES) {
+    /* It's full. Remove the first entry in the list */
+    struct Curl_llist_node *e = Curl_llist_head(&asi->list);
+    struct altsvc *oldas = Curl_node_elem(e);
+    Curl_node_remove(e);
+    altsvc_free(oldas);
+  }
+  Curl_llist_append(&asi->list, as, &as->node);
+}
+
 /* only returns SERIOUS errors */
 static CURLcode altsvc_add(struct altsvcinfo *asi, const char *line)
 {
@@ -180,7 +194,6 @@ static CURLcode altsvc_add(struct altsvcinfo *asi, const char *line)
      curlx_str_newline(&line))
     ;
   else {
-    struct altsvc *as;
     char dbuf[MAX_ALTSVC_DATELEN + 1];
     time_t expires = 0;
     time_t now = time(NULL);
@@ -192,12 +205,12 @@ static CURLcode altsvc_add(struct altsvcinfo *asi, const char *line)
     Curl_getdate_capped(dbuf, &expires);
 
     if(now < expires) {
-      as = altsvc_create(&srchost, &dsthost, &srcalpn, &dstalpn,
-                         (size_t)srcport, (size_t)dstport);
+      struct altsvc *as = altsvc_create(&srchost, &dsthost, &srcalpn, &dstalpn,
+                                        (size_t)srcport, (size_t)dstport);
       if(as) {
         as->expires = expires;
         as->persist = persist ? 1 : 0;
-        Curl_llist_append(&asi->list, as, &as->node);
+        altsvc_append(asi, as);
       }
       else
         return CURLE_OUT_OF_MEMORY;
@@ -594,7 +607,7 @@ CURLcode Curl_altsvc_parse(struct Curl_easy *data,
             else
               as->expires = maxage + secs;
             as->persist = persist;
-            Curl_llist_append(&asi->list, as, &as->node);
+            altsvc_append(asi, as);
             infof(data, "Added alt-svc: %.*s:%d over %s",
                   (int)curlx_strlen(&dsthost), curlx_str(&dsthost),
                   dstport, Curl_alpnid2str(dstalpnid));

diff --git a/lib/altsvc.h b/lib/altsvc.h
index d1fed4fe1d667775737693b1b6ccda300a0a448c..dc1740bce13ebbda76f69090552f8c4d3f6086b4 100644 (file)
--- a/lib/altsvc.h
+++ b/lib/altsvc.h
@@ -28,6 +28,9 @@
 #if !defined(CURL_DISABLE_HTTP) && !defined(CURL_DISABLE_ALTSVC)
 #include "llist.h"
 
+/* the maximum number of alt-svc entries kept in a single cache */
+#define MAX_ALTSVC_ENTRIES 5000
+
 struct althost {
   char *host;
   uint16_t port;

diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 78e3f265857c26ae15eb1ddeae5b1ee45019e827..c3496b133f37cf7612010a19725e41b4cbd8b8d2 100644 (file)
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -221,7 +221,7 @@ test1638 test1639 test1640 test1641 test1642 test1643 test1644 \
 \
 test1650 test1651 test1652 test1653 test1654 test1655 test1656 test1657 \
 test1658 test1659 test1660 test1661 test1662 test1663 test1664 test1665 \
-test1666 test1667 test1668 \
+test1666 test1667 test1668 test1669 \
 \
 test1670 test1671 test1672 test1673 \
 \

diff --git a/tests/data/test1669 b/tests/data/test1669
new file mode 100644 (file)
index 0000000..204b45b
--- /dev/null
+++ b/tests/data/test1669
@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="US-ASCII"?>
+<testcase>
+<info>
+<keywords>
+unittest
+Alt-Svc
+</keywords>
+</info>
+
+<client>
+<features>
+unittest
+alt-svc
+</features>
+
+# This date is exactly "20190124 22:34:21" UTC
+<setenv>
+CURL_TIME=1548369261
+</setenv>
+<name>
+alt-svc load cache with >5000 entries
+</name>
+<command>
+%LOGDIR/%TESTNUMBER
+</command>
+<file name="%LOGDIR/%TESTNUMBER" mode="text">
+%repeat[5010 x h2 example.com 443 h3 shiny.example.com 8443 "20191231 00:00:00" 0 0%0a]%
+</file>
+</client>
+
+<verify>
+<limits>
+Allocations: 5100
+Maximum allocated: 600000
+</limits>
+</verify>
+</testcase>

diff --git a/tests/unit/Makefile.inc b/tests/unit/Makefile.inc
index 459aee5f841aa0f2f112980dc3d5bdf72d6934fc..d8b2ed2d2edc9eafa238e6dd96c5a13a40cf62e4 100644 (file)
--- a/tests/unit/Makefile.inc
+++ b/tests/unit/Makefile.inc
@@ -41,7 +41,7 @@ TESTS_C = \
   unit1636.c \
   unit1650.c unit1651.c unit1652.c unit1653.c unit1654.c unit1655.c unit1656.c \
   unit1657.c unit1658.c            unit1660.c unit1661.c unit1663.c unit1664.c \
-             unit1666.c unit1667.c unit1668.c \
+             unit1666.c unit1667.c unit1668.c unit1669.c \
   unit1979.c unit1980.c \
   unit2600.c unit2601.c unit2602.c unit2603.c unit2604.c unit2605.c \
   unit3200.c                                             unit3205.c \

diff --git a/tests/unit/unit1669.c b/tests/unit/unit1669.c
new file mode 100644 (file)
index 0000000..625e004
--- /dev/null
+++ b/tests/unit/unit1669.c
@@ -0,0 +1,58 @@
+/***************************************************************************
+ *                                  _   _ ____  _
+ *  Project                     ___| | | |  _ \| |
+ *                             / __| | | | |_) | |
+ *                            | (__| |_| |  _ <| |___
+ *                             \___|\___/|_| \_\_____|
+ *
+ * Copyright (C) Daniel Stenberg, <daniel@haxx.se>, et al.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at https://curl.se/docs/copyright.html.
+ *
+ * You may opt to use, copy, modify, merge, publish, distribute and/or sell
+ * copies of the Software, and permit persons to whom the Software is
+ * furnished to do so, under the terms of the COPYING file.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ * SPDX-License-Identifier: curl
+ *
+ ***************************************************************************/
+#include "unitcheck.h"
+
+#include "urldata.h"
+#include "altsvc.h"
+
+static CURLcode test_unit1669(const char *arg)
+{
+  UNITTEST_BEGIN_SIMPLE
+
+#if !defined(CURL_DISABLE_HTTP) && !defined(CURL_DISABLE_ALTSVC)
+  char outname[256];
+  CURL *curl;
+  CURLcode result;
+  struct altsvcinfo *asi = Curl_altsvc_init();
+  abort_if(!asi, "Curl_altsvc_init");
+  result = Curl_altsvc_load(asi, arg);
+  fail_if(result, "Curl_altsvc_load");
+  if(result)
+    goto fail;
+  curl_global_init(CURL_GLOBAL_ALL);
+  curl = curl_easy_init();
+  fail_if(!curl, "curl_easy_init");
+  if(!curl)
+    goto fail;
+  fail_unless(Curl_llist_count(&asi->list) == MAX_ALTSVC_ENTRIES,
+              "wrong number of entries");
+  curl_msnprintf(outname, sizeof(outname), "%s-out", arg);
+
+  curl_easy_cleanup(curl);
+fail:
+  Curl_altsvc_cleanup(&asi);
+#endif
+
+  UNITTEST_END(curl_global_cleanup())
+}