Make rcirc parsing more RFC2812 compliant (Bug#36233)

Do continue to allow multiple spaces between arguments, even though
that is technically not allowed by the RFC.
* lisp/net/rcirc.el (rcirc-process-server-response-1): Fix parsing of
arguments which contain colons.
* test/lisp/net/rcirc-tests.el: New test.

2 files changed, 71 insertions, 8 deletions

diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
index 8926772b944..24084c828e1 100644
--- a/lisp/net/rcirc.el
+++ b/lisp/net/rcirc.el
@@ -774,22 +774,33 @@ Function is called with PROCESS, COMMAND, SENDER, ARGS and LINE.")
     (rcirc-process-server-response-1 process text)))
 
 (defun rcirc-process-server-response-1 (process text)
-  (if (string-match "^\\(:\\([^ ]+\\) \\)?\\([^ ]+\\) \\(.+\\)$" text)
+  ;; See https://tools.ietf.org/html/rfc2812#section-2.3.1.  We're a
+  ;; bit more accepting than the RFC: We allow any non-space
+  ;; characters in the command name, multiple spaces between
+  ;; arguments, and allow the last argument to omit the leading ":",
+  ;; even if there are less than 15 arguments.
+  (if (string-match "^\\(:\\([^ ]+\\) \\)?\\([^ ]+\\)" text)
       (let* ((user (match-string 2 text))
 	     (sender (rcirc-user-nick user))
              (cmd (match-string 3 text))
-             (args (match-string 4 text))
+             (cmd-end (match-end 3))
+             (args nil)
              (handler (intern-soft (concat "rcirc-handler-" cmd))))
-        (string-match "^\\([^:]*\\):?\\(.+\\)?$" args)
-        (let* ((args1 (match-string 1 args))
-               (args2 (match-string 2 args))
-               (args (delq nil (append (split-string args1 " " t)
-				       (list args2)))))
+        (cl-loop with i = cmd-end
+                 repeat 14
+                 while (eql i (string-match " +\\([^: ][^ ]*\\)" text i))
+                 do (progn (push (match-string 1 text) args)
+                           (setq i (match-end 0)))
+                 finally
+                 (progn (if (eql i (string-match " +:?" text i))
+                            (push (substring text (match-end 0)) args)
+                          (cl-assert (= i (length text))))
+                        (cl-callf nreverse args)))
         (if (not (fboundp handler))
             (rcirc-handler-generic process cmd sender args text)
           (funcall handler process sender args text))
         (run-hook-with-args 'rcirc-receive-message-functions
-                            process cmd sender args text)))
+                            process cmd sender args text))
     (message "UNHANDLED: %s" text)))
 
 (defvar rcirc-responses-no-activity '("305" "306")
diff --git a/test/lisp/net/rcirc-tests.el b/test/lisp/net/rcirc-tests.el
new file mode 100644
index 00000000000..128cb2e7540
--- /dev/null
+++ b/test/lisp/net/rcirc-tests.el
@@ -0,0 +1,52 @@
+;;; rcirc-tests.el --- Tests for rcirc -*- lexical-binding:t -*-
+
+;; Copyright (C) 2019 Free Software Foundation, Inc.
+
+;; This program is free software: you can redistribute it and/or
+;; modify it under the terms of the GNU General Public License as
+;; published by the Free Software Foundation, either version 3 of the
+;; License, or (at your option) any later version.
+;;
+;; This program is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see `https://www.gnu.org/licenses/'.
+
+;;; Code:
+
+(require 'ert)
+(require 'rcirc)
+(require 'cl-lib)
+
+(defun rcirc-tests--parse-server-response (cmd text)
+  (cl-letf* ((received-args nil)
+             ((symbol-function (intern (concat "rcirc-handler-" cmd)))
+              (lambda (_process sender args text)
+                (setq received-args (list sender cmd args text))))
+             (rcirc-receive-message-functions nil)
+             (rcirc-trap-errors-flag nil))
+    (rcirc-process-server-response nil text)
+    received-args))
+
+(defmacro rcirc-tests--server-response-parse-should-be
+    (text expected-sender expected-cmd expected-args)
+  (declare (debug t))
+  (macroexp-let2* nil ((cmd expected-cmd))
+    `(should (equal (rcirc-tests--parse-server-response ,cmd ,text)
+                    (list ,expected-sender ,cmd ,expected-args ,text)))))
+
+(ert-deftest rcirc-process-server-response ()
+  (rcirc-tests--server-response-parse-should-be
+   "MODE #cchan +kl a:b :999"
+   nil "MODE" '("#cchan" "+kl" "a:b" "999"))
+  (rcirc-tests--server-response-parse-should-be
+   "MODE #cchan +kl a:b 999"
+   nil "MODE" '("#cchan" "+kl" "a:b" "999"))
+  (rcirc-tests--server-response-parse-should-be
+    "MODE #cchan +kl :a:b"
+    nil "MODE" '("#cchan" "+kl" "a:b")))
+
+;;; rcirc-tests.el ends here