patch 9.1.0164: Internal error when passing invalid position to getregion()

Problem:  Internal error or crash when passing invalid position to
          getregion().
Solution: Give an error for invalid position (zeertzjq).

closes: #14172

Signed-off-by: zeertzjq <zeertzjq@outlook.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

diff --git a/src/errors.h b/src/errors.h
index 0dbc5a571941a46b7b4890eb004f3f590b0dc994..6e7f28d899b2dec4b1f1a156a1f333cd5abf1dc6 100644 (file)
--- a/src/errors.h
+++ b/src/errors.h
@@ -2526,9 +2526,11 @@ EXTERN char e_invalid_action_str_2[]
 EXTERN char e_setting_v_str_to_value_with_wrong_type[]
        INIT(= N_("E963: Setting v:%s to value with wrong type"));
 #endif
-#ifdef FEAT_PROP_POPUP
+#if defined(FEAT_PROP_POPUP) || defined(FEAT_EVAL)
 EXTERN char_u e_invalid_column_number_nr[]
        INIT(= N_("E964: Invalid column number: %ld"));
+#endif
+#ifdef FEAT_PROP_POPUP
 EXTERN char e_missing_property_type_name[]
        INIT(= N_("E965: Missing property type name"));
 #endif

diff --git a/src/evalfunc.c b/src/evalfunc.c
index 2bb9ccec55347fee714cc90b6b9757761e0bb0dd..1b394f00a5d49e5b7eed465cd9c5b6cfab247c2c 100644 (file)
--- a/src/evalfunc.c
+++ b/src/evalfunc.c
@@ -5495,6 +5495,7 @@ f_getregion(typval_T *argvars, typval_T *rettv)
     pos_T              p1, p2;
     char_u             *type;
     buf_T              *save_curbuf = curbuf;
+    buf_T              *findbuf = curbuf;
     char_u             default_type[] = "v";
     int                        save_virtual = -1;
     int                        l;
@@ -5536,19 +5537,44 @@ f_getregion(typval_T *argvars, typval_T *rettv)
     else if (type[0] == Ctrl_V && type[1] == NUL)
        region_type = MBLOCK;
     else
+    {
+       semsg(_(e_invalid_value_for_argument_str_str), "type", type);
        return;
+    }
 
     if (fnum1 != 0)
     {
-       buf_T   *findbuf;
-
        findbuf = buflist_findnr(fnum1);
        // buffer not loaded
        if (findbuf == NULL || findbuf->b_ml.ml_mfp == NULL)
+       {
+           emsg(_(e_buffer_is_not_loaded));
            return;
-       curbuf = findbuf;
+       }
+    }
+
+    if (p1.lnum < 1 || p1.lnum > findbuf->b_ml.ml_line_count)
+    {
+       semsg(_(e_invalid_line_number_nr), p1.lnum);
+       return;
+    }
+    if (p1.col < 1 || p1.col > ml_get_buf_len(findbuf, p1.lnum) + 1)
+    {
+       semsg(_(e_invalid_column_number_nr), p1.col);
+       return;
+    }
+    if (p2.lnum < 1 || p2.lnum > findbuf->b_ml.ml_line_count)
+    {
+       semsg(_(e_invalid_line_number_nr), p2.lnum);
+       return;
+    }
+    if (p2.col < 1 || p2.col > ml_get_buf_len(findbuf, p2.lnum) + 1)
+    {
+       semsg(_(e_invalid_column_number_nr), p2.col);
+       return;
     }
 
+    curbuf = findbuf;
     save_virtual = virtual_op;
     virtual_op = virtual_active();
 
@@ -5582,7 +5608,7 @@ f_getregion(typval_T *argvars, typval_T *rettv)
            else if (p2.lnum > 1)
            {
                p2.lnum--;
-               p2.col = (colnr_T)STRLEN(ml_get(p2.lnum));
+               p2.col = ml_get_len(p2.lnum);
                if (p2.col > 0)
                {
                    p2.col--;

diff --git a/src/testdir/test_visual.vim b/src/testdir/test_visual.vim
index b80f43eedd384a7fdb532ea199f7f38a2a306289..ca13966975a6105ad72cacf7e57034c086958500 100644 (file)
--- a/src/testdir/test_visual.vim
+++ b/src/testdir/test_visual.vim
@@ -1737,40 +1737,60 @@ func Test_visual_getregion()
           \ getregion(getpos('v'), getpos('.'), {'type': "\<C-v>" }))
     set virtualedit&
 
-    #" Invalid position
+    #" using wrong types for positions
     call cursor(1, 1)
     call feedkeys("\<ESC>vjj$", 'tx')
     call assert_fails("call getregion(1, 2)", 'E1211:')
     call assert_fails("call getregion(getpos('.'), {})", 'E1211:')
-    call assert_equal([], getregion(getpos('.'), getpos('.'), {'type': '' }))
-
-    #" using the wrong type
     call assert_fails(':echo "."->getpos()->getregion("$", [])', 'E1211:')
 
+    #" using invalid value for "type"
+    call assert_fails("call getregion(getpos('.'), getpos('.'), {'type': '' })", 'E475:')
+
     #" using a mark from another buffer to current buffer
     new
-    VAR newbuf = bufnr()
+    LET g:buf = bufnr()
     call setline(1, range(10))
     normal! GmA
     wincmd p
-    call assert_equal([newbuf, 10, 1, 0], getpos("'A"))
+    call assert_equal([g:buf, 10, 1, 0], getpos("'A"))
     call assert_equal([], getregion(getpos('.'), getpos("'A"), {'type': 'v' }))
     call assert_equal([], getregion(getpos("'A"), getpos('.'), {'type': 'v' }))
-    exe $':{newbuf}bwipe!'
 
-    #" using a mark from another buffer to another buffer
-    new
-    VAR anotherbuf = bufnr()
-    call setline(1, range(10))
-    normal! GmA
+    #" using two marks from another buffer
+    wincmd p
     normal! GmB
     wincmd p
-    call assert_equal([anotherbuf, 10, 1, 0], getpos("'A"))
+    call assert_equal([g:buf, 10, 1, 0], getpos("'B"))
     call assert_equal(['9'], getregion(getpos("'B"), getpos("'A"), {'type': 'v' }))
-    exe $':{anotherbuf}bwipe!'
+
+    #" using two positions from another buffer
+    for type in ['v', 'V', "\<C-V>"]
+      for exclusive in [v:false, v:true]
+        call assert_equal(range(10)->mapnew('string(v:val)'),
+              \ getregion([g:buf, 1, 1, 0], [g:buf, 10, 2, 0]),
+              \ {'type': type, 'exclusive': exclusive })
+        call assert_equal(range(10)->mapnew('string(v:val)'),
+              \ getregion([g:buf, 10, 2, 0], [g:buf, 1, 1, 0]),
+              \ {'type': type, 'exclusive': exclusive })
+      endfor
+    endfor
+
+    #" using invalid positions in buffer
+    call assert_fails('call getregion([g:buf, 0, 1, 0], [g:buf, 10, 2, 0])', 'E966:')
+    call assert_fails('call getregion([g:buf, 10, 2, 0], [g:buf, 0, 1, 0])', 'E966:')
+    call assert_fails('call getregion([g:buf, 1, 1, 0], [g:buf, 11, 2, 0])', 'E966:')
+    call assert_fails('call getregion([g:buf, 11, 2, 0], [g:buf, 1, 1, 0])', 'E966:')
+    call assert_fails('call getregion([g:buf, 1, 1, 0], [g:buf, 10, 0, 0])', 'E964:')
+    call assert_fails('call getregion([g:buf, 10, 0, 0], [g:buf, 1, 1, 0])', 'E964:')
+    call assert_fails('call getregion([g:buf, 1, 1, 0], [g:buf, 10, 3, 0])', 'E964:')
+    call assert_fails('call getregion([g:buf, 10, 3, 0], [g:buf, 1, 1, 0])', 'E964:')
 
     #" using invalid buffer
-    call assert_equal([], getregion([10000, 10, 1, 0], [10000, 10, 1, 0]))
+    call assert_fails('call getregion([10000, 10, 1, 0], [10000, 10, 1, 0])', 'E681:')
+
+    exe $':{g:buf}bwipe!'
+    unlet g:buf
   END
   call v9.CheckLegacyAndVim9Success(lines)
 
@@ -1931,7 +1951,7 @@ func Test_getregion_invalid_buf()
   call assert_equal(['Move around:'], getregion(getpos("'A"), getpos("'B")))
   " close the help window
   q
-  call assert_equal([], getregion(getpos("'A"), getpos("'B")))
+  call assert_fails("call getregion(getpos(\"'A\"), getpos(\"'B\"))", 'E681:')
   bwipe!
 endfunc
 

diff --git a/src/version.c b/src/version.c
index d1ca31bf65999d50cf6be6791f11aeb21e714798..b46b3558607ae311bab35af6e621d30bf6c7d2a8 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -704,6 +704,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    164,
 /**/
     163,
 /**/