patch 8.1.0993: ch_read() may return garbage if terminating NL is missing

Problem:    ch_read() may return garbage if terminating NL is missing.
Solution:   Add terminating NUL. (Ozaki Kiichi, closes #4065)

diff --git a/src/channel.c b/src/channel.c
index a9e44113954627e9d6eec0ac8ae5fbb77e993a9b..176a3d7e1ededfa588a495afc47d52ea85111e31 100644 (file)
--- a/src/channel.c
+++ b/src/channel.c
@@ -1797,6 +1797,7 @@ channel_consume(channel_T *channel, ch_part_T part, int len)
 
     mch_memmove(buf, buf + len, node->rq_buflen - len);
     node->rq_buflen -= len;
+    node->rq_buffer[node->rq_buflen] = NUL;
 }
 
 /*
@@ -1819,7 +1820,7 @@ channel_collapse(channel_T *channel, ch_part_T part, int want_nl)
        return FAIL;
 
     last_node = node->rq_next;
-    len = node->rq_buflen + last_node->rq_buflen + 1;
+    len = node->rq_buflen + last_node->rq_buflen;
     if (want_nl)
        while (last_node->rq_next != NULL
                && channel_first_nl(last_node) == NULL)
@@ -1828,7 +1829,7 @@ channel_collapse(channel_T *channel, ch_part_T part, int want_nl)
            len += last_node->rq_buflen;
        }
 
-    p = newbuf = alloc(len);
+    p = newbuf = alloc(len + 1);
     if (newbuf == NULL)
        return FAIL;        /* out of memory */
     mch_memmove(p, node->rq_buffer, node->rq_buflen);
@@ -1842,6 +1843,7 @@ channel_collapse(channel_T *channel, ch_part_T part, int want_nl)
        p += n->rq_buflen;
        vim_free(n->rq_buffer);
     }
+    *p = NUL;
     node->rq_buflen = (long_u)(p - newbuf);
 
     /* dispose of the collapsed nodes and their buffers */
@@ -2666,30 +2668,20 @@ may_invoke_callback(channel_T *channel, ch_part_T part)
            }
            buf = node->rq_buffer;
 
-           if (nl == NULL)
-           {
-               /* Flush remaining message that is missing a NL. */
-               char_u  *new_buf;
-
-               new_buf = vim_realloc(buf, node->rq_buflen + 1);
-               if (new_buf == NULL)
-                   /* This might fail over and over again, should the message
-                    * be dropped? */
-                   return FALSE;
-               buf = new_buf;
-               node->rq_buffer = buf;
-               nl = buf + node->rq_buflen++;
-               *nl = NUL;
-           }
-
-           /* Convert NUL to NL, the internal representation. */
-           for (p = buf; p < nl && p < buf + node->rq_buflen; ++p)
+           // Convert NUL to NL, the internal representation.
+           for (p = buf; (nl == NULL || p < nl)
+                                           && p < buf + node->rq_buflen; ++p)
                if (*p == NUL)
                    *p = NL;
 
-           if (nl + 1 == buf + node->rq_buflen)
+           if (nl == NULL)
+           {
+               // get the whole buffer, drop the NL
+               msg = channel_get(channel, part, NULL);
+           }
+           else if (nl + 1 == buf + node->rq_buflen)
            {
-               /* get the whole buffer, drop the NL */
+               // get the whole buffer
                msg = channel_get(channel, part, NULL);
                *nl = NUL;
            }

diff --git a/src/testdir/test_channel.vim b/src/testdir/test_channel.vim
index 67163e606b0a99f5aff682d00909add8c26557f0..2f2b5597fb31b3231ae5477f149b57138c94937b 100644 (file)
--- a/src/testdir/test_channel.vim
+++ b/src/testdir/test_channel.vim
@@ -203,8 +203,7 @@ func Ch_communicate(port)
   let start = reltime()
   call assert_equal(v:none, ch_read(handle, {'timeout': 333}))
   let elapsed = reltime(start)
-  call assert_true(reltimefloat(elapsed) > 0.3)
-  call assert_true(reltimefloat(elapsed) < 0.6)
+  call assert_inrange(0.3, 0.6, reltimefloat(reltime(start)))
 
   " Send without waiting for a response, then wait for a response.
   call ch_sendexpr(handle, 'wait a bit')
@@ -434,9 +433,7 @@ func Test_connect_waittime()
     else
       " Failed connection should wait about 500 msec.  Can be longer if the
       " computer is busy with other things.
-      let elapsed = reltime(start)
-      call assert_true(reltimefloat(elapsed) > 0.3)
-      call assert_true(reltimefloat(elapsed) < 1.5)
+      call assert_inrange(0.3, 1.5, reltimefloat(reltime(start)))
     endif
   catch
     if v:exception !~ 'Connection reset by peer'
@@ -1590,8 +1587,7 @@ func Test_exit_callback_interval()
   else
     let elapsed = 1.0
   endif
-  call assert_true(elapsed > 0.5)
-  call assert_true(elapsed < 1.0)
+  call assert_inrange(0.5, 1.0, elapsed)
 endfunc
 
 """""""""
@@ -1764,10 +1760,6 @@ func Test_raw_passes_nul()
   bwipe!
 endfunc
 
-func MyLineCountCb(ch, msg)
-  let g:linecount += 1
-endfunc
-
 func Test_read_nonl_line()
   if !has('job')
     return
@@ -1775,8 +1767,28 @@ func Test_read_nonl_line()
 
   let g:linecount = 0
   let arg = 'import sys;sys.stdout.write("1\n2\n3")'
-  call job_start([s:python, '-c', arg], {'callback': 'MyLineCountCb'})
+  call job_start([s:python, '-c', arg], {'callback': {-> execute('let g:linecount += 1')}})
   call WaitForAssert({-> assert_equal(3, g:linecount)})
+  unlet g:linecount
+endfunc
+
+func Test_read_nonl_in_close_cb()
+  if !has('job')
+    return
+  endif
+
+  func s:close_cb(ch)
+    while ch_status(a:ch) == 'buffered'
+      let g:out .= ch_read(a:ch)
+    endwhile
+  endfunc
+
+  let g:out = ''
+  let arg = 'import sys;sys.stdout.write("1\n2\n3")'
+  call job_start([s:python, '-c', arg], {'close_cb': function('s:close_cb')})
+  call WaitForAssert({-> assert_equal('123', g:out)})
+  unlet g:out
+  delfunc s:close_cb
 endfunc
 
 func Test_read_from_terminated_job()
@@ -1786,8 +1798,9 @@ func Test_read_from_terminated_job()
 
   let g:linecount = 0
   let arg = 'import os,sys;os.close(1);sys.stderr.write("test\n")'
-  call job_start([s:python, '-c', arg], {'callback': 'MyLineCountCb'})
+  call job_start([s:python, '-c', arg], {'callback': {-> execute('let g:linecount += 1')}})
   call WaitForAssert({-> assert_equal(1, g:linecount)})
+  unlet g:linecount
 endfunc
 
 func Test_job_start_windows()

diff --git a/src/version.c b/src/version.c
index 4ac981e6a01c68eeee3363519a9868d5719d5851..8722a832c7f4a3d451316e5823242eb159dbb4f9 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -779,6 +779,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    993,
 /**/
     992,
 /**/