Subject: bug#54154: Emacs commit ae963e80a79f5a9184daabfc8197f211a39b136d (sieve-manage)

From: Kai Tetzlaff <[email-masked]>
Date: Thu, 19 Jan 2023 16:59:36 +0100

Eli Zaretskii <[email-masked]> writes:
>> > 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.
>
> I'm not sure why you need to force the encoding of the process buffer,
> when you already set the coding-system to be used for decoding stuff
> from the process.  Is that really needed?

Not sure if it is really needed. But I wanted to make sure that both,
the process buffer and the log buffer use identical settings. Otherwise,
the content of the log buffer might be misleading.

> But if you really need this, then just make the insertion of the text
> into the buffer you create optional: then for the process-buffer pass
> nil as the text to insert, and you can do the with-current-buffer
> dance only inside that function.

Sorry, you lost me there. I don't understand what you want to tell me.
Which (optional) text in which buffer?

> Since you seem to be encoding and decoding to/from UTF-8 by hand in
> sieve-manage-encode/decode, you should use 'binary' as the
> process-codings-system for the network connection to the server, and
> that's it.

That works. Done.

> What you should do is call sieve-manage-encode inside
> sieve-manage-send, and count the bytes there after encoding the
> payload.

Unfortunately, that is too late since the sent data - in case that the
sent text may contain CRLF sequences - contains its own length. So in
order to insert the correct length, I need to encode before sending.
See:

    (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-oknobye)))
 
>> 1. set `sieve-manage--coding-system' to 'utf-8-unix and
>> 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
>> instead of the previous, asymmetric mix of `binary and
>> `sieve-manage--coding-system'.
>
> This could work, but AFAIU, you need to specify the content length in
> bytes for the PUTSCRIPT command, so you must encode the content
> yourself.  Thus my suggestion to use 'binary' in the :coding attribute
> of the process, and instead encode/decode using
> sieve-manage-encode/decode to/from UTF-8 inside sieve-manage-send and
> sieve-manage-parse-* functions.

Yes, that's what I explained above (before reading this part of your
reply). Unfortunately, just using `sieve-manage--coding-system' for the
:coding property didn't work, but I'm now using 'binary' encoding for
both directions.

>> > 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.
>
> Isn't BYE provides some capabilities that user/callers would like to
> know about?

>From RFC5804:

    response-nobye        = ("NO" / "BYE") [SP "(" resp-code ")"]
                            [SP string] CRLF
                            ;; The string contains human-readable text
                            ;; encoded as UTF-8.

As far as I understand, the difference between NO and BYE is that BYE is
just a different (and more drastic, because the server will also
disconnect) way of signalling an error. Fortunately, the human readable
<string> is typically included in these responses and will be shown to
the user.

Here is some more info about where BYE SHOULD (not MUST) be used:

   The BYE response SHOULD be used if the server wishes to close the
   connection.  A server may wish to do this because the client was idle
   for too long or there were too many failed authentication attempts.
   This response can be issued at any time and should be immediately
   followed by a server hang-up of the connection.  ...

If I remember correctly, the timeout case was the reason why I added the
BYE handling (since during my experiments, I sometimes used the debugger
to understand what's going on which introduced long delays between
connection establishment/authentication and sending the first request
and resulted in a BYE instead of a NO).

There's also one additional (more interesting) case:

   REFERRAL

   This response code may be returned with a BYE result from any
   command, and includes a mandatory parameter that indicates what
   server to access to manage this user's Sieve scripts.  The server
   will be specified by a Sieve URL (see Section 3).  The scriptname
   portion of the URL MUST NOT be specified.  The client should
   authenticate to the specified server and use it for all further
   commands in the current session.

However, even my updated sieve-manage code doesn't handle REFERRALs.

So I still think that to understand the difference between a BYE and a
NO would require the user to take a (deeper) dive into the RFC. But to
avoid a longer discussion, how about:

   * Changes in Specialized Modes and Packages in Emacs 30.1
   ...
   ** sieve-manage

   --
   *** Added (partial) handling of BYE responses
   The managesieve client in `sieve-manage' now handles BYE responses
   sent be the server (in addition to OK and NO). This makes the
   implementation more robust in case of e.g. timeouts and
   authentication failures.
   Note: The special case of a REFERRAL/BYE responses is still not
   handled by the client (s. RFC5804 for more details).

>From 977734f16874636c4f2f5e3bb41a86e4338247c4 Mon Sep 17 00:00:00 2001
From: Kai Tetzlaff <[email-masked]>
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

>From 55e3dfae66be5df73aff96fccd8dc5704a2c5dc5 Mon Sep 17 00:00:00 2001
From: Kai Tetzlaff <[email-masked]>
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

>From 786325d3adf7514967ae74c2db84c532f19dc070 Mon Sep 17 00:00:00 2001
From: Kai Tetzlaff <[email-masked]>
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-manage-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 <[email-masked]>
+
+;; 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 <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+
+;;; Code:
+
+(require 'ert)
+(require 'sieve-manage)
+
+(defvar sieve-script-multibyte-unix
+  "if body :matches \"ä\" { 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 `ä`
+               (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= sieve-script-multibyte-unix
+                   (managesieve-getscript sieve-script-multibyte-unix))))
+
+
+;;; sieve-manage-tests.el ends here
-- 
2.39.0

>From 6c63f891589e6525bf91ef53a0a8114a836a7781 Mon Sep 17 00:00:00 2001
From: Kai Tetzlaff <[email-masked]>
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