Re: [PATCH] Fix random selection of keyserver

[Philip Kaludercic]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Perhaps, the intent of this patch was just to fix the currently broken
>> behaviour suggested by epa-keyserver's type.
>>
>> I am not sure if a :set function could be used to randomize the
>> keyserver for every request. Then again, is might not even really be
>> necessary to provide such a feature.
>
> I wonder whether it might make sense instead to allow `epa-keyserver' to
> be a list, and if so, then choose a random server from that list.
> That'd give users control over what servers to use when randomising. 

That does seem to be a better approach, how does the patch below look
like?

-- 
        Philip K.

>From fdbb17539a465c49aad93cef4e0b8b4b76699561 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Sun, 30 May 2021 20:34:54 +0200
Subject: [PATCH] Improve random selection of keyservers

* epa-ks.el (epa-keyserver): Interpret a list as a pool
(epa-ks--query-url): Add new auxiliary function
(epa-ks--fetch-key): Use epa-ks--query-url
(epa-search-keys): Use epa-ks--query-url
---
 lisp/epa-ks.el | 42 +++++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/lisp/epa-ks.el b/lisp/epa-ks.el
index a33025b112..1d17c1399c 100644
--- a/lisp/epa-ks.el
+++ b/lisp/epa-ks.el
@@ -42,8 +42,9 @@ epa-keyserver
   "Domain of keyserver.
 
 This is used by `epa-ks-lookup-key', for looking up public keys."
-  :type '(choice :tag "Keyserver"
-                 (const random)
+  :type `(choice :tag "Keyserver"
+                 (repeat :tag "Random pool"
+                         (string :tag "Keyserver address"))
                  (const "keyring.debian.org")
                  (const "keys.gnupg.net")
                  (const "keyserver.ubuntu.com")
@@ -141,20 +142,33 @@ epa-ks-do-key-to-fetch
         (epa-ks--fetch-key id))))
   (tabulated-list-clear-all-tags))
 
+(defun epa-ks--query-url (query exact)
+  "Return URL for QUERY.
+If EXACT is non-nil, don't accept approximate matches."
+  (format "https://%s/pks/lookup?%s";
+          (cond ((null epa-keyserver)
+                 (user-error "Empty keyserver pool"))
+                ((listp epa-keyserver)
+                 (nth (random (length epa-keyserver))
+                      epa-keyserver))
+                ((stringp epa-keyserver)
+                 epa-keyserver)
+                ((error "Invalid type for `epa-keyserver'")))
+          (url-build-query-string
+           (append `(("search" ,query)
+                     ("options" "mr")
+                     ("op" "index"))
+                   (and exact '(("exact" "on")))))))
+
 (defun epa-ks--fetch-key (id)
   "Send request to import key with specified ID."
   (url-retrieve
-   (format "https://%s/pks/lookup?%s";
-           epa-keyserver
-           (url-build-query-string
-            `(("search" ,(concat "0x" (url-hexify-string id)))
-              ("options" "mr")
-              ("op" "get"))))
+   (epa-ks--query-url (concat "0x" (url-hexify-string id)) t)
    (lambda (status)
      (when (plist-get status :error)
        (error "Request failed: %s"
-           (caddr (assq (caddr (plist-get status :error))
-                        url-http-codes))))
+              (caddr (assq (caddr (plist-get status :error))
+                           url-http-codes))))
      (forward-paragraph)
      (save-excursion
        (goto-char (point-max))
@@ -224,13 +238,7 @@ epa-search-keys
         (erase-buffer))
       (epa-ks-search-mode))
     (url-retrieve
-     (format "https://%s/pks/lookup?%s";
-             epa-keyserver
-             (url-build-query-string
-              (append `(("search" ,query)
-                        ("options" "mr")
-                        ("op" "index"))
-                      (and exact '(("exact" "on"))))))
+     (epa-ks--query-url query exact)
      (lambda (status)
        (when (plist-get status :error)
          (when buf
-- 
2.30.2