From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Kai Tetzlaff Newsgroups: gmane.emacs.bugs Subject: bug#54154: 29.0.50; [PATCH] `sieve-manage-getscript' fails if script contains multibyte characters Date: Mon, 28 Feb 2022 13:27:42 +0100 Message-ID: <87sfs3w4zl.fsf@tetzco.de> References: <87wnhj5nbk.fsf@tetzco.de> <878rtzxhnc.fsf@gnus.org> <875yp3w0pr.fsf@gnus.org> <87ilt33pi3.fsf@tetzco.de> <877d9hvf8d.fsf@gnus.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="7111"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 54154@debbugs.gnu.org To: Lars Ingebrigtsen Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Feb 28 17:21:59 2022 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1nOimc-0001Xx-C2 for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 28 Feb 2022 17:21:58 +0100 Original-Received: from localhost ([::1]:54390 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nOimb-0006s3-19 for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 28 Feb 2022 11:21:57 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:49112) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nOikl-0004rn-77 for bug-gnu-emacs@gnu.org; Mon, 28 Feb 2022 11:20:03 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:40703) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nOikk-0005cB-Tz for bug-gnu-emacs@gnu.org; Mon, 28 Feb 2022 11:20:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1nOikk-0004Ka-O9 for bug-gnu-emacs@gnu.org; Mon, 28 Feb 2022 11:20:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Kai Tetzlaff Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 28 Feb 2022 16:20:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 54154 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 54154-submit@debbugs.gnu.org id=B54154.164606517616603 (code B ref 54154); Mon, 28 Feb 2022 16:20:02 +0000 Original-Received: (at 54154) by debbugs.gnu.org; 28 Feb 2022 16:19:36 +0000 Original-Received: from localhost ([127.0.0.1]:34599 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nOikJ-0004Ji-HB for submit@debbugs.gnu.org; Mon, 28 Feb 2022 11:19:36 -0500 Original-Received: from mout-p-101.mailbox.org ([80.241.56.151]:47242) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nOf8K-0001p4-GO for 54154@debbugs.gnu.org; Mon, 28 Feb 2022 07:28:13 -0500 Original-Received: from smtp1.mailbox.org (smtp1.mailbox.org [IPv6:2001:67c:2050:105:465:1:1:0]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-p-101.mailbox.org (Postfix) with ESMTPS id 4K6fmL0Mshz9sHY; Mon, 28 Feb 2022 13:28:02 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tetzlaff.eu; s=20210624; t=1646051266; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=T/k60dYoBh92lO3qi7hK/VTqHEWeb4DOzBpbBz2lGb8=; b=ZZSttbTA47u4BdoEOZOxoAxX0hEBVMtQLAVPAhzYT5TcdX4P/O8+4bJ18waILIVvLg2mNB oiFjVxzd2nA/3++bI4HFhUHmuckpVHJYgUSan7bODqwpvTyx0PUjT/lQLZnqE+xv9xiCDQ h9AredSj4eJ8i0tw5AVvp7gHXw5inbMB0CTmdKR2yql55uBptdYH0KkLukH/3LPl+3UAY6 jV2wsG/hxyRtrAWDnU2wtP6Pp0tojb/+YAWsBwcmEpJqz3QejDAFeAhJXN2zE4fPplO69B Dhu0nhkpyKaEKRk2rP2NFOcI+Nb0QxUXEsi85YqpjX0j1EtFGopVjpl51ZO77D/jdR+wVk mjOUl9fXqp5VfCmurl3am3XBglyA4mTdlgQNxKfdo4fxKTDsZtkmS1sxPUM5fGXh2fwTb2 TbJFH+8aSR0oWnlLmWpQLGjTgPKywJfgXh/VKwJ9X41xqBk2Y1R34acy4MCESGwjfbiOSA 0ViRVgnx3iwX486UoJe4tE0qp1OLb9tzIsh42KcCjf3KfvmFXjkWkn0nI9ofVz DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tetzlaff.eu; s=MBO0001; t=1646051280; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=T/k60dYoBh92lO3qi7hK/VTqHEWeb4DOzBpbBz2lGb8=; b=Fw75l3w+nysQ+bPRUqtd7WC0UJ6LJYP5z7aN22/3iGT4iTc5Cg4ZXbRNMzWI/TmClVOhhK SgPpP3t9IC/6I9/XYETxVgwm93yMRQvwIPHUYgNJ5lSgB8S9f15ae+IwxXharA4f7l2YEJ 2rMJOWqSb+7DN0LbNpPBy47NkyLSU3pL2MhIZMziNAlpMBFCq5NLgM6cvbppNSD/8iYWW8 9UMcZ6pX4FRqGuzoxqrtAcsm5nhEr71PlUrBxjKaICCnPgZg7zXPuuxjUq+XvPx5Y+xwds LnpHOuqTyzWkyW3FDo4CawZdszpE3LPQpz+756gCHyG6ImfGgJK1gotTWCTIlA== In-Reply-To: <877d9hvf8d.fsf@gnus.org> (Lars Ingebrigtsen's message of "Sat, 26 Feb 2022 16:07:14 +0100") X-Rspamd-Queue-Id: 40FDB6C00B7 X-Spamd-Result: default: False [-6.79 / 30.00]; BAYES_HAM(-3.00)[100.00%]; NEURAL_HAM(-3.00)[-1.000]; GENERIC_REPUTATION(-0.69)[-0.68541051511092]; MIME_GOOD(-0.10)[multipart/mixed,text/plain,text/x-diff]; RCVD_COUNT_ZERO(0.00)[0]; FROM_EQ_ENVFROM(0.00)[]; RCPT_COUNT_TWO(0.00)[2]; DKIM_SIGNED(0.00)[tetzlaff.eu:s=20210624]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_TRACE(0.00)[0:+,1:+,2:+,3:+,4:+]; FROM_HAS_DN(0.00)[]; TO_DN_SOME(0.00)[]; ARC_NA(0.00)[] X-Rspamd-Server: rakaposhi X-Mailman-Approved-At: Mon, 28 Feb 2022 11:19:34 -0500 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:227830 Archived-At: --=-=-= Content-Type: text/plain Lars Ingebrigtsen writes: > Kai Tetzlaff writes: > >> So just reverting it won't work. I will try to undo the parts relevant >> to this issue. > > Sounds good. Ok, I'm attaching two patches which fix all issues I noticed. What I ended up with is quite a bit more than the initial attempt. Since these changes are non-trivial, I will need to do the copyright assignment. About a week ago I actually sent an email to assign@gnu.org to get the process started. But I haven't received a reply. Could you please send me the necessary papers? I'm in Germany, so my understanding is that it should be possible to do this electronically? The first (and major) set of fixes are in sieve-manage.el for the issues with multibyte characters in sieve scripts (sieve-manage-getscript/putscript). This also adds supports for multibyte characters in script names (sieve-manage-listscripts/getscript/putscript/havespace/deletescript/setactive). There is now also some handling of getscript errors reported by the server and improved logging. --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Fix-mostly-multibyte-issues-in-sieve-manage.el-Bug-5.patch Content-Description: fix multibyte issues >From fd18929ce2004f7448ab997bc86e206afdbd8673 Mon Sep 17 00:00:00 2001 From: Kai Tetzlaff Date: Mon, 28 Feb 2022 11:08:07 +0100 Subject: [PATCH 1/2] Fix (mostly multibyte) issues in sieve-manage.el (Bug#54154) The managesieve protocol (s. RFC5804) requires support for (a sightly restricted variant of) UTF-8 in script content and script names. This commit fixes/improves the handling of multibyte characters. In addition, `sieve-manage-getscript' now properly handles NO responses from the server instead of inflooping. There are also some logging improvements. * lisp/net/sieve-manage.el (sieve-manage--append-to-log): (sieve-manage--message): (sieve-manage--error): (sieve-manage-encode): (sieve-manage-decode): (sieve-manage-no-p): New functions. (sieve-manage-make-process-buffer): Switch process buffer to unibyte. (sieve-manage-open-server): Add `:coding 'raw-text-unix` to `open-network-stream' call. Use unix EOLs in order to keep matching CRLF (aka "\r\n") intact. (sieve-manage-send): Make sure that UTF-8 multibyte characters are properly encoded before sending data to the server. (sieve-manage-getscript): (sieve-manage-putscript): Use the changes above to fix down/uploading scripts containing UTF-8 multibyte characters. (sieve-manage-listscripts): (sieve-manage-havespace) (sieve-manage-getscript) (sieve-manage-putscript): (sieve-manage-deletescript): (sieve-manage-setactive): Use the changes above to fix handling of script names which contain UTF-8 multibyte characters. (sieve-manage-parse-string): (sieve-manage-getscript): Add handling of server responses with type NO. Abort `sieve-manage-getscript' and show error message in message area. (sieve-manage-erase): (sieve-manage-drop-next-answer): (sieve-manage-parse-crlf): Return erased/dropped data (instead of nil). (sieve-sasl-auth): (sieve-manage-getscript): (sieve-manage-erase): (sieve-manage-open-server): (sieve-manage-open): (sieve-manage-send): Improve logging. --- lisp/net/sieve-manage.el | 125 +++++++++++++++++++++++++++------------ 1 file changed, 86 insertions(+), 39 deletions(-) diff --git a/lisp/net/sieve-manage.el b/lisp/net/sieve-manage.el index 50342b9105..a57d81efcd 100644 --- a/lisp/net/sieve-manage.el +++ b/lisp/net/sieve-manage.el @@ -167,7 +167,52 @@ sieve-manage-process (defvar sieve-manage-capability nil) ;; Internal utility functions -(autoload 'mm-enable-multibyte "mm-util") +(defun sieve-manage--append-to-log (&rest args) + "Append ARGS to sieve-manage log buffer. + +ARGS can be a string or a list of strings. +The buffer to use for logging is specifified via +`sieve-manage-log'. If it is nil, logging is disabled." + (when sieve-manage-log + (with-current-buffer (or (get-buffer sieve-manage-log) + (with-current-buffer + (get-buffer-create sieve-manage-log) + (set-buffer-multibyte nil) + (buffer-disable-undo))) + (goto-char (point-max)) + (apply #'insert args)))) + +(defun sieve-manage--message (format-string &rest args) + "Wrapper around `message' which also logs to sieve manage log. + +See `sieve-manage--append-to-log'." + (let ((ret (apply #'message + (concat "sieve-manage: " format-string) + args))) + (sieve-manage--append-to-log ret "\n") + ret)) + +(defun sieve-manage--error (format-string &rest args) + "Wrapper around `error' which also logs to sieve manage log. + +See `sieve-manage--append-to-log'." + (let ((msg (apply #'format + (concat "sieve-manage/ERROR: " format-string) + args))) + (sieve-manage--append-to-log msg "\n") + (error msg))) + +(defun sieve-manage-encode (utf8-string) + "Convert UTF8-STRING to managesieve protocol octets." + (encode-coding-string utf8-string 'raw-text t)) + +(defun sieve-manage-decode (octets &optional buffer) + "Convert managesieve protocol OCTETS to utf-8 string. + +If optional BUFFER is non-nil, insert decoded string into BUFFER." + (when octets + ;; eol type unix is required to preserve "\r\n" + (decode-coding-string octets 'utf-8-unix t buffer))) (defun sieve-manage-make-process-buffer () (with-current-buffer @@ -175,22 +220,19 @@ sieve-manage-make-process-buffer sieve-manage-server sieve-manage-port)) (mapc #'make-local-variable sieve-manage-local-variables) - (mm-enable-multibyte) + (set-buffer-multibyte nil) + (setq-local after-change-functions nil) (buffer-disable-undo) (current-buffer))) (defun sieve-manage-erase (&optional p buffer) - (let ((buffer (or buffer (current-buffer)))) - (and sieve-manage-log - (with-current-buffer (get-buffer-create sieve-manage-log) - (mm-enable-multibyte) - (buffer-disable-undo) - (goto-char (point-max)) - (insert-buffer-substring buffer (with-current-buffer buffer - (point-min)) - (or p (with-current-buffer buffer - (point-max))))))) - (delete-region (point-min) (or p (point-max)))) + (with-current-buffer (or buffer (current-buffer)) + (let* ((start (point-min)) + (end (or p (point-max))) + (logdata (buffer-substring-no-properties start end))) + (sieve-manage--append-to-log logdata) + (delete-region start end) + logdata))) (defun sieve-manage-open-server (server port &optional stream buffer) "Open network connection to SERVER on PORT. @@ -202,6 +244,8 @@ sieve-manage-open-server (open-network-stream "SIEVE" buffer server port :type stream + ;; eol type unix is required to preserve "\r\n" + :coding 'raw-text-unix :capability-command "CAPABILITY\r\n" :end-of-command "^\\(OK\\|NO\\).*\n" :success "^OK.*\n" @@ -224,7 +268,7 @@ sieve-manage-open-server ;; Authenticators (defun sieve-sasl-auth (buffer mech) "Login to server using the SASL MECH method." - (message "sieve: Authenticating using %s..." mech) + (sieve-manage--message "Authenticating using %s..." mech) (with-current-buffer buffer (let* ((auth-info (auth-source-search :host sieve-manage-server :port "sieve" @@ -275,11 +319,15 @@ sieve-sasl-auth (if (and (setq step (sasl-next-step client step)) (setq data (sasl-step-data step))) ;; We got data for server but it's finished - (error "Server not ready for SASL data: %s" data) + (sieve-manage--error + "Server not ready for SASL data: %s" data) ;; The authentication process is finished. + (sieve-manage--message "Logged in as %s using %s" + user-name mech) (throw 'done t))) (unless (stringp rsp) - (error "Server aborted SASL authentication: %s" (caddr rsp))) + (sieve-manage--error + "Server aborted SASL authentication: %s" (caddr rsp))) (sasl-step-set-data step (base64-decode-string rsp)) (setq step (sasl-next-step client step)) (sieve-manage-send @@ -288,8 +336,7 @@ sieve-sasl-auth (base64-encode-string (sasl-step-data step) 'no-line-break) "\"") - "")))) - (message "sieve: Login using %s...done" mech)))) + ""))))))) (defun sieve-manage-cram-md5-p (buffer) (sieve-manage-capability "SASL" "CRAM-MD5" buffer)) @@ -353,7 +400,7 @@ sieve-manage-open sieve-manage-default-stream) sieve-manage-auth (or auth sieve-manage-auth)) - (message "sieve: Connecting to %s..." sieve-manage-server) + (sieve-manage--message "Connecting to %s..." sieve-manage-server) (sieve-manage-open-server sieve-manage-server sieve-manage-port sieve-manage-stream @@ -368,7 +415,8 @@ sieve-manage-open (setq sieve-manage-auth auth) (cl-return))) (unless sieve-manage-auth - (error "Couldn't figure out authenticator for server"))) + (sieve-manage--error + "Couldn't figure out authenticator for server"))) (sieve-manage-erase) (current-buffer)))) @@ -433,11 +481,7 @@ sieve-manage-havespace (defun sieve-manage-putscript (name content &optional buffer) (with-current-buffer (or buffer (current-buffer)) (sieve-manage-send (format "PUTSCRIPT \"%s\" {%d+}%s%s" name - ;; Here we assume that the coding-system will - ;; replace each char with a single byte. - ;; This is always the case if `content' is - ;; a unibyte string. - (length content) + (length (sieve-manage-encode content)) sieve-manage-client-eol content)) (sieve-manage-parse-okno))) @@ -449,11 +493,10 @@ sieve-manage-deletescript (defun sieve-manage-getscript (name output-buffer &optional buffer) (with-current-buffer (or buffer (current-buffer)) (sieve-manage-send (format "GETSCRIPT \"%s\"" name)) - (let ((script (sieve-manage-parse-string))) - (sieve-manage-parse-crlf) - (with-current-buffer output-buffer - (insert script)) - (sieve-manage-parse-okno)))) + (sieve-manage-decode (sieve-manage-parse-string) + output-buffer) + (sieve-manage-parse-crlf) + (sieve-manage-parse-okno))) (defun sieve-manage-setactive (name &optional buffer) (with-current-buffer (or buffer (current-buffer)) @@ -478,6 +521,9 @@ sieve-manage-drop-next-answer (defun sieve-manage-ok-p (rsp) (string= (downcase (or (car-safe rsp) "")) "ok")) +(defun sieve-manage-no-p (rsp) + (string= (downcase (or (car-safe rsp) "")) "no")) + (defun sieve-manage-is-okno () (when (looking-at (concat "^\\(OK\\|NO\\)\\( (\\([^)]+\\))\\)?\\( \\(.*\\)\\)?" @@ -528,7 +574,11 @@ sieve-manage-parse-string (while (null rsp) (accept-process-output (get-buffer-process (current-buffer)) 1) (goto-char (point-min)) - (setq rsp (sieve-manage-is-string))) + (unless (setq rsp (sieve-manage-is-string)) + (when (sieve-manage-no-p (sieve-manage-is-okno)) + ;; simple `error' is enough since `sieve-manage-erase' + ;; already adds the server response to the log + (error (sieve-manage-erase))))) (sieve-manage-erase (point)) rsp)) @@ -540,7 +590,8 @@ sieve-manage-parse-listscripts (let (tmp rsp data) (while (null rsp) (while (null (or (setq rsp (sieve-manage-is-okno)) - (setq tmp (sieve-manage-is-string)))) + (setq tmp (sieve-manage-decode + (sieve-manage-is-string))))) (accept-process-output (get-buffer-process (current-buffer)) 1) (goto-char (point-min))) (when tmp @@ -559,13 +610,9 @@ sieve-manage-parse-listscripts rsp))) (defun sieve-manage-send (cmdstr) - (setq cmdstr (concat cmdstr sieve-manage-client-eol)) - (and sieve-manage-log - (with-current-buffer (get-buffer-create sieve-manage-log) - (mm-enable-multibyte) - (buffer-disable-undo) - (goto-char (point-max)) - (insert cmdstr))) + (setq cmdstr (sieve-manage-encode + (concat cmdstr sieve-manage-client-eol))) + (sieve-manage--append-to-log cmdstr) (process-send-string sieve-manage-process cmdstr)) (provide 'sieve-manage) -- 2.34.1 --=-=-= Content-Type: text/plain Both, the (internal) process/protocol buffer and the log buffer are now unibyte. The conversion to multibyte UTF-8 is only done for user visible (UI) buffers. To properly handle the protocol line termination (CRLF), I added `:coding 'raw-text-unix` (with explicit unix EOL convention) to the `open-network-stream' call (also in the new `manage-sieve-encode' function. This was needed to allow keep the various (looking-at "...\r\n" ...) calls working. This is something which still feels a bit weird, but I haven't found another way to do it. I did some tests with (setq-default buffer-file-coding-system 'utf-8-unix/'utf-8-dos) which did not show any issues. I would also add some ERT tests, probably in a separate commit? In addition, I found that `sieve-manage-quit' in sieve.el had the tendency to kill unrelated buffers in case of errors during earlier steps. For this, I created a sepate patch: --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0002-Improve-robustnes-of-sieve-manage-quit-in-case-of-er.patch Content-Description: avoid killing unrelated buffers >From 559ce20b4c9b75f67bef3a1e23b7501bdeaa98d2 Mon Sep 17 00:00:00 2001 From: Kai Tetzlaff Date: Mon, 28 Feb 2022 11:33:56 +0100 Subject: [PATCH 2/2] Improve robustnes of `sieve-manage-quit' in case of errors * lisp/net/sieve.el (sieve-manage-quit): Avoid killing buffers it's not supposed to touch. --- lisp/net/sieve.el | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lisp/net/sieve.el b/lisp/net/sieve.el index 630ea04070..5680526389 100644 --- a/lisp/net/sieve.el +++ b/lisp/net/sieve.el @@ -154,7 +154,8 @@ sieve-manage-quit (interactive) (sieve-manage-close sieve-manage-buffer) (kill-buffer sieve-manage-buffer) - (kill-buffer (current-buffer))) + (when-let ((buffer (get-buffer sieve-buffer))) + (kill-buffer buffer))) (defun sieve-bury-buffer () "Bury the Manage Sieve buffer without closing the connection." -- 2.34.1 --=-=-=--