[PATCH] Possible bug in org-protocol

[PATCH] Possible bug in org-protocol

[Pierre Téchoueyres]

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


Hello Org developers,

I'm facing an unexpected behaviour since some days :
When I want to record some link from web sites using Org Capture extension
(https://github.com/sprig/org-capture-extension) for Firefox all my text
and links are encoded like urls.

It's look like the commit #cc7c4a673 removed some url decoding when
the org-protocol url is processed.

See bellow the steps to reproduce :

1) define some templates for org-protocol, mine are bellow 
#+begin_src elisp
(setq org-capture-templates '(("L" "Protocol Link" entry (file+olp ,inbox "Web")
                                ,(concat "* %:description :@website:\n"
                                         "  Captured On: %a, %U\n"
                                         "  %?\n\n"))
                               ("p" "Protocol Selection" entry (file+olp ,inbox "Web")
                                ,(concat "* %:description :@website:\n"
                                         "  Source:  %a, %U\n"
                                         "  #+begin_quote\n%i\n  #+end_quote\n"
                                         "  %?\n\n"))))
#+end_src

2) use emacsclient to trigger org-protocol like this
#+begin_src sh
emacsclient -n "org-protocol://capture?template=p&url=https%3A%2F%2Fgithub.com%2FCruiserX%2Fsha256_plsql&title=CruiserX%2Fsha256_plsql%3A%20SHA256%20PL%2FSQL%20Implementation%20for%20Oracle%2010g%2C11g.&body=SHA256%20PL%2FSQL%20Implementation%20for%20Oracle%2010g%2C11g.%20"
#+end_src


the resulting buffer is :
#+begin_example
** CruiserX%2Fsha256_plsql%3A%20SHA256%20PL%2FSQL%20Implementation%20for%20Oracle%2010g%2C11g. :@website:
   Source:  [[https%253A%252F%252Fgithub.com%252FCruiserX%252Fsha256_plsql][CruiserX%2Fsha256_plsql%3A%20SHA256%20PL%2FSQL%20Implementation%20for%20Oracle%2010g%2C11g.]], [2019-03-07 jeu. 23:36]
   #+begin_quote
 SHA256%20PL%2FSQL%20Implementation%20for%20Oracle%2010g%2C11g.%20
   #+end_quote
#+end_example

I think that one possible fix could be to rollback one part of the git commit #cc7c4a673 like
in the patch bellow.

What do you think ?

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Fix org-protocol unescape --]
[-- Type: text/x-patch, Size: 488 bytes --]

diff --git a/lisp/org-protocol.el b/lisp/org-protocol.el
index f3e7281ee..d9fdef00e 100644
--- a/lisp/org-protocol.el
+++ b/lisp/org-protocol.el
@@ -381,7 +381,7 @@ If INFO is already a property list, return it unchanged."
 	  (while data
 	    (setq result
 		  (append result
-			  (list (pop data) (pop data)))))
+			  (list (pop data) (org-link-unescape (pop data))))))
 	  result)
       (let ((data (org-protocol-split-data info t org-protocol-data-separator)))
 	(if default-order

[-- Attachment #3: Type: text/plain, Size: 9 bytes --]


Pierre.

Re: [PATCH] Possible bug in org-protocol

[Nicolas Goaziou]

Hello,

Pierre Téchoueyres <pierre.techoueyres@free.fr> writes:

> I'm facing an unexpected behaviour since some days :
> When I want to record some link from web sites using Org Capture extension
> (https://github.com/sprig/org-capture-extension) for Firefox all my text
> and links are encoded like urls.
>
> It's look like the commit #cc7c4a673 removed some url decoding when
> the org-protocol url is processed.
>
> See bellow the steps to reproduce :
>
> 1) define some templates for org-protocol, mine are bellow 
>
> #+begin_src elisp
> (setq org-capture-templates '(("L" "Protocol Link" entry (file+olp ,inbox "Web")
>                                 ,(concat "* %:description :@website:\n"
>                                          "  Captured On: %a, %U\n"
>                                          "  %?\n\n"))
>                                ("p" "Protocol Selection" entry (file+olp ,inbox "Web")
>                                 ,(concat "* %:description :@website:\n"
>                                          "  Source:  %a, %U\n"
>                                          "  #+begin_quote\n%i\n  #+end_quote\n"
>                                          "  %?\n\n"))))
> #+end_src
>
>
> 2) use emacsclient to trigger org-protocol like this
>
> #+begin_src sh
> emacsclient -n "org-protocol://capture?template=p&url=https%3A%2F%2Fgithub.com%2FCruiserX%2Fsha256_plsql&title=CruiserX%2Fsha256_plsql%3A%20SHA256%20PL%2FSQL%20Implementation%20for%20Oracle%2010g%2C11g.&body=SHA256%20PL%2FSQL%20Implementation%20for%20Oracle%2010g%2C11g.%20"
> #+end_src

Why is the link percent encoded in the first place? Is it necessary?

Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Possible bug in org-protocol

[Pierre Téchoueyres]

Hi Nicolas,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

>>
>> 2) use emacsclient to trigger org-protocol like this
>>
>> #+begin_src sh
>> emacsclient -n "org-protocol://capture?template=p&url=https%3A%2F%2Fgithub.com%2FCruiserX%2Fsha256_plsql&title=CruiserX%2Fsha256_plsql%3A%20SHA256%20PL%2FSQL%20Implementation%20for%20Oracle%2010g%2C11g.&body=SHA256%20PL%2FSQL%20Implementation%20for%20Oracle%2010g%2C11g.%20"
>> #+end_src
>
> Why is the link percent encoded in the first place? Is it necessary?

I think so, as org-protocol, for what I've  understood, is triggered by
an URL where protocol is org-protocol. So the content of the URL itself
must be encoded.

> Regards,
Regards,

Pierre.

Re: [PATCH] Possible bug in org-protocol

[Nicolas Goaziou]

Hello,

pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:

> I think so, as org-protocol, for what I've  understood, is triggered by
> an URL where protocol is org-protocol. So the content of the URL itself
> must be encoded.

OK. I applied you fix. Thank you.

Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Possible bug in org-protocol

[Pierre Téchoueyres]

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

Hello Nicolas,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

> Hello,
>
> pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:
>
>> I think so, as org-protocol, for what I've  understood, is triggered by
>> an URL where protocol is org-protocol. So the content of the URL itself
>> must be encoded.
>
> OK. I applied you fix. Thank you.

Thanks for your quick action. I've also written another test for
this patch.  Would you mind adding it too ?

As a bonus it show the manual paragraph explaining the neccessity to
encode the parts of url befor calling org-protocol.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Test for org-protocol unescape --]
[-- Type: text/x-patch, Size: 1113 bytes --]

diff --git a/testing/lisp/test-org-protocol.el b/testing/lisp/test-org-protocol.el
index fc764386c..6ee368dcf 100644
--- a/testing/lisp/test-org-protocol.el
+++ b/testing/lisp/test-org-protocol.el
@@ -35,6 +35,16 @@
   (let ((data (org-protocol-parse-parameters "url=abc&title=def" t)))
     (should (string= (plist-get data :url) "abc"))
     (should (string= (plist-get data :title) "def")))
+  ;; Parse new-style complex links
+  (let* ((url (concat "template=p&"
+		      "url=https%3A%2F%2Forgmode.org%2Forg.html%23capture-protocol&"
+		      "title=The%20Org%20Manual&"
+		      "body=9.4.2%20capture%20protocol"))
+	 (data (org-protocol-parse-parameters url)))
+    (should (string= (plist-get data :template) "p"))
+    (should (string= (plist-get data :url) "https://orgmode.org/org.html#capture-protocol"))
+    (should (string= (plist-get data :title) "The Org Manual"))
+    (should (string= (plist-get data :body) "9.4.2 capture protocol")))
   ;; Parse old-style links
   (let ((data (org-protocol-parse-parameters "abc/def" nil '(:url :title))))
     (should (string= (plist-get data :url) "abc"))

[-- Attachment #3: Type: text/plain, Size: 118 bytes --]


Regards,

P.S.
Is simple diffs an acceptable format for patches or should I send you an
fully formatted git commit ?

Re: [PATCH] Possible bug in org-protocol

[Nicolas Goaziou]

Hello,

pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:

> Thanks for your quick action. I've also written another test for
> this patch.  Would you mind adding it too ?

Thank you for the patch.

> Is simple diffs an acceptable format for patches or should I send you an
> fully formatted git commit ?

The latter, please.

Could you send a complete version of the patch?

Regards,

-- 
Nicolas Goaziou

Re: [PATCH] Possible bug in org-protocol

[Pierre Téchoueyres]

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


Hello Nicolas,

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:
> ...
>> Is simple diffs an acceptable format for patches or should I send you an
>> fully formatted git commit ?
>
> The latter, please.
>
> Could you send a complete version of the patch?

What dou you think of the attached one ?

Regards,

Pierre


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: tests for org-protocol url decoding --]
[-- Type: text/x-patch, Size: 1648 bytes --]

From 92db52eb3e546ed03f0ade483a6cb149300ca6c5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pierre=20T=C3=A9choueyres?= <pierre.techoueyres@free.fr>
Date: Sun, 10 Mar 2019 17:50:22 +0100
Subject: [PATCH] org-protocol: Fix URL handling -- add tests

* testing/lisp/test-org-protocol.el (test-org-protocol/org-protocol-parse-parameters):
  add test for the commit e0bfdec22 which un-escape extracted link as
  it is url-encoded externally.
---
 testing/lisp/test-org-protocol.el | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/testing/lisp/test-org-protocol.el b/testing/lisp/test-org-protocol.el
index fc764386c..6ee368dcf 100644
--- a/testing/lisp/test-org-protocol.el
+++ b/testing/lisp/test-org-protocol.el
@@ -35,6 +35,16 @@
   (let ((data (org-protocol-parse-parameters "url=abc&title=def" t)))
     (should (string= (plist-get data :url) "abc"))
     (should (string= (plist-get data :title) "def")))
+  ;; Parse new-style complex links
+  (let* ((url (concat "template=p&"
+		      "url=https%3A%2F%2Forgmode.org%2Forg.html%23capture-protocol&"
+		      "title=The%20Org%20Manual&"
+		      "body=9.4.2%20capture%20protocol"))
+	 (data (org-protocol-parse-parameters url)))
+    (should (string= (plist-get data :template) "p"))
+    (should (string= (plist-get data :url) "https://orgmode.org/org.html#capture-protocol"))
+    (should (string= (plist-get data :title) "The Org Manual"))
+    (should (string= (plist-get data :body) "9.4.2 capture protocol")))
   ;; Parse old-style links
   (let ((data (org-protocol-parse-parameters "abc/def" nil '(:url :title))))
     (should (string= (plist-get data :url) "abc"))
-- 
2.20.1

Re: [PATCH] Possible bug in org-protocol

[Nicolas Goaziou]

pierre.techoueyres@free.fr (Pierre Téchoueyres) writes:

> Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

>> Could you send a complete version of the patch?
>
> What dou you think of the attached one ?

Perfect.

Applied. Thank you.