[Bug] org-protocol does not kill client process with org-protocol-capture

[Bug] org-protocol does not kill client process with org-protocol-capture

[Matt Lundin]

I would like to report two bugs in org-protocol.

1. The call to (server-edit) on line 626 of org-protocol.el does not
succeed in terminating the server process or signaling to the client to
stop waiting for org-protocol-capture (despite the :kill-server
property). This can have potentially dangerous side-effects when calling
org-protocol from a web browser. On Linux, for instance, if I call
org-protocol-capture via a bookmark from Chromium or Firefox and then
abort the capture within emacs (with "q" in the capture buffer or with
"C-g"), xdg receives an error signal and then tries to send the command
to another application. In my case, because of the default xdg settings,
this leads to a search being run in firefox that contains the entire
org-protocol string (thus raising the possibility of leaking private
information to a third-party server).

A long time ago (in 2009) server-delete-client was replaced by
server-edit (commit 13fe6b1858033eca882c38dd8bc422c0c4c6bb9b):

--8<---------------cut here---------------start------------->8---
-                  (if (plist-get (cdr prolist) :kill-client)
-                      (server-delete-client client t))
+                  (when (plist-get (cdr prolist) :kill-client)
+		    (message "Greedy org-protocol handler. Killing client.")
+		    (server-edit))
--8<---------------cut here---------------end--------------->8---

Does anyone remember the reason for change? I tried server-delete-client
and it succeeds in killing the process and signaling to client to stop
waiting. By contrast, server-edit does not successfully kill the
process.

2. The condition-case added with commit
25eb14bc2cadab293e200c018284870a1eeb4490 on December 2, 2015 (lines
631-635) causes org-protocol-capture to run twice when the process is
aborted, warning one (often incorrectly) to use new-style links. This is
because typing "q" in the capture buffer signals an error, which
triggers the condition case, leading to the warning and causing
org-protocol-capture to be run a second time with the false warning.

Here are the steps to replicate these bugs:

1. /usr/bin/emacs -Q -l minimal.el

...where minimal contains the following

--8<---------------cut here---------------start------------->8---
(add-to-list 'load-path "~/org-mode/lisp")
(add-to-list 'load-path "~/org-mode/contrib/lisp")

(require 'org-protocol)

(setq org-capture-templates
      '(("n" "A note" entry (file "~/test.org") "* %a\n %U\n %?\n %i")))

(server-start)
--8<---------------cut here---------------end--------------->8---

2. open a terminal and enter the following:

emacsclient "org-protocol://capture?url=http://orgmode.org/&title=Org mode for Emacs – Your Life in Plain Text&body=Testing"

3. abort the capture process with "q" or C-g

What happens:

1. The emacsclient continues to wait. The terminal reads: "Waiting for
Emacs..." (Bug #1)

Please note that using the "-n" flag with emacsclient will do nothing in
this context, since org-protocol is hijacks the server process before
the no-wait flag is processed.

2. Upon pressing C-g or q, a warning appears in Emacs ("Please update
your org protocol handler to deal with new-style links") and org-capture
runs again, requiring a second abort. (Bug #2)

3. emacsclient in the terminal outputs ("*ERROR*: Abort). (Bug #1)

What I would expect to happen:

When using the capture protocol, emacsclient would not wait and instead
would simply exit (because of the :kill-server property in
org-protocol-protocol-alist-default). Likewise, aborting a capture
process would not send an error message to the client.

Thanks,
Matt