bug#36233: 26.2; Tokenization error in rcirc parser

[Noam Postavsky]

severity 36233 minor
tags 36233 + patch
quit

"Jeff Johnson" <trnsz@pobox.com> writes:

> Example, if the server sends:
> "MODE #cchan +kl a:b :999" 
>
> That is somehow parsed by rcirc as:
> "MODE +kl a b :999"

Yeah, it was using [^:]* to match the middle args.  Patch below.  I
added a test for this case in the patch, although this could probably
use some more testing to make sure I haven't broken other cases.

>From 9b0cb9e737e3c68f6553f1995983f524bdd92453 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sun, 16 Jun 2019 13:48:56 -0400
Subject: [PATCH] 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.
---
 lisp/net/rcirc.el            | 25 ++++++++++++++-------
 test/lisp/net/rcirc-tests.el | 52 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 8 deletions(-)
 create mode 100644 test/lisp/net/rcirc-tests.el

diff --git a/lisp/net/rcirc.el b/lisp/net/rcirc.el
index 8926772b94..d81fa939b9 100644
--- a/lisp/net/rcirc.el
+++ b/lisp/net/rcirc.el
@@ -774,22 +774,31 @@ (defun rcirc-process-server-response (process text)
     (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 accept
+  ;; multiple spaces between args, even though the RFC doesn't allow
+  ;; that.
+  (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 0000000000..128cb2e754
--- /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
-- 
2.11.0