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 14:03:19 +0100 Message-ID: <87mtibw3c8.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="38553"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/29.0.50 (gnu/linux) 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:20:17 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 1nOikz-0009qc-7x for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 28 Feb 2022 17:20:17 +0100 Original-Received: from localhost ([::1]:51690 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1nOiky-0004u4-9o for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 28 Feb 2022 11:20:16 -0500 Original-Received: from eggs.gnu.org ([209.51.188.92]:49104) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nOikk-0004r2-Ov for bug-gnu-emacs@gnu.org; Mon, 28 Feb 2022 11:20:02 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:40702) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1nOikk-0005b6-Fw 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-0004KT-6h 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.164606516916586 (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:29 +0000 Original-Received: from localhost ([127.0.0.1]:34597 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nOik8-0004JL-R1 for submit@debbugs.gnu.org; Mon, 28 Feb 2022 11:19:29 -0500 Original-Received: from mail1.tetzco.de ([140.238.222.122]:42703 helo=mx2.tetzco.de) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1nOfgX-000579-BD for 54154@debbugs.gnu.org; Mon, 28 Feb 2022 08:03:30 -0500 Original-Received: from mail.tetzco.de (unknown [IPv6:2a02:810d:1380:7900::9]) by mx2.tetzco.de (Postfix) with ESMTPS id 525C3BD32C; Mon, 28 Feb 2022 14:03:22 +0100 (CET) Original-Received: from moka (moka.tetzco.de [172.30.42.200]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-256) server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: kai@tetzco.de) by mail.tetzco.de (Postfix) with ESMTPSA id B34DC6C00B7; Mon, 28 Feb 2022 14:03:20 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tetzco.de; s=20210624; t=1646053400; 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=QNXwyAVrDrx67iwZo0vfxJsfMtRmNd7AVvVnttje4iE=; b=oK3AgXzytBd28neVh01DU5LQQXYSEY0hH+AOCcl4+Ce+G1557SjWbDst+rYqRvrSQlz1l0 sBoLCUCquDFZW1SNXqmPZEBfqBtLGxTuz1hbm6yc83tDTjteWGCn8kVQoXG81TbHiY0Dv/ LrcGDSKINBLzPaumYdkefYlfNf43LEGqNsvSBTdTCCiWYmlZjcgy6L1G4A3+zwjQb7Ygru 3WMbHrRK+B5JwniazikFz24ZiWdtnQZxOOX9OfQpZqNVXaf7Fa7P1Q9M4zcJVwiALqSmOA ns0e7x1BM1YtSu/BwGFDg2ZNWlPw3VJxQWxnzNSgTriX8IDWyxtjO4S7h9Zhyudqg68p3F 6WSIAMU67QERB1BfJi98qLiu5y80M+JR9731n89f3W/HTfTedRloFHW75T6jF6b2xo2muw 00DtXTujjoaGJPox8D5c6TLMSUC/A/rk+/I8f1V+trDez79a9z6J9xauOgWH7VFdFz2Jv8 9qfJVu9YgKZrrRS81KMzHgaM0hHIyUg7FJuVyLhZToh9GwXRf6Iv3VYnEvXUi6l In-Reply-To: <877d9hvf8d.fsf@gnus.org> (Lars Ingebrigtsen's message of "Sat, 26 Feb 2022 16:07:14 +0100") X-Rspamd-Queue-Id: B34DC6C00B7 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.6870502201696]; MIME_GOOD(-0.10)[multipart/mixed,text/plain,text/x-diff]; RCVD_COUNT_ZERO(0.00)[0]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+,1:+,2:+,3:+,4:+]; RCPT_COUNT_TWO(0.00)[2]; DKIM_SIGNED(0.00)[tetzco.de:s=20210624]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MID_RHS_MATCH_FROM(0.00)[]; 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:23 -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:227829 Archived-At: --=-=-= Content-Type: text/plain Oh - the previous version of the first patch in my last email still contained a bug (I forgot to re-run `git format-patch` before sending the mail). 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 3a4ecad9f680d130fba9e792b87824e1f5e6a6eb 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 | 126 +++++++++++++++++++++++++++------------ 1 file changed, 87 insertions(+), 39 deletions(-) diff --git a/lisp/net/sieve-manage.el b/lisp/net/sieve-manage.el index 50342b9105..4a36f94431 100644 --- a/lisp/net/sieve-manage.el +++ b/lisp/net/sieve-manage.el @@ -167,7 +167,53 @@ 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) + (current-buffer))) + (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 +221,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 +245,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 +269,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 +320,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 +337,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 +401,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 +416,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 +482,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 +494,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 +522,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 +575,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 +591,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 +611,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 83ab45907c7b528ae4db98f33415e05e679c312e 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 --=-=-=--