Skip to content

Commit

Permalink
patch 8.0.1647: terminal API may call any user function
Browse files Browse the repository at this point in the history
Problem:    Terminal API may call a function not meant to be called by this
            API.
Solution:   Require the function to start with Tapi_.
  • Loading branch information
brammool committed Mar 26, 2018
1 parent 4368d5c commit 2a77d21
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 17 deletions.
24 changes: 15 additions & 9 deletions runtime/doc/terminal.txt
Expand Up @@ -423,20 +423,26 @@ Currently supported commands:

call {funcname} {argument}

Call a user defined function with [argument]. The function is
called with the buffer number of the terminal and the decoded
argument. The user function must sanity check the argument.
Call a user defined function with {argument}.
The function is called with two arguments: the buffer number
of the terminal and {argument}, the decoded JSON argument.
The function name must start with "Tapi_" to avoid
accidentally calling a function not meant to be used for the
terminal API
The user function should sanity check the argument.
The function can use |term_sendkeys()| to send back a reply.
Example in JSON: >
["call", "Impression", ["play", 14]]
["call", "Tapi_Impression", ["play", 14]]
< Calls a function defined like this: >
function Impression(bufnum, arglist)
function Tapi_Impression(bufnum, arglist)
if len(a:arglist) == 2
echo "impression " . a:arglist[0]
echo "count " . a:arglist[1]
echomsg "impression " . a:arglist[0]
echomsg "count " . a:arglist[1]
endif
endfunc
<
< Output from `:echo` may be erased by a redraw, use `:echomsg`
to be able to see it with `:messages`.

drop {filename}

Let Vim open a file, like the `:drop` command. If {filename}
Expand All @@ -447,7 +453,7 @@ Currently supported commands:
A trick to have Vim send this escape sequence: >
exe "set t_ts=\<Esc>]51; t_fs=\x07"
let &titlestring = '["call","TryThis",["hello",123]]'
let &titlestring = '["call","Tapi_TryThis",["hello",123]]'
redraw
set t_ts& t_fs&
Expand Down
2 changes: 1 addition & 1 deletion src/terminal.c
Expand Up @@ -3193,7 +3193,7 @@ handle_call_command(term_T *term, channel_T *channel, listitem_T *item)
}
func = get_tv_string(&item->li_tv);

if (!ASCII_ISUPPER(*func))
if (STRNCMP(func, "Tapi_", 5) != 0)
{
ch_log(channel, "Invalid function name: %s", func);
return;
Expand Down
34 changes: 27 additions & 7 deletions src/testdir/test_terminal.vim
Expand Up @@ -1072,24 +1072,28 @@ func Test_terminal_api_drop_oldwin()
bwipe Xtextfile
endfunc

func TryThis(bufnum, arg)
func Tapi_TryThis(bufnum, arg)
let g:called_bufnum = a:bufnum
let g:called_arg = a:arg
endfunc

func Test_terminal_api_call()
if !CanRunVimInTerminal()
return
endif

func WriteApiCall(funcname)
" Use the title termcap entries to output the escape sequence.
call writefile([
\ 'set title',
\ 'exe "set t_ts=\<Esc>]51; t_fs=\x07"',
\ 'let &titlestring = ''["call","TryThis",["hello",123]]''',
\ 'let &titlestring = ''["call","' . a:funcname . '",["hello",123]]''',
\ 'redraw',
\ "set t_ts=",
\ ], 'Xscript')
endfunc

func Test_terminal_api_call()
if !CanRunVimInTerminal()
return
endif

call WriteApiCall('Tapi_TryThis')
let buf = RunVimInTerminal('-S Xscript', {})
call WaitFor({-> exists('g:called_bufnum')})
call assert_equal(buf, g:called_bufnum)
Expand All @@ -1100,3 +1104,19 @@ func Test_terminal_api_call()
unlet g:called_bufnum
unlet g:called_arg
endfunc

func Test_terminal_api_call_fails()
if !CanRunVimInTerminal()
return
endif

call WriteApiCall('TryThis')
call ch_logfile('Xlog', 'w')
let buf = RunVimInTerminal('-S Xscript', {})
call WaitFor({-> string(readfile('Xlog')) =~ 'Invalid function name: TryThis'})

call StopVimInTerminal(buf)
call delete('Xscript')
call ch_logfile('', '')
call delete('Xlog')
endfunc
2 changes: 2 additions & 0 deletions src/version.c
Expand Up @@ -766,6 +766,8 @@ static char *(features[]) =

static int included_patches[] =
{ /* Add new patch number below this line */
/**/
1647,
/**/
1646,
/**/
Expand Down

5 comments on commit 2a77d21

@mattn
Copy link
Member

@mattn mattn commented on 2a77d21 Mar 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brammool Thanks for fixing this. But I think that it's better to be callback function related on the job options like below. How do you think?

call job_setoptions(job, {"response_cb", Fn)

@mattn
Copy link
Member

@mattn mattn commented on 2a77d21 Mar 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or this is better to be an option for term_start.

@brammool
Copy link
Contributor Author

@brammool brammool commented on 2a77d21 Mar 27, 2018 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mattn
Copy link
Member

@mattn mattn commented on 2a77d21 Mar 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When start terminals A and B, and want to handle response messages from only A, it have to look argument a:bufnum?

@brammool
Copy link
Contributor Author

@brammool brammool commented on 2a77d21 Mar 27, 2018 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.