problems importing keys via epa-search-keys

problems importing keys via epa-search-keys

[Giovanni Biscuolo]

[-- Attachment #1: Type: text/plain, Size: 955 bytes --]

Hello,

(please if you reply include me in Cc: since I'm not subscribed)

if I "M-x epa-search-keys" and try to fetch a key with =f= and then =x=
in =Key search= buffer, I get this in *Messages*:

--8<---------------cut here---------------start------------->8---

Searching keys...
Press ‘f’ to mark a key, ‘x’ to fetch all marked keys.
Contacting host: keys.openpgp.org:443

--8<---------------cut here---------------end--------------->8---

but the key is never imported in my gnupg keyring.

I set the keyserver this way (evaluating expression in a *scratch*
buffer, for testing):

--8<---------------cut here---------------start------------->8---

(setq epa-keyserver "keys.openpgp.org")

--8<---------------cut here---------------end--------------->8---

Please have you any hint for what I'm doing wrong or what can I do to
debug my problem?

Thanks! Gio'

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 849 bytes --]

Re: problems importing keys via epa-search-keys

[Filipp Gunbin]

On 09/03/2023 08:14 +0100, Giovanni Biscuolo wrote:

> Hello,
>
> (please if you reply include me in Cc: since I'm not subscribed)
>
> if I "M-x epa-search-keys" and try to fetch a key with =f= and then =x=
> in =Key search= buffer, I get this in *Messages*:
>
>
> Searching keys...
> Press ‘f’ to mark a key, ‘x’ to fetch all marked keys.
> Contacting host: keys.openpgp.org:443
>
>
> but the key is never imported in my gnupg keyring.
>

[...]

Hi,

Yes, broken since 23b6cd41f55b833e8a562c51642d36b5211a510b, the patch is
below.

Eli, would this be OK for emacs-29?  This is clearly a regression.

Thanks.


diff --git a/lisp/epa-ks.el b/lisp/epa-ks.el
index 77d896fa438..2ab550cb9b1 100644
--- a/lisp/epa-ks.el
+++ b/lisp/epa-ks.el
@@ -140,8 +140,8 @@ 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.
+(defun epa-ks--query-url (query operation exact)
+  "Return URL for QUERY and OPERATION.
 If EXACT is non-nil, don't accept approximate matches."
   (format "https://%s/pks/lookup?%s"
           (cond ((null epa-keyserver)
@@ -154,13 +154,13 @@ epa-ks--query-url
           (url-build-query-string
            (append `(("search" ,query)
                      ("options" "mr")
-                     ("op" "index"))
+                     ("op" ,operation))
                    (and exact '(("exact" "on")))))))
 
 (defun epa-ks--fetch-key (id)
   "Send request to import key with specified ID."
   (url-retrieve
-   (epa-ks--query-url (concat "0x" (url-hexify-string id)) t)
+   (epa-ks--query-url (concat "0x" (url-hexify-string id)) "get" t)
    (lambda (status)
      (when (plist-get status :error)
        (error "Request failed: %s"
@@ -236,7 +236,7 @@ epa-search-keys
         (erase-buffer))
       (epa-ks-search-mode))
     (url-retrieve
-     (epa-ks--query-url query exact)
+     (epa-ks--query-url query "index" exact)
      (lambda (status)
        (when (plist-get status :error)
          (when buf

Re: problems importing keys via epa-search-keys

[Giovanni Biscuolo]

[-- Attachment #1: Type: text/plain, Size: 859 bytes --]

Hi Filipp,

Filipp Gunbin <fgunbin@fastmail.fm> writes:

> On 09/03/2023 08:14 +0100, Giovanni Biscuolo wrote:

[...]

>> if I "M-x epa-search-keys" and try to fetch a key with =f= and then =x=
>> in =Key search= buffer, I get this in *Messages*:

[...]

>> but the key is never imported in my gnupg keyring.

[...]

> Yes, broken since 23b6cd41f55b833e8a562c51642d36b5211a510b, the patch is
> below.

Thanks a lot!

I'm using Guix so I'll try to apply your patch for Emacs 28.2 to that
package and submit it for inclusion in official Guix distribution.

AFAIU there's no bug report for this, or am I wrong?  In case there
isn't one I'm going to submit a new bug report.

> Eli, would this be OK for emacs-29?  This is clearly a regression.

[...]

Happy hacking!  Gio'.

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 849 bytes --]

Re: problems importing keys via epa-search-keys

[Eli Zaretskii]

> From: Filipp Gunbin <fgunbin@fastmail.fm>
> Cc: help-gnu-emacs@gnu.org, Philip Kaludercic <philipk@posteo.net>, Eli
>  Zaretskii <eliz@gnu.org>
> Date: Sat, 11 Mar 2023 01:58:43 +0300
> 
> Yes, broken since 23b6cd41f55b833e8a562c51642d36b5211a510b, the patch is
> below.
> 
> Eli, would this be OK for emacs-29?  This is clearly a regression.

It's a regression that existed since Emacs 28.1, right?  So its fix
should try to avoid changing APIs.  Therefore, please try to make a
change that leaves the signature of epa-ks--query-url unchanged, I
think it should be easy in this case.  (Yes, I know it's an internal
function, but that doesn't mean we can change the signature when we
can avoid that.)

Thanks.

P.S. And why are you posting patches and their discussion here, not on
the bug tracker?

Re: problems importing keys via epa-search-keys

[Filipp Gunbin]

Hi Giovanni,

On 11/03/2023 08:05 +0100, Giovanni Biscuolo wrote:

[...]

> I'm using Guix so I'll try to apply your patch for Emacs 28.2 to that
> package and submit it for inclusion in official Guix distribution.

No need for that, I'll install it either on master or on emacs-29,
depending on what Eli says.

> AFAIU there's no bug report for this, or am I wrong?  In case there
> isn't one I'm going to submit a new bug report.

No need for that either.  But yes, it's better to submit problems using
M-x report-emacs-bug, for the future.

Thanks.

Re: problems importing keys via epa-search-keys

[Filipp Gunbin]

On 11/03/2023 09:17 +0200, Eli Zaretskii wrote:

>> From: Filipp Gunbin <fgunbin@fastmail.fm>
>> Cc: help-gnu-emacs@gnu.org, Philip Kaludercic <philipk@posteo.net>, Eli
>>  Zaretskii <eliz@gnu.org>
>> Date: Sat, 11 Mar 2023 01:58:43 +0300
>>
>> Yes, broken since 23b6cd41f55b833e8a562c51642d36b5211a510b, the patch is
>> below.
>>
>> Eli, would this be OK for emacs-29?  This is clearly a regression.
>
> It's a regression that existed since Emacs 28.1, right?  So its fix
> should try to avoid changing APIs.  Therefore, please try to make a
> change that leaves the signature of epa-ks--query-url unchanged, I
> think it should be easy in this case.  (Yes, I know it's an internal
> function, but that doesn't mean we can change the signature when we
> can avoid that.)

Yes, since 28.1.  The updated patch is below.

But why an internal function is part of API?  Don't we assume that
anyone using them is doing so at their own risk?  In this case the
default for operation is "index" (to preserve compatibility), but it's
better here to not have the default value at all.

> P.S. And why are you posting patches and their discussion here, not on
> the bug tracker?

Because OP wrote here?  And we didn't open a bug for this.  Should we,
even for such simple fixes?

Thanks.

diff --git a/lisp/epa-ks.el b/lisp/epa-ks.el
index 77d896fa438..015bf910ac6 100644
--- a/lisp/epa-ks.el
+++ b/lisp/epa-ks.el
@@ -140,8 +140,8 @@ 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.
+(defun epa-ks--query-url (query exact &optional operation)
+  "Return URL for QUERY and OPERATION (defaults to \"index\").
 If EXACT is non-nil, don't accept approximate matches."
   (format "https://%s/pks/lookup?%s"
           (cond ((null epa-keyserver)
@@ -154,13 +154,13 @@ epa-ks--query-url
           (url-build-query-string
            (append `(("search" ,query)
                      ("options" "mr")
-                     ("op" "index"))
+                     ("op" ,(or operation "index")))
                    (and exact '(("exact" "on")))))))
 
 (defun epa-ks--fetch-key (id)
   "Send request to import key with specified ID."
   (url-retrieve
-   (epa-ks--query-url (concat "0x" (url-hexify-string id)) t)
+   (epa-ks--query-url (concat "0x" (url-hexify-string id)) t "get")
    (lambda (status)
      (when (plist-get status :error)
        (error "Request failed: %s"
@@ -236,7 +236,7 @@ epa-search-keys
         (erase-buffer))
       (epa-ks-search-mode))
     (url-retrieve
-     (epa-ks--query-url query exact)
+     (epa-ks--query-url query exact "index")
      (lambda (status)
        (when (plist-get status :error)
          (when buf

Re: problems importing keys via epa-search-keys

[Eli Zaretskii]

> From: Filipp Gunbin <fgunbin@fastmail.fm>
> Cc: help-gnu-emacs@gnu.org
> Date: Sun, 12 Mar 2023 02:04:21 +0300
> 
> On 11/03/2023 09:17 +0200, Eli Zaretskii wrote:
> 
> > It's a regression that existed since Emacs 28.1, right?  So its fix
> > should try to avoid changing APIs.  Therefore, please try to make a
> > change that leaves the signature of epa-ks--query-url unchanged, I
> > think it should be easy in this case.  (Yes, I know it's an internal
> > function, but that doesn't mean we can change the signature when we
> > can avoid that.)
> 
> Yes, since 28.1.  The updated patch is below.
> 
> But why an internal function is part of API?  Don't we assume that
> anyone using them is doing so at their own risk?  In this case the
> default for operation is "index" (to preserve compatibility), but it's
> better here to not have the default value at all.

I agree that we can break the API if we need, but we don't need to,
and it is always better to keep the signature unless it's impossible.

So please modify your patch to leave the signature intact, I don't
think it's hard in this case.

> > P.S. And why are you posting patches and their discussion here, not on
> > the bug tracker?
> 
> Because OP wrote here?  And we didn't open a bug for this.  Should we,
> even for such simple fixes?

It's better, because then the discussion is recorded and referenced in
the commit.

Re: problems importing keys via epa-search-keys

[Giovanni Biscuolo]

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

Hi Filipp

Filipp Gunbin <fgunbin@fastmail.fm> writes:

> Hi Giovanni,
>
> On 11/03/2023 08:05 +0100, Giovanni Biscuolo wrote:
>
> [...]
>
>> I'm using Guix so I'll try to apply your patch for Emacs 28.2 to that
>> package and submit it for inclusion in official Guix distribution.
>
> No need for that, I'll install it either on master or on emacs-29,
> depending on what Eli says.

Fine thanks, but I'd backport that patch to 28.2 in Guix, also.

>> AFAIU there's no bug report for this, or am I wrong?  In case there
>> isn't one I'm going to submit a new bug report.
>
> No need for that either.  But yes, it's better to submit problems using
> M-x report-emacs-bug, for the future.

I was not sure it was a bug but next time I'm going to use it, thanks!

Happy hacking, Gio'.

-- 
Giovanni Biscuolo

Xelera IT Infrastructures

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 849 bytes --]

Re: problems importing keys via epa-search-keys

[Filipp Gunbin]

On 12/03/2023 08:33 +0200, Eli Zaretskii wrote:

>> From: Filipp Gunbin <fgunbin@fastmail.fm>
>> Cc: help-gnu-emacs@gnu.org
>> Date: Sun, 12 Mar 2023 02:04:21 +0300
>> 
>> On 11/03/2023 09:17 +0200, Eli Zaretskii wrote:
>> 
>> > It's a regression that existed since Emacs 28.1, right?  So its fix
>> > should try to avoid changing APIs.  Therefore, please try to make a
>> > change that leaves the signature of epa-ks--query-url unchanged, I
>> > think it should be easy in this case.  (Yes, I know it's an internal
>> > function, but that doesn't mean we can change the signature when we
>> > can avoid that.)
>> 
>> Yes, since 28.1.  The updated patch is below.
>> 
>> But why an internal function is part of API?  Don't we assume that
>> anyone using them is doing so at their own risk?  In this case the
>> default for operation is "index" (to preserve compatibility), but it's
>> better here to not have the default value at all.
>
> I agree that we can break the API if we need, but we don't need to,
> and it is always better to keep the signature unless it's impossible.

Thanks, noted.

> So please modify your patch to leave the signature intact, I don't
> think it's hard in this case.

But I did in the last patch?  There's now an optional parameter, which
preserves API compatibility.  Or did you mean something else?

For convenience, I post the patch again below.

I still need your decision where to install this - master or emacs-29.

>> > P.S. And why are you posting patches and their discussion here, not on
>> > the bug tracker?
>> 
>> Because OP wrote here?  And we didn't open a bug for this.  Should we,
>> even for such simple fixes?
>
> It's better, because then the discussion is recorded and referenced in
> the commit.

Noted.

Thanks.


diff --git a/lisp/epa-ks.el b/lisp/epa-ks.el
index 77d896fa438..015bf910ac6 100644
--- a/lisp/epa-ks.el
+++ b/lisp/epa-ks.el
@@ -140,8 +140,8 @@ 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.
+(defun epa-ks--query-url (query exact &optional operation)
+  "Return URL for QUERY and OPERATION (defaults to \"index\").
 If EXACT is non-nil, don't accept approximate matches."
   (format "https://%s/pks/lookup?%s"
           (cond ((null epa-keyserver)
@@ -154,13 +154,13 @@ epa-ks--query-url
           (url-build-query-string
            (append `(("search" ,query)
                      ("options" "mr")
-                     ("op" "index"))
+                     ("op" ,(or operation "index")))
                    (and exact '(("exact" "on")))))))
 
 (defun epa-ks--fetch-key (id)
   "Send request to import key with specified ID."
   (url-retrieve
-   (epa-ks--query-url (concat "0x" (url-hexify-string id)) t)
+   (epa-ks--query-url (concat "0x" (url-hexify-string id)) t "get")
    (lambda (status)
      (when (plist-get status :error)
        (error "Request failed: %s"
@@ -236,7 +236,7 @@ epa-search-keys
         (erase-buffer))
       (epa-ks-search-mode))
     (url-retrieve
-     (epa-ks--query-url query exact)
+     (epa-ks--query-url query exact "index")
      (lambda (status)
        (when (plist-get status :error)
          (when buf

Re: problems importing keys via epa-search-keys

[Eli Zaretskii]

> From: Filipp Gunbin <fgunbin@fastmail.fm>
> Cc: help-gnu-emacs@gnu.org
> Date: Mon, 13 Mar 2023 15:37:31 +0300
> 
> > So please modify your patch to leave the signature intact, I don't
> > think it's hard in this case.
> 
> But I did in the last patch?  There's now an optional parameter, which
> preserves API compatibility.  Or did you mean something else?

I meant to have the EXACT argument accept additional values, which
will allow us to leave the signature alone.  But perhaps your
suggestion is also OK.

> I still need your decision where to install this - master or emacs-29.

emacs-29 is fine.

Re: problems importing keys via epa-search-keys

[Filipp Gunbin]

On 13/03/2023 16:43 +0200, Eli Zaretskii wrote:

>> From: Filipp Gunbin <fgunbin@fastmail.fm>
>> Cc: help-gnu-emacs@gnu.org
>> Date: Mon, 13 Mar 2023 15:37:31 +0300
>>
>> > So please modify your patch to leave the signature intact, I don't
>> > think it's hard in this case.
>>
>> But I did in the last patch?  There's now an optional parameter, which
>> preserves API compatibility.  Or did you mean something else?
>
> I meant to have the EXACT argument accept additional values, which
> will allow us to leave the signature alone.  But perhaps your
> suggestion is also OK.

I'll leave it as I did, because "exact" is a separate variable in spec
(https://datatracker.ietf.org/doc/html/draft-shaw-openpgp-hkp-00#section-3.2.3),
so including other semantics in it would be confusing.

>> I still need your decision where to install this - master or emacs-29.
>
> emacs-29 is fine.

Thanks, installed.