|
From: | Kai Tetzlaff |
Subject: | bug#54154: Emacs commit ae963e80a79f5a9184daabfc8197f211a39b136d (sieve-manage) |
Date: | Thu, 19 Jan 2023 16:59:36 +0100 |
User-agent: | Gnus/5.13 (Gnus v5.13) |
Eli Zaretskii <eliz@gnu.org> 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).
0001-Fix-bug-in-sieve-manage-append-to-log-improve-sieve-.patch
Description: Text Data
0002-Handle-BYE-in-sieve-manage-server-responses.patch
Description: Text Data
0003-Add-test-lisp-net-sieve-manage-tests.el.patch
Description: Text Data
0004-Some-minor-fixes-in-lisp-net-sieve.el.patch
Description: Text Data
[Prev in Thread] | Current Thread | [Next in Thread] |