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: Emacs commit ae963e80a79f5a9184daabfc8197f211a39b136d (sieve-manage) Date: Thu, 19 Jan 2023 15:16:16 +0100 Message-ID: <87h6wmboen.fsf@tetzco.de> References: <87wnhj5nbk.fsf@tetzco.de> <835yd31wk8.fsf@gnu.org> <874jsngod2.fsf@tetzco.de> <83tu0nynla.fsf@gnu.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="23722"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: herbert@gojira.at, 54154@debbugs.gnu.org, larsi@gnus.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Jan 19 15:17:19 2023 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 1pIVjC-0005wj-HW for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 19 Jan 2023 15:17:18 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pIVj0-00039P-Uy; Thu, 19 Jan 2023 09:17:06 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pIVix-00038J-PJ for bug-gnu-emacs@gnu.org; Thu, 19 Jan 2023 09:17:05 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pIViw-0001ds-4V for bug-gnu-emacs@gnu.org; Thu, 19 Jan 2023 09:17:03 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pIViv-0003QV-Pr for bug-gnu-emacs@gnu.org; Thu, 19 Jan 2023 09:17:01 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Kai Tetzlaff Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 19 Jan 2023 14:17:01 +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.167413778913119 (code B ref 54154); Thu, 19 Jan 2023 14:17:01 +0000 Original-Received: (at 54154) by debbugs.gnu.org; 19 Jan 2023 14:16:29 +0000 Original-Received: from localhost ([127.0.0.1]:42944 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pIViO-0003PW-8E for submit@debbugs.gnu.org; Thu, 19 Jan 2023 09:16:29 -0500 Original-Received: from mx2.tetzco.de ([152.67.86.91]:43219) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pIViK-0003Ox-Gy for 54154@debbugs.gnu.org; Thu, 19 Jan 2023 09:16:26 -0500 Original-Received: from mail.tetzco.de (unknown [IPv6:2a02:810d:1380:7900::9]) (Authenticated sender: relay@tetzco.de) by mx2.tetzco.de (Postfix) with ESMTPSA id AAD06BD078; Thu, 19 Jan 2023 15:16:18 +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 ECDF86C00B2; Thu, 19 Jan 2023 15:36:09 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tetzco.de; s=20210624; t=1674138970; 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=+mzs+WLC2QhRGO/+r5+onbP56ir44h9frquuuKWTZs0=; b=udiHIC4oanNrDHZquDt/8KIzlJR+GFiLviSKf+dGfP27VnOd7R2yGLzWVfCtquhODHlwOB 8PEu5SVwbcuYi+s3t8LsKzZlO8h4Yp5DmBojN6XMhFW/aLTghN696YzyEhXUPkU6SR4uKE mj8JNtbOPPFFFpxAHZyrXF4QZLHJ3+857YGnry5Ae6qgtqknE7O4FTLCeYAJlvbDUR0clA kY9gA1u8dSzn0O7TPgeyQePrNvzs822sBZ6DJAm87QyEXm9W8K8wS492kYwe2SHQE0SDH9 DNMqo1HCl5GDivFuNr7GB5sMuty3AN+9UTLNmqYl/grQQb9CgZqBpdY9WJBGNF+UIV1OXL Qs6tD4HtSg76tt5fYDzoUV1Ny4ieNR+KHF80LU60nsWkn9ifisZj4Z+bYxK0uM38z5Ur3H 8CL9bBmgHQU+vqUt7OczD61OXkVMW3aDaAGg0klLIcCil781FdF7kmwVWA+yOqI0l5bXp1 NKFIz5YLpAxSDxioZQxqQLJap+RG2iyXvuu5trh07oDpYfUShaXHQ0Y0bDAY4Be In-Reply-To: <83tu0nynla.fsf@gnu.org> (Eli Zaretskii's message of "Thu, 19 Jan 2023 09:45:21 +0200") X-Rspamd-Server: rakaposhi X-Rspamd-Queue-Id: ECDF86C00B2 X-Rspamd-Action: no action X-Spamd-Result: default: False [-6.07 / 30.00]; BAYES_HAM(-3.00)[100.00%]; NEURAL_HAM(-2.97)[-0.991]; 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:+,5:+,6:+,7:+]; DKIM_SIGNED(0.00)[tetzco.de:s=20210624]; TAGGED_FROM(0.00)[bug]; ARC_NA(0.00)[]; TO_MATCH_ENVRCPT_ALL(0.00)[]; FROM_HAS_DN(0.00)[]; HAS_ATTACHMENT(0.00)[]; RCPT_COUNT_THREE(0.00)[4]; TO_DN_SOME(0.00)[]; MID_RHS_MATCH_FROM(0.00)[] 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-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:253709 Archived-At: --=-=-= Content-Type: text/plain Here's the update. As a summary, this change: 2. changed the call to `open-network-stream' in the patch above to >> @@ -244,8 +267,7 @@ 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 >> + :coding sieve-manage--coding-system needed to be reverted to what I had earlier: >> @@ -244,8 +267,7 @@ 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 >> + :coding `(binary . ,sieve-manage--coding-system) I updated the text below and the patches accordingly. Hello Eli, thanks for looking into this! Eli Zaretskii writes: > The duplicate use of with-current-buffer is sub-optimal, > IMO. What about the simpler code below: > > (when sieve-manage-log > (let* ((existing-log-buffer (get-buffer sieve-manage-log)) > (log-buffer (or existing-log-buffer > (get-buffer-create sieve-manage-log)))) > (with-current-buffer log-buffer > (unless existing-log-buffer > ;; Do this only once, when creating the log buffer. > (set-buffer-multibyte nil) > (buffer-disable-undo)) > (goto-char (point-max)) > (apply #'insert args))))) Yes, that provides more insight into what the code intends to do. Here's the patch (with additional updated doc string): --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Fix-bug-in-sieve-manage-append-to-log-emacs-29-only.patch Content-Description: fix sieve-manage--append-to-log for emacs-29 >From 62d03f302125c0b1aab2e3ae4f5b12b531d30d74 Mon Sep 17 00:00:00 2001 From: Kai Tetzlaff Date: Thu, 19 Jan 2023 03:16:14 +0100 Subject: [PATCH] ; Fix bug in sieve-manage--append-to-log (emacs-29 only) This is emacs-29 only, use more elaborate fix for Emacs 30.x (master). * lisp/net/sieve-manage.el (sieve-manage--append-to-log): Fix log buffer creation. --- lisp/net/sieve-manage.el | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/lisp/net/sieve-manage.el b/lisp/net/sieve-manage.el index 5bee4f4c4ad..4866f788bff 100644 --- a/lisp/net/sieve-manage.el +++ b/lisp/net/sieve-manage.el @@ -168,19 +168,25 @@ sieve-manage-capability ;; Internal utility functions (defun sieve-manage--append-to-log (&rest args) - "Append ARGS to sieve-manage log buffer. + "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." +The buffer to use for logging is specifified via `sieve-manage-log'. +If it is nil, logging is disabled. + +When the `sieve-manage-log' buffer doesn't exist, it gets created (and +configured with some initial settings)." (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)))) + (let* ((existing-log-buffer (get-buffer sieve-manage-log)) + (log-buffer (or existing-log-buffer + (get-buffer-create sieve-manage-log)))) + (with-current-buffer log-buffer + (unless existing-log-buffer + ;; Do this only once, when creating the log buffer. + (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. -- 2.39.0 --=-=-= Content-Type: text/plain >> ;; Internal utility functions >> +(defun sieve-manage--set-internal-buffer-properties (buffer) >> + "Set BUFFER properties for internally used buffers. >> + >> +Used for process and log buffers, this function makes sure that >> +those buffers keep received and sent data intact by: >> +- setting the coding system to 'sieve-manage--coding-system', >> +- setting `after-change-functions' to nil to avoid those >> + functions messing with buffer content. >> +Also disables undo (to save a bit of memory and improve >> +performance). >> + >> +Returns BUFFER." >> + (with-current-buffer buffer >> + (set-buffer-file-coding-system sieve-manage--coding-system) >> + (setq-local after-change-functions nil) >> + (buffer-disable-undo) >> + (current-buffer))) >> + >> (defun sieve-manage--append-to-log (&rest args) >> "Append ARGS to sieve-manage log buffer. >> >> @@ -175,10 +202,8 @@ sieve-manage--append-to-log >> `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))) >> + (sieve-manage--set-internal-buffer-properties >> + (get-buffer-create sieve-manage-log))) >> (goto-char (point-max)) >> (apply #'insert args)))) > > This still uses a less-than-elegant implementation that calls > with-current-buffer twice. Yes, true. But since `sieve-manage--set-internal-buffer-properties' is used in two different places, the more elegant solution you suggested above would require duplicating the body of the function in those places. I just didn't see a better way. In particular because `set-buffer-file-coding-system' and `setq-local' only work with (current-buffer). If you can point me to code which can replace these with something that takes BUFFER arguments, I can rewrite `sieve-manage--set-internal-buffer-properties' and avoid using `with-current-buffer'. >> (defun sieve-manage-encode (utf8-string) >> "Convert UTF8-STRING to managesieve protocol octets." >> - (encode-coding-string utf8-string 'raw-text t)) >> + (encode-coding-string utf8-string sieve-manage--coding-system t)) > > Why is the argument called utf8-string? If it's indeed a string > encoded in UTF-8, why do you need to encode it again with > raw-text-unix? it should be a no-op in that case. So please tell more > about the underlying issue. I chose the name as a hint to the user that the incoming string should be UTF-8 encoded. But that is probably misleading since the string itself doesn't have an encoding? So let's change the function to: (defun sieve-manage-encode (str) "Convert STR to managesieve protocol octets." (encode-coding-string str sieve-manage--coding-system t)) Regarding the potential double encoding: When sending data over the network connection, `sieve-manage-encode' intends to make sure that `utf8-string' data is converted to a byte/octet representation. I tried to explain that in the doc string of `sieve-manage--coding-system': (defconst sieve-manage--coding-system 'raw-text-unix "Use 'raw-text-unix coding system for (network) communication. Sets the coding system used for the internal (process, log) buffers and the network stream created to communicate with the managesieve server. Using 'raw-text encoding enables unibyte mode and makes sure that sent/received octets (bytes) remain untouched by the coding system. The explicit use of `-unix` avoids EOL conversions (and thus keeps CRLF (\"\\r\\n\") intact).") The original problem was that when communicating with the sievemanage server, we need to handle length elements where we need make sure that calculated values take into account that UTF-8 characters may comprise multiple octets. Even after reading the relevant sections of the documentation multiple times I was (and am still) not sure what exactly the various coding system settings do and how they interact with buffers and networking functions. So forgive me if what I'm doing there looks weird to your expert eyes. When working on the original patch, I had several uhoh moments where data sent to or received from the network seemed to have been automatically modified by the coding system (unfortunately, I don't remember the exact details). So I tried to eliminate any such automatic modifications by using 'binary or 'raw-text encodings on code paths which handle network data. Basically, my thinking was: 'better do things twice/thrice/... before introducing new points of failure'. >> @@ -244,8 +267,7 @@ 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 >> + :coding `(binary . ,sieve-manage--coding-system) > > Same question about encoding with raw-text-unix here: using it means > some other code will need to encode the text with a real encoding, > which in this case is UTF-8 (AFAIU the managesieve protocol RFC). So > why not use utf-8-unix here? Same as above: I'm just not sure that this is the right thing. But after thinking about it some more, I made the following change (as an experiment): 1. set `sieve-manage--coding-system' to 'utf-8-unix (and update the doc string) With this change, there are still no issues when connecting to my managesieve server which still contains a script with a name that contains utf-8 multibyte characters. Also, the test I wrote still works with that change. So if you think that this makes things clearer, I'm happy to make the change. I just don't feel confident enough to do this without additional guidance. I was also experimenting with some additional changes with the hope to to just use coding system settings instead of calling `sieve-manage-encode'/`sieve-manage-decode'. But I couldn't get that to work. I added an updated set of Emacs 30.x patches with the changes described above (plus an additional change in sieve.el which makes sure that the sieve-manage buffer containing the list of available sieve scripts is also using coding system 'utf-8-unix from `sieve-manage--coding-system'). > Should the addition of BYE support be mentioned in NEWS? I can certainly do that if you think that this is useful. It just seems to be more of an internal detail which probably doesn't mean much to most users. > On balance, I think the additional patches should go to master, > indeed. But let's resolve the issues mentioned above first. Ok, awaiting further input... --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Fix-bug-in-sieve-manage-append-to-log-improve-sieve-.patch >From 977734f16874636c4f2f5e3bb41a86e4338247c4 Mon Sep 17 00:00:00 2001 From: Kai Tetzlaff Date: Mon, 28 Feb 2022 11:08:07 +0100 Subject: [PATCH 1/4] Fix bug in sieve-manage--append-to-log, improve sieve-manage buffer config Also update/add some comments/docs. * lisp/net/sieve-manage.el (sieve-manage-coding-system-for-read) (sieve-manage-coding-system-for-write): Remove unused constants. (sieve-manage-encode): Change misleading argument name (utf8-string -> str). (sieve-manage--coding-system): New constant. (sieve-manage--set-internal-buffer-properties): New function. (sieve-manage-open-server): Use `sieve-manage--coding-system'. (sieve-manage--append-to-log): Use `sieve-manage--set-internal-buffer-properties' to fix log buffer creation. (sieve-manage-encode) (sieve-manage-make-process-buffer): Use `sieve-manage--set-internal-buffer-properties'. --- lisp/net/sieve-manage.el | 65 +++++++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 21 deletions(-) diff --git a/lisp/net/sieve-manage.el b/lisp/net/sieve-manage.el index 5bee4f4c4ad..adfecc7b309 100644 --- a/lisp/net/sieve-manage.el +++ b/lisp/net/sieve-manage.el @@ -58,7 +58,7 @@ ;; ;; References: ;; -;; draft-martin-managesieve-02.txt, +;; RFC5804, ;; "A Protocol for Remotely Managing Sieve Scripts", ;; by Tim Martin. ;; @@ -145,6 +145,15 @@ sieve-manage-ignore-starttls :type 'boolean) ;; Internal variables: +(defconst sieve-manage--coding-system 'utf-8-unix + "Use \\='utf-8-unix coding system for (network) communication. + +Defines the coding system used for the internal (process, log) +buffers and the network stream created to communicate with the +managesieve server. Using \\='utf-8-unix encoding corresponds to +the use of UTF-8 in RFC5804 (managesieve). The explicit use of +\\='-unix\\=' avoids EOL conversions (and thus keeps CRLF (\"\\r\\n\") +sequences intact).") (defconst sieve-manage-local-variables '(sieve-manage-server sieve-manage-port @@ -154,8 +163,6 @@ sieve-manage-local-variables sieve-manage-client-eol sieve-manage-server-eol sieve-manage-capability)) -(defconst sieve-manage-coding-system-for-read 'binary) -(defconst sieve-manage-coding-system-for-write 'binary) (defvar sieve-manage-stream nil) (defvar sieve-manage-auth nil) (defvar sieve-manage-server nil) @@ -167,18 +174,37 @@ sieve-manage-process (defvar sieve-manage-capability nil) ;; Internal utility functions +(defun sieve-manage--set-internal-buffer-properties (buffer) + "Set BUFFER properties for internally used buffers. + +Used for process and log buffers, this function makes sure that +those buffers keep received and sent data intact by: +- setting the coding system to 'sieve-manage--coding-system', +- setting `after-change-functions' to nil to avoid those + functions messing with buffer content. +Also disables undo (to save a bit of memory and improve +performance). + +Returns BUFFER." + (with-current-buffer buffer + (set-buffer-file-coding-system sieve-manage--coding-system) + (setq-local after-change-functions nil) + (buffer-disable-undo) + (current-buffer))) + (defun sieve-manage--append-to-log (&rest args) - "Append ARGS to sieve-manage log buffer. + "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." +The buffer to use for logging is specifified via `sieve-manage-log'. +If it is nil, logging is disabled. + +When the `sieve-manage-log' buffer doesn't exist, it gets created (and +configured with some initial settings)." (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))) + (sieve-manage--set-internal-buffer-properties + (get-buffer-create sieve-manage-log))) (goto-char (point-max)) (apply #'insert args)))) @@ -202,9 +228,9 @@ sieve-manage--error (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-encode (str) + "Convert STR to managesieve protocol octets." + (encode-coding-string str sieve-manage--coding-system t)) (defun sieve-manage-decode (octets &optional buffer) "Convert managesieve protocol OCTETS to utf-8 string. @@ -216,13 +242,11 @@ sieve-manage-decode (defun sieve-manage-make-process-buffer () (with-current-buffer - (generate-new-buffer (format " *sieve %s:%s*" - sieve-manage-server - sieve-manage-port)) + (sieve-manage--set-internal-buffer-properties + (generate-new-buffer (format " *sieve %s:%s*" + sieve-manage-server + sieve-manage-port))) (mapc #'make-local-variable sieve-manage-local-variables) - (set-buffer-multibyte nil) - (setq-local after-change-functions nil) - (buffer-disable-undo) (current-buffer))) (defun sieve-manage-erase (&optional p buffer) @@ -244,8 +268,7 @@ 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 + :coding `(binary . ,sieve-manage--coding-system) :capability-command "CAPABILITY\r\n" :end-of-command "^\\(OK\\|NO\\).*\n" :success "^OK.*\n" -- 2.39.0 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0002-Handle-BYE-in-sieve-manage-server-responses.patch >From 55e3dfae66be5df73aff96fccd8dc5704a2c5dc5 Mon Sep 17 00:00:00 2001 From: Kai Tetzlaff Date: Thu, 19 Jan 2023 03:52:43 +0100 Subject: [PATCH 2/4] Handle BYE in sieve-manage server responses * lisp/net/sieve-manage.el (sieve-manage-regex-oknobye): New function. (sieve-manage-parse-oknobye): Renamed from `sieve-manage-parse-okno'. (sieve-manage-open-server) (sieve-sasl-auth) (sieve-manage-listscripts) (sieve-manage-putscript) (sieve-manage-deletescript) (sieve-manage-getscript) (sieve-manage-setactive) (sieve-manage-is-okno) (sieve-manage-parse-oknobye) (sieve-manage-parse-parse-string) (sieve-manage-parse-parse-crlf): Use `sieve-manage-regex-oknobye' to handle BYE in addition to OK and NO. --- lisp/net/sieve-manage.el | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/lisp/net/sieve-manage.el b/lisp/net/sieve-manage.el index adfecc7b309..ed3dcef8d04 100644 --- a/lisp/net/sieve-manage.el +++ b/lisp/net/sieve-manage.el @@ -270,8 +270,8 @@ sieve-manage-open-server :type stream :coding `(binary . ,sieve-manage--coding-system) :capability-command "CAPABILITY\r\n" - :end-of-command "^\\(OK\\|NO\\).*\n" - :success "^OK.*\n" + :end-of-command "^\\(OK\\|NO\\|BYE\\).*\r\n" + :success "^OK.*\r\n" :return-list t :starttls-function (lambda (capabilities) @@ -330,7 +330,7 @@ sieve-sasl-auth (setq rsp nil) (goto-char (match-end 0)) rsp)) - (setq rsp (sieve-manage-is-okno)))) + (setq rsp (sieve-manage-is-oknobye)))) (accept-process-output sieve-manage-process 1) (goto-char (point-min))) (sieve-manage-erase) @@ -499,19 +499,19 @@ sieve-manage-listscripts (defun sieve-manage-havespace (name size &optional buffer) (with-current-buffer (or buffer (current-buffer)) (sieve-manage-send (format "HAVESPACE \"%s\" %s" name size)) - (sieve-manage-parse-okno))) + (sieve-manage-parse-oknobye))) (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 (length (sieve-manage-encode content)) sieve-manage-client-eol content)) - (sieve-manage-parse-okno))) + (sieve-manage-parse-oknobye))) (defun sieve-manage-deletescript (name &optional buffer) (with-current-buffer (or buffer (current-buffer)) (sieve-manage-send (format "DELETESCRIPT \"%s\"" name)) - (sieve-manage-parse-okno))) + (sieve-manage-parse-oknobye))) (defun sieve-manage-getscript (name output-buffer &optional buffer) (with-current-buffer (or buffer (current-buffer)) @@ -519,17 +519,22 @@ sieve-manage-getscript (sieve-manage-decode (sieve-manage-parse-string) output-buffer) (sieve-manage-parse-crlf) - (sieve-manage-parse-okno))) + (sieve-manage-parse-oknobye))) (defun sieve-manage-setactive (name &optional buffer) (with-current-buffer (or buffer (current-buffer)) (sieve-manage-send (format "SETACTIVE \"%s\"" name)) - (sieve-manage-parse-okno))) + (sieve-manage-parse-oknobye))) ;; Protocol parsing routines +(defun sieve-manage-regexp-oknobye () + "Return regexp for managesieve 'response-oknobye'." + (concat + "^\\(OK\\|NO\\|BYE\\)\\( (\\([^)]+\\))\\)?\\( \\(.*\\)\\)?" + sieve-manage-server-eol)) (defun sieve-manage-wait-for-answer () - (let ((pattern "^\\(OK\\|NO\\).*\n") + (let ((pattern (sieve-manage-regexp-oknobye)) pos) (while (not pos) (setq pos (search-forward-regexp pattern nil t)) @@ -547,10 +552,8 @@ sieve-manage-ok-p (defun sieve-manage-no-p (rsp) (string= (downcase (or (car-safe rsp) "")) "no")) -(defun sieve-manage-is-okno () - (when (looking-at (concat - "^\\(OK\\|NO\\)\\( (\\([^)]+\\))\\)?\\( \\(.*\\)\\)?" - sieve-manage-server-eol)) +(defun sieve-manage-is-oknobye () + (when (looking-at (sieve-manage-regexp-oknobye)) (let ((status (match-string 1)) (resp-code (match-string 3)) (response (match-string 5))) @@ -559,12 +562,12 @@ sieve-manage-is-okno (setq response (sieve-manage-is-string))) (list status resp-code response)))) -(defun sieve-manage-parse-okno () +(defun sieve-manage-parse-oknobye () (let (rsp) (while (null rsp) (accept-process-output (get-buffer-process (current-buffer)) 1) (goto-char (point-min)) - (setq rsp (sieve-manage-is-okno))) + (setq rsp (sieve-manage-is-oknobye))) (sieve-manage-erase) rsp)) @@ -598,7 +601,7 @@ sieve-manage-parse-string (accept-process-output (get-buffer-process (current-buffer)) 1) (goto-char (point-min)) (unless (setq rsp (sieve-manage-is-string)) - (when (sieve-manage-no-p (sieve-manage-is-okno)) + (when (sieve-manage-no-p (sieve-manage-is-oknobye)) ;; simple `error' is enough since `sieve-manage-erase' ;; already adds the server response to the log (error (sieve-manage-erase))))) @@ -612,7 +615,7 @@ sieve-manage-parse-crlf (defun sieve-manage-parse-listscripts () (let (tmp rsp data) (while (null rsp) - (while (null (or (setq rsp (sieve-manage-is-okno)) + (while (null (or (setq rsp (sieve-manage-is-oknobye)) (setq tmp (sieve-manage-decode (sieve-manage-is-string))))) (accept-process-output (get-buffer-process (current-buffer)) 1) -- 2.39.0 --=-=-= Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename=0003-Add-test-lisp-net-sieve-manage-tests.el.patch Content-Transfer-Encoding: quoted-printable >From 786325d3adf7514967ae74c2db84c532f19dc070 Mon Sep 17 00:00:00 2001 From: Kai Tetzlaff Date: Tue, 22 Mar 2022 20:48:09 +0100 Subject: [PATCH 3/4] Add test/lisp/net/sieve-manage-tests.el * test/lisp/net/sieve-manage-tests.el: (mk-literal-s2c) (mk-rsp-oknobye) (mk-rsp-getscript-ok) (managesieve-getscript): Test helper functions. (ert/managesieve-getscript-multibyte): Test `sieve-manage-getscript' with script names containing multibyte characters. --- test/lisp/net/sieve-manage-tests.el | 106 ++++++++++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 test/lisp/net/sieve-manage-tests.el diff --git a/test/lisp/net/sieve-manage-tests.el b/test/lisp/net/sieve-mana= ge-tests.el new file mode 100644 index 00000000000..d4c616b3364 --- /dev/null +++ b/test/lisp/net/sieve-manage-tests.el @@ -0,0 +1,106 @@ +;;; sieve-manage-tests.el --- tests for sieve-manage.el -*- lexical-= binding: t; -*- + +;; Copyright (C) 2022 Free Software Foundation, Inc. + +;; Author: Kai Tetzlaff + +;; This file is part of GNU Emacs. + +;; GNU Emacs 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. + +;; GNU Emacs 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 GNU Emacs. If not, see . + +;;; Commentary: + + +;;; Code: + +(require 'ert) +(require 'sieve-manage) + +(defvar sieve-script-multibyte-unix + "if body :matches \"=C3=A4\" { stop; }\n" + "Simple multibyte sieve script.") + +(defun mk-literal-s2c (string) + "Encode STRING to managesieve 'literal-s2c'." + (let ((data (sieve-manage-encode string))) + (concat (format "{%d}\r\n" (length data)) + data))) + +(defun mk-rsp-oknobye (type &optional resp-code string) + "Encode TYPE, RESP-CODE and STRING to managesieve 'response-oknobye'." + (when (memq type '(OK NO BYE)) + (concat + (mapconcat #'identity + (delq nil + (list (symbol-name type) + (when resp-code + (format "(%s)" resp-code)) + (when string + (format "\"%s\"" + (sieve-manage-encode string))))) + " ") + "\r\n"))) +;; (mk-rsp-oknobye 'OK nil "Getscript completed.") +;; (mk-rsp-oknobye 'NO "TRYLATER" "Server is busy.") + +(defun mk-rsp-getscript-ok (script) + "Encode SCRIPT to managesieve 'response-getscript'." + (concat (mk-literal-s2c script) + "\r\n" + (mk-rsp-oknobye 'OK "Getscript completed."))) +;; (mk-rsp-getscript-ok sieve-script-multibyte-unix) + +(defun managesieve-getscript (script) + "Simulate managesieve getscript response to test +`sieve-manage-getscript' function." + (let* ((script-name "test.sieve") + ;; `sieve-manage-server' and `sieve-manage-port' are used in + ;; `sieve-manage-make-process-buffer' + (sieve-manage-server) + (sieve-manage-port "sieve") + (sieve-buffer (sieve-manage-make-process-buffer)) + (output-buffer (generate-new-buffer script-name))) + ;; use cl-letf to mock some functions in call chain of + ;; sieve-manage-getscript + (cl-letf (((symbol-function 'sieve-manage-send) + ;; simulate sieve server response with a single + ;; multibyte character `=C3=A4` + (lambda (_) + (with-current-buffer sieve-buffer + (goto-char (point-min)) + (insert (mk-rsp-getscript-ok script))))) + ((symbol-function 'accept-process-output) + (lambda (&optional _ _ _ _) nil)) + ((symbol-function 'get-buffer-process) (lambda (_) nil))) + ;; extract sieve script from sieve-buffer and put it into + ;; output-buffer + (sieve-manage-getscript script-name output-buffer sieve-buffer) + ;; extract script from output-buffer and return it as a string + (let ((script (with-current-buffer output-buffer + (set-buffer-modified-p nil) + (buffer-string)))) + ;; cleanup + (kill-buffer sieve-buffer) + (kill-buffer output-buffer) + (when (get-buffer sieve-manage-log) + (kill-buffer sieve-manage-log)) + ;; return script + script)))) + +(ert-deftest ert/managesieve-getscript-multibyte () + (should (string=3D sieve-script-multibyte-unix + (managesieve-getscript sieve-script-multibyte-unix)))) + + +;;; sieve-manage-tests.el ends here --=20 2.39.0 --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0004-Some-minor-fixes-in-lisp-net-sieve.el.patch >From 6c63f891589e6525bf91ef53a0a8114a836a7781 Mon Sep 17 00:00:00 2001 From: Kai Tetzlaff Date: Thu, 19 Jan 2023 04:06:22 +0100 Subject: [PATCH 4/4] Some minor fixes in lisp/net/sieve.el * lisp/net/sieve.el (sieve-next-line) (sieve-prev-line): Handle situations where point in `sieve-buffer' is either before or after the list of server side scripts. (sieve-server-script-list): New variable. (sieve-refresh-scriptlist) (sieve-upload): Use `sieve-server-script-list' to make sure that local list of server side scripts in`sieve-buffer' is up-to-date after uploading a (new) script. (sieve-edit-script): Make sure that the newly created user facing sieve-manage buffer is using 'utf-8-unix encoding. --- lisp/net/sieve.el | 40 +++++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/lisp/net/sieve.el b/lisp/net/sieve.el index fbd07dee27c..1e5aae7825c 100644 --- a/lisp/net/sieve.el +++ b/lisp/net/sieve.el @@ -95,6 +95,8 @@ sieve-template (defvar sieve-manage-buffer nil) (defvar sieve-buffer-header-end nil) +(defvar sieve-server-script-list nil + "Current list of server-side sieve scripts.") (defvar sieve-buffer-script-name nil "The real script name of the buffer.") (make-local-variable 'sieve-buffer-script-name) @@ -204,6 +206,8 @@ sieve-edit-script (if (not (string-equal name sieve-new-script)) (let ((newbuf (generate-new-buffer name)) err) + (with-current-buffer newbuf + (set-buffer-file-coding-system sieve-manage--coding-system)) (setq err (sieve-manage-getscript name newbuf sieve-manage-buffer)) (switch-to-buffer newbuf) (if (sieve-manage-ok-p err) @@ -235,23 +239,28 @@ sieve-next-line (interactive) (unless arg (setq arg 1)) - (if (save-excursion - (forward-line arg) - (sieve-script-at-point)) - (sieve-change-region - (forward-line arg)) - (message "End of list"))) + (if (sieve-script-at-point) + (if (save-excursion + (forward-line arg) + (sieve-script-at-point)) + (sieve-change-region + (forward-line arg)) + (message "End of list")) + (goto-char (next-overlay-change sieve-buffer-header-end)))) (defun sieve-prev-line (&optional arg) (interactive) (unless arg (setq arg -1)) - (if (save-excursion - (forward-line arg) - (sieve-script-at-point)) - (sieve-change-region - (forward-line arg)) - (message "Beginning of list"))) + (if (sieve-script-at-point) + (if (save-excursion + (forward-line arg) + (sieve-script-at-point)) + (sieve-change-region + (forward-line arg)) + (message "Beginning of list")) + (goto-char (previous-overlay-change (point-max))) + (beginning-of-line))) (defun sieve-help () "Display help for various sieve commands." @@ -319,6 +328,9 @@ sieve-refresh-scriptlist (let* ((scripts (sieve-manage-listscripts sieve-manage-buffer)) (count (length scripts)) (keys (substitute-command-keys "\\[sieve-edit-script]"))) + (setq sieve-server-script-list + (mapcar (lambda (elt) (if (consp elt) (cdr elt) elt)) + scripts)) (insert (if (null scripts) (format @@ -361,7 +373,9 @@ sieve-upload (if (not (sieve-manage-ok-p err)) (message "Sieve upload failed: %s" (nth 2 err)) (message "Sieve upload done. Use %s to manage scripts." - (substitute-command-keys "\\[sieve-manage]")))) + (substitute-command-keys "\\[sieve-manage]")) + (when (not (member script-name sieve-server-script-list)) + (sieve-refresh-scriptlist)))) (set-buffer-modified-p nil)))) ;;;###autoload -- 2.39.0 --=-=-=--