fix: escape and url-encode embark field values; respect user --type=

spot-embark.el

@@ -41,23 +41,32 @@
     (switch-to-buffer buf)
     (goto-char (point-min))))
 
+(defun spot--quote-field-value (value)
+  "Wrap VALUE in double quotes for use in a Spotify field filter.
+Embedded double quotes are stripped, since Spotify search syntax
+defines no escape sequence."
+  (concat "\"" (replace-regexp-in-string "\"" "" value) "\""))
+
 (defun spot-action--list-album-tracks (item)
   "Search for tracks on the album represented by ITEM."
   (let* ((table (get-text-property 0 'multi-data item))
          (album-name (ht-get* table 'name))
          (artist-name (ht-get* (nth 0 (ht-get* table 'artists)) 'name)))
     (spot-consult-search
      (concat
-      "album:" album-name
+      "album:" (spot--quote-field-value album-name)
       " "
-      "artist:" artist-name " -- --type=track"))))
+      "artist:" (spot--quote-field-value artist-name)
+      " -- --type=track"))))
 
 (defun spot-action--list-artist-tracks (item)
   "Search for tracks by the artist represented by ITEM."
   (let* ((table (get-text-property 0 'multi-data item))
          (artist-name (ht-get* table 'name)))
     (spot-consult-search
-     (concat "artist:" artist-name " -- --type=track"))))
+     (concat
+      "artist:" (spot--quote-field-value artist-name)
+      " -- --type=track"))))
 
 (defun spot-action--list-playlist-tracks (item)
   "List tracks in the playlist represented by ITEM."

spot-search.el

@@ -25,6 +25,7 @@
 
 (require 'ht)
 (require 'dash)
+(require 'url-util)
 
 (require 'spot-util)
 (require 'spot-var)
@@ -61,23 +62,42 @@
 (defvar spot--candidates-audiobook '()
   "Current audiobook candidates from the most recent search.")
 
+(defun spot--find-args-separator (input)
+  "Return the start index of the args separator in INPUT, or nil.
+The separator is whitespace, \"--\", whitespace.  Occurrences
+inside double-quoted regions of INPUT are skipped."
+  (let ((i 0) (len (length input)) (in-quote nil) (pos nil))
+    (while (and (null pos) (< i len))
+      (let ((c (aref input i)))
+        (cond
+         ((eq c ?\")
+          (setq in-quote (not in-quote))
+          (setq i (1+ i)))
+         ((and (not in-quote)
+               (eq (string-match "\\s-+--\\s-+" input i) i))
+          (setq pos i))
+         (t (setq i (1+ i))))))
+    pos))
+
 (defun spot--parse-command (input)
   "Parse INPUT into a query and optional arguments.
 Returns a list of (QUERY ARGS) where ARGS is an alist.
 Arguments are separated from the query by \" -- \" and are
-in the form \"--key=value\"."
-  (if (string-match "\\(.*?\\)\\s-+--\\s-+\\(.*\\)" input)
-      (let* ((query (match-string 1 input))
-             (args-str (match-string 2 input))
-             (args (mapcar
-                    (lambda (arg)
-                      (when (string-match
-                             "\\(?:--\\)?\\([^=]+\\)=\\(.*\\)" arg)
-                        (cons (match-string 1 arg)
-                              (match-string 2 arg))))
-                    (split-string args-str))))
-        (list query (delq nil args)))
-    (list input nil)))
+in the form \"--key=value\".  Occurrences of \" -- \" inside
+double-quoted regions of INPUT are treated as part of the query."
+  (let ((sep (spot--find-args-separator input)))
+    (if (and sep (string-match "\\s-+--\\s-+" input sep))
+        (let* ((query (substring input 0 sep))
+               (args-str (substring input (match-end 0)))
+               (args (mapcar
+                      (lambda (arg)
+                        (when (string-match
+                               "\\(?:--\\)?\\([^=]+\\)=\\(.*\\)" arg)
+                          (cons (match-string 1 arg)
+                                (match-string 2 arg))))
+                      (split-string args-str))))
+          (list query (delq nil args)))
+      (list input nil))))
 
 (defun spot--transform-alist-to-q-params (alist)
   "Transform ALIST into URL query parameter string."
@@ -88,19 +108,26 @@ in the form \"--key=value\"."
          pairs "")
       "")))
 
+(defun spot--build-search-q-params (parsed-command)
+  "Build the search URL query parameter string for PARSED-COMMAND.
+PARSED-COMMAND is the result of `spot--parse-command'.  A default
+`type' covering every supported item kind is included unless the
+user supplied their own `type' argument."
+  (let* ((query (car parsed-command))
+         (args-alist (cadr parsed-command))
+         (args (spot--transform-alist-to-q-params (cdr parsed-command)))
+         (default-types
+          (unless (assoc "type" args-alist)
+            "&type=album,artist,playlist,track,show,episode,audiobook")))
+    (concat "?q=" (url-hexify-string query)
+            (or default-types "")
+            args)))
+
 (defun spot--search-items (input)
   "Search for items on Spotify based on INPUT.
 Returns a hash table of search results."
-  (let* ((parsed-command (spot--parse-command input))
-         (query (car parsed-command))
-         (args (cdr parsed-command))
-         (args (spot--transform-alist-to-q-params args))
-         (q-params (concat
-                    "?type=" "album," "artist,"
-                    "playlist," "track," "show," "episode,"
-                    "audiobook"
-                    "&q=" query
-                    args))
+  (let* ((q-params (spot--build-search-q-params
+                    (spot--parse-command input)))
          (alist (spot-request
                  :method "GET"
                  :url spot-search-url

test/spot-test.el

@@ -94,6 +94,51 @@
     (should (equal (car result) "jazz"))
     (should (= (length (cadr result)) 2))))
 
+(ert-deftest spot-test-parse-command/quoted-field-with-separator ()
+  "A \" -- \" inside a double-quoted field is part of the query."
+  (let ((result (spot--parse-command
+                 "album:\"Foo -- Live\" artist:\"Bar\" -- --type=track")))
+    (should (equal (car result) "album:\"Foo -- Live\" artist:\"Bar\""))
+    (should (equal (cadr result) '(("type" . "track"))))))
+
+(ert-deftest spot-test-parse-command/quoted-field-no-args ()
+  "A quoted field with an internal \" -- \" but no real args separator."
+  (let ((result (spot--parse-command "album:\"Foo -- Live\"")))
+    (should (equal (car result) "album:\"Foo -- Live\""))
+    (should-not (cadr result))))
+
+(ert-deftest spot-test-parse-command/quoted-field-with-spaces ()
+  "Spaces inside a quoted field are preserved in the query."
+  (let ((result (spot--parse-command
+                 "artist:\"Pink Floyd\" -- --type=track")))
+    (should (equal (car result) "artist:\"Pink Floyd\""))
+    (should (equal (cadr result) '(("type" . "track"))))))
+
+
+;;; spot--build-search-q-params
+
+(ert-deftest spot-test-build-search-q-params/default-types-when-no-args ()
+  "The default type list is included when no args are given."
+  (let ((q (spot--build-search-q-params '("radiohead" nil))))
+    (should (string-match-p "q=radiohead" q))
+    (should (string-match-p
+             "&type=album,artist,playlist,track,show,episode,audiobook"
+             q))))
+
+(ert-deftest spot-test-build-search-q-params/user-type-replaces-default ()
+  "A user-supplied type arg replaces the default type list."
+  (let ((q (spot--build-search-q-params
+            '("foo" (("type" . "track"))))))
+    (should-not (string-match-p "type=album," q))
+    (should (string-match-p "&type=track" q))))
+
+(ert-deftest spot-test-build-search-q-params/non-type-args-keep-default ()
+  "Non-type args don't suppress the default type list."
+  (let ((q (spot--build-search-q-params
+            '("foo" (("market" . "US"))))))
+    (should (string-match-p "&type=album," q))
+    (should (string-match-p "&market=US" q))))
+
 
 ;;; spot--filter and spot--type-equals