unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
@ 2023-04-21 10:36 Vasilij Schneidermann
  2023-04-24  8:51 ` Robert Pluim
  0 siblings, 1 reply; 16+ messages in thread
From: Vasilij Schneidermann @ 2023-04-21 10:36 UTC (permalink / raw)
  To: 62990


[-- Attachment #1.1: Type: text/plain, Size: 3820 bytes --]

I'm doing network programming with Emacs Lisp and one of the tasks was
implementing a UDP server reading individual packets. I've found that
when a client sends an empty packet, the server hangs up with a
connection error and no longer accepts subsequent packets. This seems to
be reasonable behavior for TCP (from what I understand, empty TCP
messages are a sign of the connection being closed), but not for
UDP. I've attached test programs written in Emacs Lisp and Guile to
reproduce both correctly and incorrectly functioning servers and clients
respectively. One minor nitpick is that sending an empty UDP packet from
Emacs Lisp doesn't work either, but I'm unsure whether this is a bug
(socat behaves similarly).


In GNU Emacs 30.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version
 3.24.37, cairo version 1.17.8) of 2023-04-15 built on odonien
Repository revision: c60b59e04c3776a90adaa8c8fe53af3075a833b8
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101007
System Description: Arch Linux

Configured using:
 'configure 'CFLAGS=-g -ggdb -O0''

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY INOTIFY
PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS
TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LC_MESSAGES: 
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date subr-x mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process
emacs)

Memory information:
((conses 16 36403 15371)
 (symbols 48 5177 0)
 (strings 32 13201 2082)
 (string-bytes 1 377158)
 (vectors 16 9346)
 (vector-slots 8 149422 15364)
 (floats 8 21 24)
 (intervals 56 212 0)
 (buffers 984 10))

[-- Attachment #1.2: bug-client.el --]
[-- Type: text/plain, Size: 380 bytes --]

;; -*- lexical-binding: t; -*-

(defun make-client ()
  (make-network-process
   :name "bug-client"
   :type 'datagram
   :host "127.0.0.1"
   :service 10000
   :family 'ipv4
   :coding 'binary))

(defvar client (make-client))

(process-send-string client "hello")
(process-send-string client "")
(process-send-string client "world")

(while t
  (accept-process-output nil 0.01))

[-- Attachment #1.3: bug-server.el --]
[-- Type: text/plain, Size: 645 bytes --]

;; -*- lexical-binding: t; -*-

(defun server-filter (process string)
  (message "Received string (%s): %S" (length string) string))

(defun server-sentinel (process event)
  (message "Received event: %S" event)
  (when (eq (process-status process) 'closed)
    (delete-process process)
    (kill-buffer (process-buffer process))))

(defun make-server ()
  (make-network-process
   :name "bug-server"
   :type 'datagram
   :server t
   :host "127.0.0.1"
   :service 10000
   :family 'ipv4
   :coding 'binary
   :filter #'server-filter
   :sentinel #'server-sentinel))

(defvar server (make-server))

(while t
  (accept-process-output nil 0.01))

[-- Attachment #1.4: fixed-client.scm --]
[-- Type: text/plain, Size: 260 bytes --]

(import (rnrs bytevectors))

(define client (socket PF_INET SOCK_DGRAM 0))
(connect client AF_INET (inet-pton AF_INET "127.0.0.1") 10000)

(send client (string->utf8 "hello"))
(send client (string->utf8 ""))
(send client (string->utf8 "world"))
(close client)

[-- Attachment #1.5: fixed-server.scm --]
[-- Type: text/plain, Size: 452 bytes --]

(use-modules (ice-9 format))

(import (rnrs bytevectors))
(import (rnrs bytevectors gnu))
(import (srfi 18))

(define server (socket PF_INET SOCK_DGRAM 0))
(bind server AF_INET (inet-pton AF_INET "127.0.0.1") 10000)

(define buffer (make-u8vector 1024))

(let loop ()
  (let* ((len (car (recvfrom! server buffer)))
         (message (utf8->string (bytevector-slice buffer 0 len))))
    (format #t "Received string (~a): ~s~%" len message)
    (loop)))

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
  2023-04-21 10:36 bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet Vasilij Schneidermann
@ 2023-04-24  8:51 ` Robert Pluim
  2023-04-24 21:04   ` Vasilij Schneidermann
  0 siblings, 1 reply; 16+ messages in thread
From: Robert Pluim @ 2023-04-24  8:51 UTC (permalink / raw)
  To: Vasilij Schneidermann; +Cc: 62990

>>>>> On Fri, 21 Apr 2023 12:36:52 +0200, Vasilij Schneidermann <mail@vasilij.de> said:

    Vasilij> I'm doing network programming with Emacs Lisp and one of the tasks was
    Vasilij> implementing a UDP server reading individual packets. I've found that
    Vasilij> when a client sends an empty packet, the server hangs up with a
    Vasilij> connection error and no longer accepts subsequent packets. This seems to
    Vasilij> be reasonable behavior for TCP (from what I understand, empty TCP
    Vasilij> messages are a sign of the connection being closed), but
    Vasilij> not for UDP.

Empty TCP messages are perfectly valid, but they should be hidden from
you. recvfrom returning 0 means the connection has been closed, but
thatʼs a separate thing.

Empty UDP messages are also valid, and are indicated by recvfrom returning
0.

Could you show how youʼre generating the empty packets?

    Vasilij> I've attached test programs written in Emacs Lisp and Guile to
    Vasilij> reproduce both correctly and incorrectly functioning servers and clients
    Vasilij> respectively. One minor nitpick is that sending an empty UDP packet from
    Vasilij> Emacs Lisp doesn't work either, but I'm unsure whether this is a bug
    Vasilij> (socat behaves similarly).

Itʼs allowed by the protocol. I guess it could be useful for people
wanting to implement their own keep-alive protocol over UDP.

Robert
-- 





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
  2023-04-24  8:51 ` Robert Pluim
@ 2023-04-24 21:04   ` Vasilij Schneidermann
  2023-04-25  8:07     ` Robert Pluim
  0 siblings, 1 reply; 16+ messages in thread
From: Vasilij Schneidermann @ 2023-04-24 21:04 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 62990

[-- Attachment #1: Type: text/plain, Size: 737 bytes --]

> Empty TCP messages are perfectly valid, but they should be hidden from
> you. recvfrom returning 0 means the connection has been closed, but
> thatʼs a separate thing.

I suspect there's a missing ifdef in the code which ends up terminating
the process on such a condition, for both TCP and UDP.

> Could you show how youʼre generating the empty packets?

Download the attachments from my previous email, launch the server with
`emacs --batch -l bug-server.el`, then the client with `guile
fixed-client.scm`. Swap out the server with `guile fixed-server.scm`.

> Itʼs allowed by the protocol. I guess it could be useful for people
> wanting to implement their own keep-alive protocol over UDP.

I think so as well.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
  2023-04-24 21:04   ` Vasilij Schneidermann
@ 2023-04-25  8:07     ` Robert Pluim
  2023-04-26  9:14       ` Robert Pluim
  2023-04-26 10:56       ` Vasilij Schneidermann
  0 siblings, 2 replies; 16+ messages in thread
From: Robert Pluim @ 2023-04-25  8:07 UTC (permalink / raw)
  To: Vasilij Schneidermann; +Cc: 62990

>>>>> On Mon, 24 Apr 2023 23:04:34 +0200, Vasilij Schneidermann <mail@vasilij.de> said:

    >> Empty TCP messages are perfectly valid, but they should be hidden from
    >> you. recvfrom returning 0 means the connection has been closed, but
    >> thatʼs a separate thing.

    Vasilij> I suspect there's a missing ifdef in the code which ends up terminating
    Vasilij> the process on such a condition, for both TCP and UDP.

Yes, although in a different place than I expected. Patch below. The
end result is to ignore the zero length message, getting it delivered
to the process would involve considerably bigger changes.

    >> Could you show how youʼre generating the empty packets?

    Vasilij> Download the attachments from my previous email, launch the server with
    Vasilij> `emacs --batch -l bug-server.el`, then the client with `guile
    Vasilij> fixed-client.scm`. Swap out the server with `guile fixed-server.scm`.

I was hoping to avoid installing guile :-)

The client works, but the server gets me this, which means Iʼm missing
some bits somewhere:

    Backtrace:
               6 (primitive-load "/home/rpluim/repos/fixed-server.scm")
    In ice-9/eval.scm:
       721:20  5 (primitive-eval (import (rnrs bytevectors gnu)))
    In ice-9/psyntax.scm:
      1241:36  4 (expand-top-sequence ((import (rnrs bytevectors gnu))) _ …)
      1233:19  3 (parse _ (("placeholder" placeholder)) ((top) #(# # …)) …)
       285:10  2 (parse _ (("placeholder" placeholder)) (()) _ c&e (eval) …)
    In ice-9/eval.scm:
       293:34  1 (_ #<directory (guile-user) 7ff24fdbec80>)
    In ice-9/boot-9.scm:
       3300:6  0 (resolve-interface (rnrs bytevectors gnu) #:select _ # _ …)

    ice-9/boot-9.scm:3300:6: In procedure resolve-interface:
    no code for module (rnrs bytevectors gnu)

Does guile deliver the empty message?

    >> Itʼs allowed by the protocol. I guess it could be useful for people
    >> wanting to implement their own keep-alive protocol over UDP.

    Vasilij> I think so as well.

Iʼll look into it. I suspect it wonʼt be a small change.

Robert
-- 

diff --git a/src/process.c b/src/process.c
index 8e467ff7511..babe926ca5b 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5947,6 +5947,11 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 #endif /* HAVE_PTYS */
 	      /* If we can detect process termination, don't consider the
 		 process gone just because its pipe is closed.  */
+#ifdef DATAGRAM_SOCKETS
+	      /* A zero byte read on a UDP socket is not an error.  */
+	      else if (nread == 0 && DATAGRAM_CHAN_P (channel))
+		;
+#endif
 	      else if (nread == 0 && !NETCONN_P (proc) && !SERIALCONN_P (proc)
 		       && !PIPECONN_P (proc))
 		;






^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
  2023-04-25  8:07     ` Robert Pluim
@ 2023-04-26  9:14       ` Robert Pluim
  2023-04-26 10:30         ` Eli Zaretskii
  2023-04-26 10:56       ` Vasilij Schneidermann
  1 sibling, 1 reply; 16+ messages in thread
From: Robert Pluim @ 2023-04-26  9:14 UTC (permalink / raw)
  To: Vasilij Schneidermann; +Cc: 62990

[-- Attachment #1: Type: text/plain, Size: 344 bytes --]


So allowing emacs to send 0-length UDP packets turned out to be not
too hard. Patch for both below. Eli, is this OK for master? It does
change 2 behaviours:

1. Receiving a 0-length UDP packet will no longer cause an error
2. Calling (process-send-string proc "") now actually sends out a
packet for a UDP process (an empty one).


Robert
-- 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-zero-length-UDP-send-receive.patch --]
[-- Type: text/x-diff, Size: 2551 bytes --]

From 4bb6dec25ad3c3b548e18e82b2129b53c1ee1b69 Mon Sep 17 00:00:00 2001
From: Robert Pluim <rpluim@gmail.com>
Date: Tue, 25 Apr 2023 12:48:26 +0200
Subject: [PATCH] Allow zero-length UDP send/receive
To: emacs-devel@gnu.org

* src/process.c (wait_reading_process_output) [DATAGRAM_SOCKETS]:
Don't close the network connection if we receive a zero-length UDP
packet.
(send_process) [DATAGRAM_SOCKETS]: Allow sending a zero-length UDP
packet.

* etc/NEWS: Announce the change.
---
 etc/NEWS      |  8 ++++++++
 src/process.c | 16 ++++++++++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index ea233865f5a..4cc02d76b8f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -82,6 +82,14 @@ The ':keepalive-idle', ':keepalive-interval', and ':keepalive-count'
 options can now be used to set the various timers and counters used
 when TCP keepalive is enabled for a connection.
 
+** Emacs can now send zero-length UDP packets.
+Previously 'process-send-string' of an empty string to a UDP process
+did nothing, now it sends out a zero-length UDP packet.
+
+** Emacs can now receive zero-length UDP packets.
+Previously, receiving a zero-length UDP packet closed the receiving
+network process. They are now silently ignored.
+
 \f
 * Editing Changes in Emacs 30.1
 
diff --git a/src/process.c b/src/process.c
index 8e467ff7511..e3233f5ad89 100644
--- a/src/process.c
+++ b/src/process.c
@@ -5947,6 +5947,11 @@ wait_reading_process_output (intmax_t time_limit, int nsecs, int read_kbd,
 #endif /* HAVE_PTYS */
 	      /* If we can detect process termination, don't consider the
 		 process gone just because its pipe is closed.  */
+#ifdef DATAGRAM_SOCKETS
+	      /* A zero byte read on a UDP socket is not an error.  */
+	      else if (nread == 0 && DATAGRAM_CHAN_P (channel))
+		got_some_output = 1;
+#endif
 	      else if (nread == 0 && !NETCONN_P (proc) && !SERIALCONN_P (proc)
 		       && !PIPECONN_P (proc))
 		;
@@ -6616,9 +6621,16 @@ send_process (Lisp_Object proc, const char *buf, ptrdiff_t len,
 	  cur_buf = buf;
 	  cur_object = object;
 	}
-
-      while (cur_len > 0)
+      /* UDP allows zero-length writes.  */
+      bool zero_length = false;
+#ifdef DATAGRAM_SOCKETS
+      if (cur_len == 0 && NETCONN_P (proc) && 0 <= p->outfd
+	  && DATAGRAM_CHAN_P (p->outfd))
+	zero_length = true;
+#endif
+      while (cur_len > 0 || zero_length)
 	{
+	  zero_length = false;
 	  /* Send this batch, using one or more write calls.  */
 	  ptrdiff_t written = 0;
 	  int outfd = p->outfd;
-- 
2.38.1.420.g319605f8f0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
  2023-04-26  9:14       ` Robert Pluim
@ 2023-04-26 10:30         ` Eli Zaretskii
  2023-04-26 10:35           ` Robert Pluim
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2023-04-26 10:30 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 62990, mail

> Cc: 62990@debbugs.gnu.org
> From: Robert Pluim <rpluim@gmail.com>
> Date: Wed, 26 Apr 2023 11:14:42 +0200
> 
> So allowing emacs to send 0-length UDP packets turned out to be not
> too hard. Patch for both below. Eli, is this OK for master? It does
> change 2 behaviours:
> 
> 1. Receiving a 0-length UDP packet will no longer cause an error
> 2. Calling (process-send-string proc "") now actually sends out a
> packet for a UDP process (an empty one).

Do we want some variable to get back the old behavior, in case some
application out there depends on that?

Otherwise, fine by me, thanks.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
  2023-04-26 10:30         ` Eli Zaretskii
@ 2023-04-26 10:35           ` Robert Pluim
  2023-04-26 10:38             ` Eli Zaretskii
  2023-04-26 10:58             ` Vasilij Schneidermann
  0 siblings, 2 replies; 16+ messages in thread
From: Robert Pluim @ 2023-04-26 10:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 62990, mail

>>>>> On Wed, 26 Apr 2023 13:30:32 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> Cc: 62990@debbugs.gnu.org
    >> From: Robert Pluim <rpluim@gmail.com>
    >> Date: Wed, 26 Apr 2023 11:14:42 +0200
    >> 
    >> So allowing emacs to send 0-length UDP packets turned out to be not
    >> too hard. Patch for both below. Eli, is this OK for master? It does
    >> change 2 behaviours:
    >> 
    >> 1. Receiving a 0-length UDP packet will no longer cause an error
    >> 2. Calling (process-send-string proc "") now actually sends out a
    >> packet for a UDP process (an empty one).

    Eli> Do we want some variable to get back the old behavior, in case some
    Eli> application out there depends on that?

For which one? Does it help if I say that [1] is a security fix? 😜

I canʼt think of anything that would be negatively affected, but weʼve
all been wrong before.

    Eli> Otherwise, fine by me, thanks.

Vasilij, does the patch work for you?

Robert
-- 





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
  2023-04-26 10:35           ` Robert Pluim
@ 2023-04-26 10:38             ` Eli Zaretskii
  2023-04-26 11:47               ` Robert Pluim
  2023-04-26 10:58             ` Vasilij Schneidermann
  1 sibling, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2023-04-26 10:38 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 62990, mail

> From: Robert Pluim <rpluim@gmail.com>
> Cc: 62990@debbugs.gnu.org,  mail@vasilij.de
> Date: Wed, 26 Apr 2023 12:35:21 +0200
> 
> >>>>> On Wed, 26 Apr 2023 13:30:32 +0300, Eli Zaretskii <eliz@gnu.org> said:
> 
>     Eli> Do we want some variable to get back the old behavior, in case some
>     Eli> application out there depends on that?
> 
> For which one?

The 2nd one.

> Does it help if I say that [1] is a security fix? 😜

No.  But it will help if you convince me that it's a bug-fix.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
  2023-04-25  8:07     ` Robert Pluim
  2023-04-26  9:14       ` Robert Pluim
@ 2023-04-26 10:56       ` Vasilij Schneidermann
  2023-04-26 11:50         ` Robert Pluim
  1 sibling, 1 reply; 16+ messages in thread
From: Vasilij Schneidermann @ 2023-04-26 10:56 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 62990

[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]

> Yes, although in a different place than I expected. Patch below. The
> end result is to ignore the zero length message, getting it delivered
> to the process would involve considerably bigger changes.

Thank you. Yes, that is the case, the network process is no longer shut
down, but the 0-length message isn't picked up. It prints the following:

    Received string (5): "hello"
    Received string (5): "world"

Rather than the expected:

    Received string (5): "hello"
    Received string (0): ""
    Received string (5): "world"

> I was hoping to avoid installing guile :-)

I figured Guile would be the safest option for the developers of GNU
software, rather than requiring Python, Ruby, ...

> The client works, but the server gets me this, which means Iʼm missing
> some bits somewhere:
>     [...]
>     ice-9/boot-9.scm:3300:6: In procedure resolve-interface:
>     no code for module (rnrs bytevectors gnu)

Hm, could be an addition specific to Guile 3. It can be worked around
probably by doing some less efficient string copying.

> Does guile deliver the empty message?

Yes it does.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
  2023-04-26 10:35           ` Robert Pluim
  2023-04-26 10:38             ` Eli Zaretskii
@ 2023-04-26 10:58             ` Vasilij Schneidermann
  1 sibling, 0 replies; 16+ messages in thread
From: Vasilij Schneidermann @ 2023-04-26 10:58 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 62990, Eli Zaretskii

[-- Attachment #1: Type: text/plain, Size: 227 bytes --]

> Vasilij, does the patch work for you?

It does solve half of the problem: The network process is no longer shut
down, but the packet is ignored rather than resulting in an additional
process filter call with an empty string.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
  2023-04-26 10:38             ` Eli Zaretskii
@ 2023-04-26 11:47               ` Robert Pluim
  2023-04-26 12:24                 ` Eli Zaretskii
  0 siblings, 1 reply; 16+ messages in thread
From: Robert Pluim @ 2023-04-26 11:47 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 62990, mail

>>>>> On Wed, 26 Apr 2023 13:38:47 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Cc: 62990@debbugs.gnu.org,  mail@vasilij.de
    >> Date: Wed, 26 Apr 2023 12:35:21 +0200
    >> 
    >> >>>>> On Wed, 26 Apr 2023 13:30:32 +0300, Eli Zaretskii <eliz@gnu.org> said:
    >> 
    Eli> Do we want some variable to get back the old behavior, in case some
    Eli> application out there depends on that?
    >> 
    >> For which one?

    Eli> The 2nd one.

I guess we could, but if the application asks us to send an empty
packet we should do it.

    >> Does it help if I say that [1] is a security fix? 😜

    Eli> No.  But it will help if you convince me that it's a bug-fix.

It transforms this:

    recvfrom(5, "hello", 4096, 0, {sa_family=AF_INET, sin_port=htons(56845), sin_addr=inet_addr("127.0.0.1")}, [16]) = 5
    Received string (5): "hello"
    recvfrom(5, "", 4096, 0, {sa_family=AF_INET, sin_port=htons(56845), sin_addr=inet_addr("127.0.0.1")}, [16]) = 0
    Received event: "connection broken by remote peer

into this:

    recvfrom(5, "hello", 4096, 0, {sa_family=AF_INET, sin_port=htons(55486), sin_addr=inet_addr("127.0.0.1")}, [16]) = 5
    Received string (5): "hello"
    recvfrom(5, "", 4096, 0, {sa_family=AF_INET, sin_port=htons(55486), sin_addr=inet_addr("127.0.0.1")}, [16]) = 0
    recvfrom(5, "world", 4096, 0, {sa_family=AF_INET, sin_port=htons(55486), sin_addr=inet_addr("127.0.0.1")}, [16]) = 5
    Received string (5): "world"

ie the UDP process is still running and can still receive messages

Robert
-- 





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
  2023-04-26 10:56       ` Vasilij Schneidermann
@ 2023-04-26 11:50         ` Robert Pluim
  2023-04-26 14:31           ` Robert Pluim
  0 siblings, 1 reply; 16+ messages in thread
From: Robert Pluim @ 2023-04-26 11:50 UTC (permalink / raw)
  To: Vasilij Schneidermann; +Cc: 62990

>>>>> On Wed, 26 Apr 2023 12:56:39 +0200, Vasilij Schneidermann <mail@vasilij.de> said:

    Vasilij> Thank you. Yes, that is the case, the network process is no longer shut
    Vasilij> down, but the 0-length message isn't picked up. It prints the following:

    Vasilij>     Received string (5): "hello"
    Vasilij>     Received string (5): "world"

    Vasilij> Rather than the expected:

    Vasilij>     Received string (5): "hello"
    Vasilij>     Received string (0): ""
    Vasilij>     Received string (5): "world"

Right. Getting emacs to deliver a zero length string to the UDP
process would be quite a change, one I canʼt get to any time soon.

    >> Does guile deliver the empty message?

    Vasilij> Yes it does.

OK.

Robert
-- 





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
  2023-04-26 11:47               ` Robert Pluim
@ 2023-04-26 12:24                 ` Eli Zaretskii
  2023-04-26 14:27                   ` Robert Pluim
  0 siblings, 1 reply; 16+ messages in thread
From: Eli Zaretskii @ 2023-04-26 12:24 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 62990, mail

> From: Robert Pluim <rpluim@gmail.com>
> Cc: 62990@debbugs.gnu.org,  mail@vasilij.de
> Date: Wed, 26 Apr 2023 13:47:49 +0200
> 
> >>>>> On Wed, 26 Apr 2023 13:38:47 +0300, Eli Zaretskii <eliz@gnu.org> said:
> 
>     >> From: Robert Pluim <rpluim@gmail.com>
>     >> Cc: 62990@debbugs.gnu.org,  mail@vasilij.de
>     >> Date: Wed, 26 Apr 2023 12:35:21 +0200
>     >> 
>     >> >>>>> On Wed, 26 Apr 2023 13:30:32 +0300, Eli Zaretskii <eliz@gnu.org> said:
>     >> 
>     Eli> Do we want some variable to get back the old behavior, in case some
>     Eli> application out there depends on that?
>     >> 
>     >> For which one?
> 
>     Eli> The 2nd one.
> 
> I guess we could, but if the application asks us to send an empty
> packet we should do it.

I didn't say that the default behavior should be that, I just asked
whether it is conceivable that someone, somewhere could actually rely
on the semi-buggy behavior, whereby empty packets aren't sent?

>     >> Does it help if I say that [1] is a security fix? 😜
> 
>     Eli> No.  But it will help if you convince me that it's a bug-fix.
> 
> It transforms this:
> 
>     recvfrom(5, "hello", 4096, 0, {sa_family=AF_INET, sin_port=htons(56845), sin_addr=inet_addr("127.0.0.1")}, [16]) = 5
>     Received string (5): "hello"
>     recvfrom(5, "", 4096, 0, {sa_family=AF_INET, sin_port=htons(56845), sin_addr=inet_addr("127.0.0.1")}, [16]) = 0
>     Received event: "connection broken by remote peer
> 
> into this:
> 
>     recvfrom(5, "hello", 4096, 0, {sa_family=AF_INET, sin_port=htons(55486), sin_addr=inet_addr("127.0.0.1")}, [16]) = 5
>     Received string (5): "hello"
>     recvfrom(5, "", 4096, 0, {sa_family=AF_INET, sin_port=htons(55486), sin_addr=inet_addr("127.0.0.1")}, [16]) = 0
>     recvfrom(5, "world", 4096, 0, {sa_family=AF_INET, sin_port=htons(55486), sin_addr=inet_addr("127.0.0.1")}, [16]) = 5
>     Received string (5): "world"
> 
> ie the UDP process is still running and can still receive messages

And no one could ever expect that?  IOW, is this behavior obviously
wrong?





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
  2023-04-26 12:24                 ` Eli Zaretskii
@ 2023-04-26 14:27                   ` Robert Pluim
  0 siblings, 0 replies; 16+ messages in thread
From: Robert Pluim @ 2023-04-26 14:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 62990, mail

>>>>> On Wed, 26 Apr 2023 15:24:03 +0300, Eli Zaretskii <eliz@gnu.org> said:
    >> 
    >> I guess we could, but if the application asks us to send an empty
    >> packet we should do it.

    Eli> I didn't say that the default behavior should be that, I just asked
    Eli> whether it is conceivable that someone, somewhere could actually rely
    Eli> on the semi-buggy behavior, whereby empty packets aren't sent?

I canʼt conceive of it. If it ever does happen the fix is easy: check
the length of the message before sending it.

    >> 
    >> ie the UDP process is still running and can still receive messages

    Eli> And no one could ever expect that?  IOW, is this behavior obviously
    Eli> wrong?

Yes. The other end has sent a valid UDP packet, and we react by
closing the connection.

Robert
-- 





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
  2023-04-26 11:50         ` Robert Pluim
@ 2023-04-26 14:31           ` Robert Pluim
  2024-03-09 13:40             ` Vasilij Schneidermann
  0 siblings, 1 reply; 16+ messages in thread
From: Robert Pluim @ 2023-04-26 14:31 UTC (permalink / raw)
  To: Vasilij Schneidermann; +Cc: 62990

>>>>> On Wed, 26 Apr 2023 13:50:17 +0200, Robert Pluim <rpluim@gmail.com> said:

>>>>> On Wed, 26 Apr 2023 12:56:39 +0200, Vasilij Schneidermann <mail@vasilij.de> said:
    Vasilij> Thank you. Yes, that is the case, the network process is no longer shut
    Vasilij> down, but the 0-length message isn't picked up. It prints the following:

    Vasilij> Received string (5): "hello"
    Vasilij> Received string (5): "world"

    Vasilij> Rather than the expected:

    Vasilij> Received string (5): "hello"
    Vasilij> Received string (0): ""
    Vasilij> Received string (5): "world"

    Robert> Right. Getting emacs to deliver a zero length string to the UDP
    Robert> process would be quite a change, one I canʼt get to any time soon.

Actually, it turned out to be a small change (on top of my previous
patch)

diff --git a/src/process.c b/src/process.c
index e3233f5ad89..eca1441062d 100644
--- a/src/process.c
+++ b/src/process.c
@@ -6305,7 +6305,13 @@ read_and_dispose_of_process_output (struct Lisp_Process *p, char *chars,
 	      coding->carryover_bytes);
       p->decoding_carryover = coding->carryover_bytes;
     }
-  if (SBYTES (text) > 0)
+  if (SBYTES (text) > 0
+#ifdef DATAGRAM_SOCKETS
+      || (SBYTES (text) == 0
+	  && 0 <= p->outfd
+	  && DATAGRAM_CHAN_P (p->outfd))
+#endif
+      )
     /* FIXME: It's wrong to wrap or not based on debug-on-error, and
        sometimes it's simply wrong to wrap (e.g. when called from
        accept-process-output).  */

Robert
-- 





^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet
  2023-04-26 14:31           ` Robert Pluim
@ 2024-03-09 13:40             ` Vasilij Schneidermann
  0 siblings, 0 replies; 16+ messages in thread
From: Vasilij Schneidermann @ 2024-03-09 13:40 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 62990

[-- Attachment #1: Type: text/plain, Size: 1219 bytes --]

Hello again,

> Actually, it turned out to be a small change (on top of my previous
> patch)
> 
> diff --git a/src/process.c b/src/process.c
> index e3233f5ad89..eca1441062d 100644
> --- a/src/process.c
> +++ b/src/process.c
> @@ -6305,7 +6305,13 @@ read_and_dispose_of_process_output (struct Lisp_Process *p, char *chars,
>  	      coding->carryover_bytes);
>        p->decoding_carryover = coding->carryover_bytes;
>      }
> -  if (SBYTES (text) > 0)
> +  if (SBYTES (text) > 0
> +#ifdef DATAGRAM_SOCKETS
> +      || (SBYTES (text) == 0
> +	  && 0 <= p->outfd
> +	  && DATAGRAM_CHAN_P (p->outfd))
> +#endif
> +      )
>      /* FIXME: It's wrong to wrap or not based on debug-on-error, and
>         sometimes it's simply wrong to wrap (e.g. when called from
>         accept-process-output).  */

I somehow completely overlooked this second patch and can confirm that
with it, both the Emacs Lisp and Guile version of the server/client
behave identically. In other words, the UDP bug is completely fixed now.

I've done a cursory search on the web and did not find anything that
would send empty packets via UDP, so I do doubt it would break existing
code or need a opt-out setting.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-03-09 13:40 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-21 10:36 bug#62990: 30.0.50; UDP server closes connection upon receiving an empty packet Vasilij Schneidermann
2023-04-24  8:51 ` Robert Pluim
2023-04-24 21:04   ` Vasilij Schneidermann
2023-04-25  8:07     ` Robert Pluim
2023-04-26  9:14       ` Robert Pluim
2023-04-26 10:30         ` Eli Zaretskii
2023-04-26 10:35           ` Robert Pluim
2023-04-26 10:38             ` Eli Zaretskii
2023-04-26 11:47               ` Robert Pluim
2023-04-26 12:24                 ` Eli Zaretskii
2023-04-26 14:27                   ` Robert Pluim
2023-04-26 10:58             ` Vasilij Schneidermann
2023-04-26 10:56       ` Vasilij Schneidermann
2023-04-26 11:50         ` Robert Pluim
2023-04-26 14:31           ` Robert Pluim
2024-03-09 13:40             ` Vasilij Schneidermann

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).