* bug#70312: [PATCH] Avoid unnecessary escaping in url-build-query-string @ 2024-04-09 14:29 Dagfinn Ilmari Mannsåker 2024-04-09 16:41 ` bug#70312: [PATCH v2] " Dagfinn Ilmari Mannsåker 0 siblings, 1 reply; 4+ messages in thread From: Dagfinn Ilmari Mannsåker @ 2024-04-09 14:29 UTC (permalink / raw) To: 70312 [-- Attachment #1: Type: text/plain, Size: 261 bytes --] 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 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Avoid-unnecessary-escaping-in-url-build-query-string.patch --] [-- Type: text/x-diff, Size: 1180 bytes --] From ca1dbe67939ac78f5db06d746cd511928a138657 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* bug#70312: [PATCH v2] Avoid unnecessary escaping in url-build-query-string 2024-04-09 14:29 bug#70312: [PATCH] Avoid unnecessary escaping in url-build-query-string Dagfinn Ilmari Mannsåker @ 2024-04-09 16:41 ` Dagfinn Ilmari Mannsåker 2024-04-18 10:10 ` Eli Zaretskii 2024-05-19 11:18 ` Philip Kaludercic 0 siblings, 2 replies; 4+ messages in thread From: Dagfinn Ilmari Mannsåker @ 2024-04-09 16:41 UTC (permalink / raw) To: 70312 [-- Attachment #1: Type: text/plain, Size: 403 bytes --] 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 [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: v2-0001-Avoid-unnecessary-escaping-in-url-build-query-str.patch --] [-- Type: text/x-diff, Size: 2898 bytes --] From 89db0a1226d8d7cca1846e9c737d4a67c971ec75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@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&") (key4 "val=eq")) t) + ("key%3Deq=val1;key%3Bsemi=val2;key%26amp=val3" + (("key=eq" val1) ("key;semi" val2) ("key&" val3)) t))) test) (while tests (setq test (car tests) -- 2.39.2 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* bug#70312: [PATCH v2] Avoid unnecessary escaping in url-build-query-string 2024-04-09 16:41 ` bug#70312: [PATCH v2] " Dagfinn Ilmari Mannsåker @ 2024-04-18 10:10 ` Eli Zaretskii 2024-05-19 11:18 ` Philip Kaludercic 1 sibling, 0 replies; 4+ messages in thread From: Eli Zaretskii @ 2024-04-18 10:10 UTC (permalink / raw) To: Dagfinn Ilmari Mannsåker; +Cc: 70312-done > From: Dagfinn Ilmari Mannsåker <ilmari@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. ^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#70312: [PATCH v2] Avoid unnecessary escaping in url-build-query-string 2024-04-09 16:41 ` bug#70312: [PATCH v2] " Dagfinn Ilmari Mannsåker 2024-04-18 10:10 ` Eli Zaretskii @ 2024-05-19 11:18 ` Philip Kaludercic 1 sibling, 0 replies; 4+ messages in thread From: Philip Kaludercic @ 2024-05-19 11:18 UTC (permalink / raw) To: Dagfinn Ilmari Mannsåker; +Cc: 70312 Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> writes: > 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. This patch breaks a script I have that authenticates via HTTP. Apparently it doesn't escape enough now: (url-build-query-string '((var "\"$%&')+:;<>?@]^{|}"))) "var=%22$%%26')+:%3B%3C%3E?@%5D%5E%7B%7C%7D" whereas it used to be: (url-build-query-string '((var "\"$%&')+:;<>?@]^{|}"))) "var=%22%24%25%26%27%29%2B%3A%3B%3C%3E%3F%40%5D%5E%7B%7C%7D" If it is true, that it just unnecessarily escapes too much (and this is not a problem), then I'd suggest reverting the patch as the easiest solution to avoid breakage in the long term. > > - ilmari > >>From 89db0a1226d8d7cca1846e9c737d4a67c971ec75 Mon Sep 17 00:00:00 2001 > From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@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&") (key4 "val=eq")) t) > + ("key%3Deq=val1;key%3Bsemi=val2;key%26amp=val3" > + (("key=eq" val1) ("key;semi" val2) ("key&" val3)) t))) > test) > (while tests > (setq test (car tests) -- Philip Kaludercic on icterid ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-05-19 11:18 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-04-09 14:29 bug#70312: [PATCH] Avoid unnecessary escaping in url-build-query-string Dagfinn Ilmari Mannsåker 2024-04-09 16:41 ` bug#70312: [PATCH v2] " Dagfinn Ilmari Mannsåker 2024-04-18 10:10 ` Eli Zaretskii 2024-05-19 11:18 ` Philip Kaludercic
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.