patch 9.0.1583: get E304 when using 'cryptmethod' "xchacha20v2"

Problem:    Get E304 when using 'cryptmethod' "xchacha20v2". (Steve Mynott)
Solution:   Add 4th crypt method to block zero ID check.  Avoid syncing a swap
            file before reading the file. (closes #12433)

src/buffer.c

@@ -218,6 +218,10 @@ open_buffer(
 	return FAIL;
     }
 
+    // Do not sync this buffer yet, may first want to read the file.
+    if (curbuf->b_ml.ml_mfp != NULL)
+	curbuf->b_ml.ml_mfp->mf_dirty = MF_DIRTY_YES_NOSYNC;
+
     // The autocommands in readfile() may change the buffer, but only AFTER
     // reading the file.
     set_bufref(&old_curbuf, curbuf);
@@ -298,6 +302,11 @@ open_buffer(
 	    retval = read_buffer(TRUE, eap, flags);
     }
 
+    // Can now sync this buffer in ml_sync_all().
+    if (curbuf->b_ml.ml_mfp != NULL
+	    && curbuf->b_ml.ml_mfp->mf_dirty == MF_DIRTY_YES_NOSYNC)
+	curbuf->b_ml.ml_mfp->mf_dirty = MF_DIRTY_YES;
+
     // if first time loading this buffer, init b_chartab[]
     if (curbuf->b_flags & BF_NEVERLOADED)
     {

src/crypt.c

@@ -525,7 +525,8 @@ crypt_create_from_header(
     if (arg.cat_seed_len > 0)
 	arg.cat_seed = header + CRYPT_MAGIC_LEN + arg.cat_salt_len;
     if (arg.cat_add_len > 0)
-	arg.cat_add = header + CRYPT_MAGIC_LEN + arg.cat_salt_len + arg.cat_seed_len;
+	arg.cat_add = header + CRYPT_MAGIC_LEN
+					 + arg.cat_salt_len + arg.cat_seed_len;
 
     return crypt_create(method_nr, key, &arg);
 }
@@ -603,7 +604,8 @@ crypt_create_for_writing(
 	if (arg.cat_seed_len > 0)
 	    arg.cat_seed = *header + CRYPT_MAGIC_LEN + arg.cat_salt_len;
 	if (arg.cat_add_len > 0)
-	    arg.cat_add = *header + CRYPT_MAGIC_LEN + arg.cat_salt_len + arg.cat_seed_len;
+	    arg.cat_add = *header + CRYPT_MAGIC_LEN
+					 + arg.cat_salt_len + arg.cat_seed_len;
 
 	// TODO: Should this be crypt method specific? (Probably not worth
 	// it).  sha2_seed is pretty bad for large amounts of entropy, so make
@@ -795,10 +797,14 @@ crypt_check_method(int method)
     }
 }
 
-#ifdef FEAT_SODIUM
+/*
-    static void
+ * If the crypt method for "curbuf" does not support encrypting the swap file
+ * then disable the swap file.
+ */
+    void
 crypt_check_swapfile_curbuf(void)
 {
+#ifdef FEAT_SODIUM
     int method = crypt_get_method_nr(curbuf);
     if (crypt_method_is_sodium(method))
     {
@@ -809,8 +815,8 @@ crypt_check_swapfile_curbuf(void)
 	msg_scroll = TRUE;
 	msg(_("Note: Encryption of swapfile not supported, disabling swap file"));
     }
-}
 #endif
+}
 
     void
 crypt_check_current_method(void)
@@ -863,9 +869,7 @@ crypt_get_key(
 		set_option_value_give_err((char_u *)"key", 0L, p1, OPT_LOCAL);
 		crypt_free_key(p1);
 		p1 = curbuf->b_p_key;
-#ifdef FEAT_SODIUM
 		crypt_check_swapfile_curbuf();
-#endif
 	    }
 	    break;
 	}
@@ -959,7 +963,8 @@ crypt_sodium_init_(
 	    sodium_free(sd_state);
 	    return FAIL;
 	}
-	if (state->method_nr == CRYPT_M_SOD2)
+	// "cat_add" should not be NULL, check anyway for safety
+	if (state->method_nr == CRYPT_M_SOD2 && arg->cat_add != NULL)
 	{
 	    memcpy(arg->cat_add, &opslimit, sizeof(opslimit));
 	    arg->cat_add += sizeof(opslimit);

src/fileio.c

@@ -125,6 +125,7 @@ readfile(
     exarg_T	*eap,			// can be NULL!
     int		flags)
 {
+    int		retval = FAIL;	// jump to "theend" instead of returning
     int		fd = 0;
     int		newfile = (flags & READ_NEW);
     int		check_readonly;
@@ -239,7 +240,7 @@ readfile(
 	    && !(flags & READ_DUMMY))
     {
 	if (set_rw_fname(fname, sfname) == FAIL)
-	    return FAIL;
+	    goto theend;
     }
 
     // Remember the initial values of curbuf, curbuf->b_ffname and
@@ -289,35 +290,41 @@ readfile(
 	    if (apply_autocmds_exarg(EVENT_BUFREADCMD, NULL, sfname,
 							  FALSE, curbuf, eap))
 	    {
-		int status = OK;
+		retval = OK;
 #ifdef FEAT_EVAL
 		if (aborting())
-		    status = FAIL;
+		    retval = FAIL;
 #endif
 		// The BufReadCmd code usually uses ":read" to get the text and
 		// perhaps ":file" to change the buffer name. But we should
 		// consider this to work like ":edit", thus reset the
 		// BF_NOTEDITED flag.  Then ":write" will work to overwrite the
 		// same file.
-		if (status == OK)
+		if (retval == OK)
 		    curbuf->b_flags &= ~BF_NOTEDITED;
-		return status;
+		goto theend;
 	    }
 	}
 	else if (apply_autocmds_exarg(EVENT_FILEREADCMD, sfname, sfname,
 							    FALSE, NULL, eap))
+	{
 #ifdef FEAT_EVAL
-	    return aborting() ? FAIL : OK;
+	    retval = aborting() ? FAIL : OK;
 #else
-	    return OK;
+	    retval = OK;
 #endif
+	    goto theend;
+	}
 
 	curbuf->b_op_start = orig_start;
 
 	if (flags & READ_NOFILE)
+	{
 	    // Return NOTDONE instead of FAIL so that BufEnter can be triggered
 	    // and other operations don't fail.
-	    return NOTDONE;
+	    retval = NOTDONE;
+	    goto theend;
+	}
     }
 
     if ((shortmess(SHM_OVER) || curbuf->b_help) && p_verbose == 0)
@@ -335,7 +342,7 @@ readfile(
 	    filemess(curbuf, fname, (char_u *)_("Illegal file name"), 0);
 	    msg_end();
 	    msg_scroll = msg_save;
-	    return FAIL;
+	    goto theend;
 	}
 
 	// If the name ends in a path separator, we can't open it.  Check here,
@@ -346,7 +353,8 @@ readfile(
 	    filemess(curbuf, fname, (char_u *)_(msg_is_a_directory), 0);
 	    msg_end();
 	    msg_scroll = msg_save;
-	    return NOTDONE;
+	    retval = NOTDONE;
+	    goto theend;
 	}
     }
 
@@ -367,8 +375,6 @@ readfile(
 # endif
 						)
 	{
-	    int retval = FAIL;
-
 	    if (S_ISDIR(perm))
 	    {
 		filemess(curbuf, fname, (char_u *)_(msg_is_a_directory), 0);
@@ -378,7 +384,7 @@ readfile(
 		filemess(curbuf, fname, (char_u *)_("is not a file"), 0);
 	    msg_end();
 	    msg_scroll = msg_save;
-	    return retval;
+	    goto theend;
 	}
 #endif
 #if defined(MSWIN)
@@ -391,7 +397,7 @@ readfile(
 	    filemess(curbuf, fname, (char_u *)_("is a device (disabled with 'opendevice' option)"), 0);
 	    msg_end();
 	    msg_scroll = msg_save;
-	    return FAIL;
+	    goto theend;
 	}
 #endif
     }
@@ -534,7 +540,7 @@ readfile(
 					 && (old_b_fname != curbuf->b_fname)))
 			{
 			    emsg(_(e_autocommands_changed_buffer_or_buffer_name));
-			    return FAIL;
+			    goto theend;
 			}
 		    }
 		    if (dir_of_file_exists(fname))
@@ -557,10 +563,10 @@ readfile(
 		    save_file_ff(curbuf);
 
 #if defined(FEAT_EVAL)
-		    if (aborting())   // autocmds may abort script processing
+		    if (!aborting())   // autocmds may abort script processing
-			return FAIL;
 #endif
-		    return OK;	    // a new file is not an error
+			retval = OK;	    // a new file is not an error
+		    goto theend;
 		}
 		else
 		{
@@ -576,7 +582,7 @@ readfile(
 		}
 	    }
 
-	return FAIL;
+	goto theend;
     }
 
     /*
@@ -614,7 +620,7 @@ readfile(
 	    emsg(_(e_autocommands_changed_buffer_or_buffer_name));
 	    if (!read_buffer)
 		close(fd);
-	    return FAIL;
+	    goto theend;
 	}
 #ifdef UNIX
 	// Set swap file protection bits after creating it.
@@ -654,7 +660,7 @@ readfile(
     {
 	if (!read_buffer && !read_stdin)
 	    close(fd);
-	return FAIL;
+	goto theend;
     }
 
     ++no_wait_return;	    // don't wait for return yet
@@ -715,7 +721,7 @@ readfile(
 	    --no_wait_return;
 	    msg_scroll = msg_save;
 	    curbuf->b_p_ro = TRUE;	// must use "w!" now
-	    return FAIL;
+	    goto theend;
 	}
 #endif
 	/*
@@ -737,7 +743,7 @@ readfile(
 	    else
 		emsg(_(e_readpre_autocommands_must_not_change_current_buffer));
 	    curbuf->b_p_ro = TRUE;	// must use "w!" now
-	    return FAIL;
+	    goto theend;
 	}
     }
 
@@ -2461,7 +2467,8 @@ failed:
 #ifdef FEAT_VIMINFO
 	    check_marks_read();
 #endif
-	    return OK;		// an interrupt isn't really an error
+	    retval = OK;	// an interrupt isn't really an error
+	    goto theend;
 	}
 
 	if (!filtering && !(flags & READ_DUMMY))
@@ -2696,13 +2703,20 @@ failed:
 	    msg_scroll = m;
 # ifdef FEAT_EVAL
 	if (aborting())	    // autocmds may abort script processing
-	    return FAIL;
+	    goto theend;
 # endif
     }
 
-    if (recoverymode && error)
+    if (!(recoverymode && error))
-	return FAIL;
+	retval = OK;
-    return OK;
+
+theend:
+    if (curbuf->b_ml.ml_mfp != NULL
+		       && curbuf->b_ml.ml_mfp->mf_dirty == MF_DIRTY_YES_NOSYNC)
+	// OK to sync the swap file now
+	curbuf->b_ml.ml_mfp->mf_dirty = MF_DIRTY_YES;
+
+    return retval;
 }
 
 #if defined(OPEN_CHR_FILES) || defined(PROTO)
@@ -2941,7 +2955,10 @@ check_for_cryptkey(
 	if (cryptkey == NULL && !*did_ask)
 	{
 	    if (*curbuf->b_p_key)
+	    {
 		cryptkey = curbuf->b_p_key;
+		crypt_check_swapfile_curbuf();
+	    }
 	    else
 	    {
 		// When newfile is TRUE, store the typed key in the 'key'

src/memfile.c

@@ -150,7 +150,7 @@ mf_open(char_u *fname, int flags)
     mfp->mf_free_first = NULL;		// free list is empty
     mfp->mf_used_first = NULL;		// used list is empty
     mfp->mf_used_last = NULL;
-    mfp->mf_dirty = FALSE;
+    mfp->mf_dirty = MF_DIRTY_NO;
     mfp->mf_used_count = 0;
     mf_hash_init(&mfp->mf_hash);
     mf_hash_init(&mfp->mf_trans);
@@ -224,7 +224,7 @@ mf_open_file(memfile_T *mfp, char_u *fname)
     if (mfp->mf_fd < 0)
 	return FAIL;
 
-    mfp->mf_dirty = TRUE;
+    mfp->mf_dirty = MF_DIRTY_YES;
     return OK;
 }
 
@@ -386,7 +386,7 @@ mf_new(memfile_T *mfp, int negative, int page_count)
 	}
     }
     hp->bh_flags = BH_LOCKED | BH_DIRTY;	// new block is always dirty
-    mfp->mf_dirty = TRUE;
+    mfp->mf_dirty = MF_DIRTY_YES;
     hp->bh_page_count = page_count;
     mf_ins_used(mfp, hp);
     mf_ins_hash(mfp, hp);
@@ -483,7 +483,8 @@ mf_put(
     if (dirty)
     {
 	flags |= BH_DIRTY;
-	mfp->mf_dirty = TRUE;
+	if (mfp->mf_dirty != MF_DIRTY_YES_NOSYNC)
+	    mfp->mf_dirty = MF_DIRTY_YES;
     }
     hp->bh_flags = flags;
     if (infile)
@@ -528,9 +529,10 @@ mf_sync(memfile_T *mfp, int flags)
     bhdr_T	*hp;
     int		got_int_save = got_int;
 
-    if (mfp->mf_fd < 0)	    // there is no file, nothing to do
+    if (mfp->mf_fd < 0)
     {
-	mfp->mf_dirty = FALSE;
+	// there is no file, nothing to do
+	mfp->mf_dirty = MF_DIRTY_NO;
 	return FAIL;
     }
 
@@ -576,7 +578,7 @@ mf_sync(memfile_T *mfp, int flags)
      * In case of an error this flag is also set, to avoid trying all the time.
      */
     if (hp == NULL || status == FAIL)
-	mfp->mf_dirty = FALSE;
+	mfp->mf_dirty = MF_DIRTY_NO;
 
     if ((flags & MFS_FLUSH) && *p_sws != NUL)
     {
@@ -675,7 +677,7 @@ mf_set_dirty(memfile_T *mfp)
     for (hp = mfp->mf_used_last; hp != NULL; hp = hp->bh_prev)
 	if (hp->bh_bnum > 0)
 	    hp->bh_flags |= BH_DIRTY;
-    mfp->mf_dirty = TRUE;
+    mfp->mf_dirty = MF_DIRTY_YES;
 }
 
 /*

src/memline.c

@@ -64,7 +64,7 @@ typedef struct pointer_entry	PTR_EN;	    // block/line-count pair
 #define BLOCK0_ID1_C0  'c'		    // block 0 id 1 'cm' 0
 #define BLOCK0_ID1_C1  'C'		    // block 0 id 1 'cm' 1
 #define BLOCK0_ID1_C2  'd'		    // block 0 id 1 'cm' 2
-// BLOCK0_ID1_C3 and BLOCK0_ID1_C4 are for libsodium enctyption.  However, for
+// BLOCK0_ID1_C3 and BLOCK0_ID1_C4 are for libsodium encryption.  However, for
 // these the swapfile is disabled, thus they will not be used.  Added for
 // consistency anyway.
 #define BLOCK0_ID1_C3  'S'		    // block 0 id 1 'cm' 3
@@ -807,6 +807,8 @@ ml_open_file(buf_T *buf)
 	    continue;
 	if (mf_open_file(mfp, fname) == OK)	// consumes fname!
 	{
+	    // don't sync yet in ml_sync_all()
+	    mfp->mf_dirty = MF_DIRTY_YES_NOSYNC;
 #if defined(MSWIN)
 	    /*
 	     * set full pathname for swap file now, because a ":!cd dir" may
@@ -939,7 +941,8 @@ ml_check_b0_id(ZERO_BL *b0p)
 		&& b0p->b0_id[1] != BLOCK0_ID1_C0
 		&& b0p->b0_id[1] != BLOCK0_ID1_C1
 		&& b0p->b0_id[1] != BLOCK0_ID1_C2
-		&& b0p->b0_id[1] != BLOCK0_ID1_C3)
+		&& b0p->b0_id[1] != BLOCK0_ID1_C3
+		&& b0p->b0_id[1] != BLOCK0_ID1_C4)
 	    )
 	return FAIL;
     return OK;
@@ -2513,7 +2516,7 @@ ml_sync_all(int check_file, int check_char)
 		need_check_timestamps = TRUE;	// give message later
 	    }
 	}
-	if (buf->b_ml.ml_mfp->mf_dirty)
+	if (buf->b_ml.ml_mfp->mf_dirty == MF_DIRTY_YES)
 	{
 	    (void)mf_sync(buf->b_ml.ml_mfp, (check_char ? MFS_STOP : 0)
 					| (bufIsChanged(buf) ? MFS_FLUSH : 0));

src/proto/crypt.pro

@@ -22,6 +22,7 @@ void crypt_encode_inplace(cryptstate_T *state, char_u *buf, size_t len, int last
 void crypt_decode_inplace(cryptstate_T *state, char_u *buf, size_t len, int last);
 void crypt_free_key(char_u *key);
 void crypt_check_method(int method);
+void crypt_check_swapfile_curbuf(void);
 void crypt_check_current_method(void);
 char_u *crypt_get_key(int store, int twice);
 void crypt_append_msg(buf_T *buf);

src/structs.h

@@ -691,6 +691,12 @@ typedef struct
     int		cmod_did_esilent;	// incremented when emsg_silent is
 } cmdmod_T;
 
+typedef enum {
+    MF_DIRTY_NO = 0,		// no dirty blocks
+    MF_DIRTY_YES,		// there are dirty blocks
+    MF_DIRTY_YES_NOSYNC,	// there are dirty blocks, do not sync yet
+} mfdirty_T;
+
 #define MF_SEED_LEN	8
 
 struct memfile
@@ -712,7 +718,7 @@ struct memfile
     blocknr_T	mf_neg_count;		// number of negative blocks numbers
     blocknr_T	mf_infile_count;	// number of pages in the file
     unsigned	mf_page_size;		// number of bytes in a page
-    int		mf_dirty;		// TRUE if there are dirty blocks
+    mfdirty_T	mf_dirty;
 #ifdef FEAT_CRYPT
     buf_T	*mf_buffer;		// buffer this memfile is for
     char_u	mf_seed[MF_SEED_LEN];	// seed for encryption

src/testdir/test_crypt.vim

@@ -1,5 +1,6 @@
 " Tests for encryption.
 
+source shared.vim
 source check.vim
 CheckFeature cryptv
 
@@ -88,6 +89,29 @@ func Test_crypt_sodium_v2()
   call Crypt_uncrypt('xchacha20v2')
 endfunc
 
+func Test_crypt_sodium_v2_startup()
+  CheckFeature sodium
+  CheckRunVimInTerminal
+
+  let buf = RunVimInTerminal('--cmd "set cm=xchacha20v2" -x Xfoo', #{wait_for_ruler: 0, rows: 6})
+  call g:TermWait(buf, g:RunningWithValgrind() ? 1000 : 50)
+  call term_sendkeys(buf, "foo\<CR>foo\<CR>")
+  call term_sendkeys(buf, "ifoo\<Esc>")
+  call term_sendkeys(buf, "ZZ")
+  call TermWait(buf)
+
+  " Wait for Vim to write the file and exit.  Then wipe out the terminal buffer.
+  call WaitForAssert({-> assert_equal("finished", term_getstatus(buf))})
+  exe buf .. 'bwipe!'
+  call assert_true(filereadable('Xfoo'))
+
+  let buf = RunVimInTerminal('--cmd "set ch=3 cm=xchacha20v2 key=foo" Xfoo', #{rows: 10})
+  call g:TermWait(buf, g:RunningWithValgrind() ? 1000 : 50)
+  call StopVimInTerminal(buf)
+
+  call delete('Xfoo')
+endfunc
+
 func Uncrypt_stable(method, crypted_text, key, uncrypted_text)
   split Xtest.txt
   set bin noeol key= fenc=latin1

src/version.c

@@ -695,6 +695,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    1583,
 /**/
     1582,
 /**/