To: vim_dev@googlegroups.com Subject: Patch 8.1.0544 Fcc: outbox From: Bram Moolenaar Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ------------ Patch 8.1.0544 (after 8.1.0540) Problem: Setting 'filetype' in a modeline causes an error (Hirohito Higashi). Solution: Don't add the P_INSECURE flag when setting 'filetype' from a modeline. Also for 'syntax'. Files: src/option.c, src/testdir/test_modeline.vim *** ../vim-8.1.0543/src/option.c 2018-11-22 03:07:30.948596188 +0100 --- src/option.c 2018-11-25 02:06:48.284746758 +0100 *************** *** 3284,3290 **** static void set_options_default(int opt_flags); static void set_string_default_esc(char *name, char_u *val, int escape); static char_u *term_bg_default(void); ! static void did_set_option(int opt_idx, int opt_flags, int new_value); static char_u *option_expand(int opt_idx, char_u *val); static void didset_options(void); static void didset_options2(void); --- 3284,3290 ---- static void set_options_default(int opt_flags); static void set_string_default_esc(char *name, char_u *val, int escape); static char_u *term_bg_default(void); ! static void did_set_option(int opt_idx, int opt_flags, int new_value, int value_checked); static char_u *option_expand(int opt_idx, char_u *val); static void didset_options(void); static void didset_options2(void); *************** *** 3295,3301 **** # define insecure_flag(opt_idx, opt_flags) (&options[opt_idx].flags) #endif static void set_string_option_global(int opt_idx, char_u **varp); ! static char_u *did_set_string_option(int opt_idx, char_u **varp, int new_value_alloced, char_u *oldval, char_u *errbuf, int opt_flags); static char_u *set_chars_option(char_u **varp); #ifdef FEAT_CLIPBOARD static char_u *check_clipboard_option(void); --- 3295,3301 ---- # define insecure_flag(opt_idx, opt_flags) (&options[opt_idx].flags) #endif static void set_string_option_global(int opt_idx, char_u **varp); ! static char_u *did_set_string_option(int opt_idx, char_u **varp, int new_value_alloced, char_u *oldval, char_u *errbuf, int opt_flags, int *value_checked); static char_u *set_chars_option(char_u **varp); #ifdef FEAT_CLIPBOARD static char_u *check_clipboard_option(void); *************** *** 4706,4711 **** --- 4706,4712 ---- else { int value_is_replaced = !prepending && !adding && !removing; + int value_checked = FALSE; if (flags & P_BOOL) /* boolean */ { *************** *** 5236,5242 **** // or 'filetype' autocommands may be triggered that can // cause havoc. errmsg = did_set_string_option(opt_idx, (char_u **)varp, ! new_value_alloced, oldval, errbuf, opt_flags); if (did_inc_secure) --secure; --- 5237,5244 ---- // or 'filetype' autocommands may be triggered that can // cause havoc. errmsg = did_set_string_option(opt_idx, (char_u **)varp, ! new_value_alloced, oldval, errbuf, ! opt_flags, &value_checked); if (did_inc_secure) --secure; *************** *** 5280,5286 **** } if (opt_idx >= 0) ! did_set_option(opt_idx, opt_flags, value_is_replaced); } skip: --- 5282,5289 ---- } if (opt_idx >= 0) ! did_set_option( ! opt_idx, opt_flags, value_is_replaced, value_checked); } skip: *************** *** 5348,5355 **** static void did_set_option( int opt_idx, ! int opt_flags, /* possibly with OPT_MODELINE */ ! int new_value) /* value was replaced completely */ { long_u *p; --- 5351,5360 ---- static void did_set_option( int opt_idx, ! int opt_flags, // possibly with OPT_MODELINE ! int new_value, // value was replaced completely ! int value_checked) // value was checked to be safe, no need to set the ! // P_INSECURE flag. { long_u *p; *************** *** 5359,5369 **** * set the P_INSECURE flag. Otherwise, if a new value is stored reset the * flag. */ p = insecure_flag(opt_idx, opt_flags); ! if (secure #ifdef HAVE_SANDBOX || sandbox != 0 #endif ! || (opt_flags & OPT_MODELINE)) *p = *p | P_INSECURE; else if (new_value) *p = *p & ~P_INSECURE; --- 5364,5374 ---- * set the P_INSECURE flag. Otherwise, if a new value is stored reset the * flag. */ p = insecure_flag(opt_idx, opt_flags); ! if (!value_checked && (secure #ifdef HAVE_SANDBOX || sandbox != 0 #endif ! || (opt_flags & OPT_MODELINE))) *p = *p | P_INSECURE; else if (new_value) *p = *p & ~P_INSECURE; *************** *** 6036,6041 **** --- 6041,6047 ---- char_u *saved_newval = NULL; #endif char_u *r = NULL; + int value_checked = FALSE; if (options[opt_idx].var == NULL) /* don't set hidden option */ return NULL; *************** *** 6063,6070 **** } #endif if ((r = did_set_string_option(opt_idx, varp, TRUE, oldval, NULL, ! opt_flags)) == NULL) ! did_set_option(opt_idx, opt_flags, TRUE); #if defined(FEAT_EVAL) /* call autocommand after handling side effects */ --- 6069,6076 ---- } #endif if ((r = did_set_string_option(opt_idx, varp, TRUE, oldval, NULL, ! opt_flags, &value_checked)) == NULL) ! did_set_option(opt_idx, opt_flags, TRUE, value_checked); #if defined(FEAT_EVAL) /* call autocommand after handling side effects */ *************** *** 6099,6110 **** */ static char_u * did_set_string_option( ! int opt_idx, /* index in options[] table */ ! char_u **varp, /* pointer to the option variable */ ! int new_value_alloced, /* new value was allocated */ ! char_u *oldval, /* previous value of the option */ ! char_u *errbuf, /* buffer for errors, or NULL */ ! int opt_flags) /* OPT_LOCAL and/or OPT_GLOBAL */ { char_u *errmsg = NULL; char_u *s, *p; --- 6105,6118 ---- */ static char_u * did_set_string_option( ! int opt_idx, // index in options[] table ! char_u **varp, // pointer to the option variable ! int new_value_alloced, // new value was allocated ! char_u *oldval, // previous value of the option ! char_u *errbuf, // buffer for errors, or NULL ! int opt_flags, // OPT_LOCAL and/or OPT_GLOBAL ! int *value_checked) // value was checked to be save, no ! // need to set P_INSECURE { char_u *errmsg = NULL; char_u *s, *p; *************** *** 6134,6143 **** errmsg = e_secure; } ! /* Check for a "normal" directory or file name in some options. Disallow a ! * path separator (slash and/or backslash), wildcards and characters that ! * are often illegal in a file name. Be more permissive if "secure" is off. ! */ else if (((options[opt_idx].flags & P_NFNAME) && vim_strpbrk(*varp, (char_u *)(secure ? "/\\*?[|;&<>\r\n" : "/\\*?[<>\r\n")) != NULL) --- 6142,6150 ---- errmsg = e_secure; } ! // Check for a "normal" directory or file name in some options. Disallow a ! // path separator (slash and/or backslash), wildcards and characters that ! // are often illegal in a file name. Be more permissive if "secure" is off. else if (((options[opt_idx].flags & P_NFNAME) && vim_strpbrk(*varp, (char_u *)(secure ? "/\\*?[|;&<>\r\n" : "/\\*?[<>\r\n")) != NULL) *************** *** 6524,6532 **** if (!valid_filetype(*varp)) errmsg = e_invarg; else ! /* load or unload key mapping tables */ errmsg = keymap_init(); if (errmsg == NULL) { if (*curbuf->b_p_keymap != NUL) --- 6531,6553 ---- if (!valid_filetype(*varp)) errmsg = e_invarg; else ! { ! int secure_save = secure; ! ! // Reset the secure flag, since the value of 'keymap' has ! // been checked to be safe. ! secure = 0; ! ! // load or unload key mapping tables errmsg = keymap_init(); + secure = secure_save; + + // Since we check the value, there is no need to set P_INSECURE, + // even when the value comes from a modeline. + *value_checked = TRUE; + } + if (errmsg == NULL) { if (*curbuf->b_p_keymap != NUL) *************** *** 7523,7529 **** --- 7544,7556 ---- if (!valid_filetype(*varp)) errmsg = e_invarg; else + { value_changed = STRCMP(oldval, *varp) != 0; + + // Since we check the value, there is no need to set P_INSECURE, + // even when the value comes from a modeline. + *value_checked = TRUE; + } } #ifdef FEAT_SYN_HL *************** *** 7532,7538 **** --- 7559,7571 ---- if (!valid_filetype(*varp)) errmsg = e_invarg; else + { value_changed = STRCMP(oldval, *varp) != 0; + + // Since we check the value, there is no need to set P_INSECURE, + // even when the value comes from a modeline. + *value_checked = TRUE; + } } #endif *************** *** 7752,7758 **** * already set to this value. */ if (!(opt_flags & OPT_MODELINE) || value_changed) { ! static int ft_recursive = 0; ++ft_recursive; did_filetype = TRUE; --- 7785,7796 ---- * already set to this value. */ if (!(opt_flags & OPT_MODELINE) || value_changed) { ! static int ft_recursive = 0; ! int secure_save = secure; ! ! // Reset the secure flag, since the value of 'filetype' has ! // been checked to be safe. ! secure = 0; ++ft_recursive; did_filetype = TRUE; *************** *** 7764,7769 **** --- 7802,7809 ---- /* Just in case the old "curbuf" is now invalid. */ if (varp != &(curbuf->b_p_ft)) varp = NULL; + + secure = secure_save; } } #ifdef FEAT_SPELL *** ../vim-8.1.0543/src/testdir/test_modeline.vim 2018-11-03 19:06:20.211795974 +0100 --- src/testdir/test_modeline.vim 2018-11-25 02:14:42.281361870 +0100 *************** *** 6,12 **** --- 6,88 ---- let modeline = &modeline set modeline call assert_fails('split Xmodeline', 'E518:') + let &modeline = modeline bwipe! call delete('Xmodeline') endfunc + + func Test_modeline_filetype() + call writefile(['vim: set ft=c :', 'nothing'], 'Xmodeline_filetype') + let modeline = &modeline + set modeline + filetype plugin on + split Xmodeline_filetype + call assert_equal("c", &filetype) + call assert_equal(1, b:did_ftplugin) + call assert_equal("ccomplete#Complete", &ofu) + + bwipe! + call delete('Xmodeline_filetype') + let &modeline = modeline + filetype plugin off + endfunc + + func Test_modeline_syntax() + call writefile(['vim: set syn=c :', 'nothing'], 'Xmodeline_syntax') + let modeline = &modeline + set modeline + syntax enable + split Xmodeline_syntax + call assert_equal("c", &syntax) + call assert_equal("c", b:current_syntax) + + bwipe! + call delete('Xmodeline_syntax') + let &modeline = modeline + syntax off + endfunc + + func Test_modeline_keymap() + call writefile(['vim: set keymap=greek :', 'nothing'], 'Xmodeline_keymap') + let modeline = &modeline + set modeline + split Xmodeline_keymap + call assert_equal("greek", &keymap) + call assert_match('greek\|grk', b:keymap_name) + + bwipe! + call delete('Xmodeline_keymap') + let &modeline = modeline + set keymap= iminsert=0 imsearch=-1 + endfunc + + func s:modeline_fails(what, text) + let fname = "Xmodeline_fails_" . a:what + call writefile(['vim: set ' . a:text . ' :', 'nothing'], fname) + let modeline = &modeline + set modeline + filetype plugin on + syntax enable + call assert_fails('split ' . fname, 'E474:') + call assert_equal("", &filetype) + call assert_equal("", &syntax) + + bwipe! + call delete(fname) + let &modeline = modeline + filetype plugin off + syntax off + endfunc + + func Test_modeline_filetype_fails() + call s:modeline_fails('filetype', 'ft=evil$CMD') + endfunc + + func Test_modeline_syntax_fails() + call s:modeline_fails('syntax', 'syn=evil$CMD') + endfunc + + func Test_modeline_keymap_fails() + call s:modeline_fails('keymap', 'keymap=evil$CMD') + endfunc *** ../vim-8.1.0543/src/version.c 2018-11-24 14:27:36.988474753 +0100 --- src/version.c 2018-11-25 02:17:53.463996557 +0100 *************** *** 794,795 **** --- 794,797 ---- { /* Add new patch number below this line */ + /**/ + 544, /**/ -- To the optimist, the glass is half full. To the pessimist, the glass is half empty. To the engineer, the glass is twice as big as it needs to be. /// Bram Moolenaar -- Bram@Moolenaar.net -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///