GNU bug report logs - #70312
[PATCH] Avoid unnecessary escaping in url-build-query-string

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 70312 in the body.
You can then email your comments to 70312 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Dagfinn Ilmari Mannsåker <ilmari <at> ilmari.org>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Avoid unnecessary escaping in url-build-query-string
Date: Tue, 09 Apr 2024 15:29:00 +0100

[Message part 1 (text/plain, inline)]

Hi,

While writing a custom URL function for git-link.el I noticed that
url-build-query-string unnecessarily escapes slashes in query parameter
values.  Here's a patch that fixes that by passing
url-query-allowed-chars to the url-hexify-string call.

- ilmari

[0001-Avoid-unnecessary-escaping-in-url-build-query-string.patch (text/x-diff, inline)]

From ca1dbe67939ac78f5db06d746cd511928a138657 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari <at> ilmari.org>
Date: Tue, 9 Apr 2024 15:02:45 +0100
Subject: [PATCH] Avoid unnecessary escaping in url-build-query-string

* lisp/url/url-util.el (url-build-query-string):
Pass url-query-allowed-chars to url-hexify-string to avoid unnecessarily
escaping characters that don't need to be escaped in a query string.
---
 lisp/url/url-util.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/url/url-util.el b/lisp/url/url-util.el
index 5f45b98c7a5..4fc0efcdf62 100644
--- a/lisp/url/url-util.el
+++ b/lisp/url/url-util.el
@@ -268,7 +268,8 @@ instead of just \"key\" as in the example above."
    (lambda (key-vals)
      (let ((escaped
             (mapcar (lambda (sym)
-                      (url-hexify-string (format "%s" sym))) key-vals)))
+                      (url-hexify-string (format "%s" sym) url-query-allowed-chars))
+                    key-vals)))
        (mapconcat (lambda (val)
                     (let ((vprint (format "%s" val))
                           (eprint (format "%s" (car escaped))))
-- 
2.39.2

Message #8 received at 70312 <at> debbugs.gnu.org (full text, mbox):

From: Dagfinn Ilmari Mannsåker <ilmari <at> ilmari.org>
To: 70312 <at> debbugs.gnu.org
Subject: [PATCH v2] Avoid unnecessary escaping in url-build-query-string
Date: Tue, 09 Apr 2024 17:41:07 +0100

[Message part 1 (text/plain, inline)]

Hi again,

I realised I'd forgotten to add tests, and that made me realise that
url-query-allowed-chars is not correct for this, since that also
contains '=', '&', and ';'. So here's an updated patch, which creates a
new url-query-key-value-allowed-chars constant, which is
url-query-allowed-chars minus the aforementioned three chars, and adds
tests covering that, for both keys and values.

- ilmari

[v2-0001-Avoid-unnecessary-escaping-in-url-build-query-str.patch (text/x-diff, inline)]

From 89db0a1226d8d7cca1846e9c737d4a67c971ec75 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari <at> ilmari.org>
Date: Tue, 9 Apr 2024 15:02:45 +0100
Subject: [PATCH v2] Avoid unnecessary escaping in url-build-query-string

* lisp/url/url-util.el (url-build-query-string):
Create a new url-query-key-value-allowed-chars constant and pass that to
url-hexify-string to avoid unnecessarily escaping characters that don't
need to be escaped in query string keys and values.
* test/lisp/url/url-util-tests.el (url-util-tests):
Add test cases.
---
 lisp/url/url-util.el            | 12 +++++++++++-
 test/lisp/url/url-util-tests.el |  6 +++++-
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/lisp/url/url-util.el b/lisp/url/url-util.el
index 5f45b98c7a5..f063efe18a6 100644
--- a/lisp/url/url-util.el
+++ b/lisp/url/url-util.el
@@ -268,7 +268,8 @@ url-build-query-string
    (lambda (key-vals)
      (let ((escaped
             (mapcar (lambda (sym)
-                      (url-hexify-string (format "%s" sym))) key-vals)))
+                      (url-hexify-string (format "%s" sym) url-query-key-value-allowed-chars))
+                    key-vals)))
        (mapconcat (lambda (val)
                     (let ((vprint (format "%s" val))
                           (eprint (format "%s" (car escaped))))
@@ -410,6 +411,15 @@ url-query-allowed-chars
   "Allowed-character byte mask for the query segment of a URI.
 These characters are specified in RFC 3986, Appendix A.")
 
+(defconst url-query-key-value-allowed-chars
+  (let ((vec (copy-sequence url-query-allowed-chars)))
+    (aset vec ?= nil)
+    (aset vec ?& nil)
+    (aset vec ?\; nil)
+    vec)
+  "Allowed-charcter byte mask for keys and values in the query segment of a URI.
+url-query-allowed-chars minus '=', '&', and ';'.")
+
 ;;;###autoload
 (defun url-encode-url (url)
   "Return a properly URI-encoded version of URL.
diff --git a/test/lisp/url/url-util-tests.el b/test/lisp/url/url-util-tests.el
index 133aa0ffd88..c6246d69a2a 100644
--- a/test/lisp/url/url-util-tests.el
+++ b/test/lisp/url/url-util-tests.el
@@ -32,7 +32,11 @@ url-util-tests
            ("key1=val1;key2=val2;key3=val1;key3=val2;key4;key5"
             ((key1 "val1") (key2 val2) (key3 val1 val2) ("key4") (key5 "")) t)
            ("key1=val1;key2=val2;key3=val1;key3=val2;key4=;key5="
-            ((key1 val1) (key2 val2) ("key3" val1 val2) (key4) (key5 "")) t t)))
+            ((key1 val1) (key2 val2) ("key3" val1 val2) (key4) (key5 "")) t t)
+           ("key1=val/slash;key2=val%3Bsemi;key3=val%26amp;key4=val%3Deq"
+            ((key1 "val/slash") (key2 "val;semi") (key3 "val&amp") (key4 "val=eq")) t)
+           ("key%3Deq=val1;key%3Bsemi=val2;key%26amp=val3"
+            (("key=eq" val1) ("key;semi" val2) ("key&amp" val3)) t)))
         test)
     (while tests
       (setq test (car tests)
-- 
2.39.2

Message #13 received at 70312-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dagfinn Ilmari Mannsåker <ilmari <at> ilmari.org>
Cc: 70312-done <at> debbugs.gnu.org
Subject: Re: bug#70312: [PATCH v2] Avoid unnecessary escaping in
 url-build-query-string
Date: Thu, 18 Apr 2024 13:10:57 +0300

> From: Dagfinn Ilmari Mannsåker <ilmari <at> ilmari.org>
> Date: Tue, 09 Apr 2024 17:41:07 +0100
> 
> I realised I'd forgotten to add tests, and that made me realise that
> url-query-allowed-chars is not correct for this, since that also
> contains '=', '&', and ';'. So here's an updated patch, which creates a
> new url-query-key-value-allowed-chars constant, which is
> url-query-allowed-chars minus the aforementioned three chars, and adds
> tests covering that, for both keys and values.

Thanks, I installed this on the master branch, and I'm closing this
bug.

With this contribution, you have exhausted the maximum amount of code
changes we can accept from you without a copyright assignment.  Would
you like to start your assignment paperwork now, so that we could
accept your future contributions without limitations?  If yes, I will
send you a form to fill and the instructions to go with it.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.