IntenseFinance/neovim
3
0
Fork 0
mirror of https://github.com/neovim/neovim.git synced 2025-02-25 18:55:25 -06:00
Code Issues Packages Projects Releases Wiki Activity

vim-patch:8.0.0685: when conversion fails written file may be truncated

Browse Source 
Problem:    When making backups is disabled and conversion with iconv fails
            the written file is truncated. (Luo Chen)
Solution:   First try converting the file and write the file only when it did
            not fail. (partly by Christian Brabandt)
e6bf655bc4
This commit is contained in:
ckelsel 2018-09-07 20:38:25 +08:00 committed by James McCoy
parent 384770556b
commit aeda13cfdf
No known key found for this signature in database
GPG Key ID: DFE691AE331BA3DB
2 changed files with 331 additions and 276 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

275
src/nvim/fileio.c
View File

@ -2267,15 +2267,16 @@ buf_write (
char_u smallbuf[SMBUFSIZE]; char_u smallbuf[SMBUFSIZE];
char_u *backup_ext; char_u *backup_ext;
int bufsize; int bufsize;
long perm; /* file permissions */ long perm; // file permissions
int retval = OK; int retval = OK;
int newfile = FALSE; /* TRUE if file doesn't exist yet */ int newfile = false; // TRUE if file doesn't exist yet
int msg_save = msg_scroll; int msg_save = msg_scroll;
int overwriting; /* TRUE if writing over original */ int overwriting; // TRUE if writing over original
int no_eol = FALSE; /* no end-of-line written */ int no_eol = false; // no end-of-line written
int device = FALSE; /* writing to a device */ int device = false; // writing to a device
int prev_got_int = got_int; int prev_got_int = got_int;
bool file_readonly = false; /* overwritten file is read-only */ int checking_conversion;
bool file_readonly = false; // overwritten file is read-only
static char *err_readonly = static char *err_readonly =
"is read-only (cannot override: \"W\" in 'cpoptions')"; "is read-only (cannot override: \"W\" in 'cpoptions')";
#if defined(UNIX) #if defined(UNIX)
@ -3156,27 +3157,40 @@ nobackup:
notconverted = TRUE; notconverted = TRUE;
} }
/* // If conversion is taking place, we may first pretend to write and check
* Open the file "wfname" for writing. // for conversion errors. Then loop again to write for real.
* We may try to open the file twice: If we can't write to the // When not doing conversion this writes for real right away.
* file and forceit is TRUE we delete the existing file and try to create for (checking_conversion = true; ; checking_conversion = false) {
* a new one. If this still fails we may have lost the original file! // There is no need to check conversion when:
* (this may happen when the user reached his quotum for number of files). // - there is no conversion
* Appending will fail if the file does not exist and forceit is FALSE. // - we make a backup file, that can be restored in case of conversion
*/ // failure.
while ((fd = os_open((char *)wfname, O_WRONLY | (append if (!converted || dobackup) {
? (forceit ? ( checking_conversion = false;
O_APPEND | }
O_CREAT) :
O_APPEND) if (checking_conversion) {
: (O_CREAT | // Make sure we don't write anything.
O_TRUNC)) fd = -1;
write_info.bw_fd = fd;
} else {
// Open the file "wfname" for writing.
// We may try to open the file twice: If we can't write to the file
// and forceit is TRUE we delete the existing file and try to
// create a new one. If this still fails we may have lost the
// original file! (this may happen when the user reached his
// quotum for number of files).
// Appending will fail if the file does not exist and forceit is
// FALSE.
while ((fd = os_open((char *)wfname,
O_WRONLY |
(append ?
(forceit ? (O_APPEND | O_CREAT) : O_APPEND)
: (O_CREAT | O_TRUNC))
, perm < 0 ? 0666 : (perm & 0777))) < 0) { , perm < 0 ? 0666 : (perm & 0777))) < 0) {
/* // A forced write will try to create a new file if the old one
* A forced write will try to create a new file if the old one is // is still readonly. This may also happen when the directory
* still readonly. This may also happen when the directory is // is read-only. In that case the mch_remove() will fail.
* read-only. In that case the os_remove() will fail.
*/
if (errmsg == NULL) { if (errmsg == NULL) {
#ifdef UNIX #ifdef UNIX
FileInfo file_info; FileInfo file_info;
@ -3192,18 +3206,20 @@ nobackup:
if (forceit && vim_strchr(p_cpo, CPO_FWRITE) == NULL if (forceit && vim_strchr(p_cpo, CPO_FWRITE) == NULL
&& perm >= 0) { && perm >= 0) {
#ifdef UNIX #ifdef UNIX
/* we write to the file, thus it should be marked // we write to the file, thus it should be marked
writable after all */ // writable after all
if (!(perm & 0200)) if (!(perm & 0200)) {
made_writable = TRUE; made_writable = true;
}
perm |= 0200; perm |= 0200;
if (file_info_old.stat.st_uid != getuid() if (file_info_old.stat.st_uid != getuid()
|| file_info_old.stat.st_gid != getgid()) { || file_info_old.stat.st_gid != getgid()) {
perm &= 0777; perm &= 0777;
} }
#endif #endif
if (!append) /* don't remove when appending */ if (!append) { // don't remove when appending
os_remove((char *)wfname); os_remove((char *)wfname);
}
continue; continue;
} }
#ifdef UNIX #ifdef UNIX
@ -3213,19 +3229,15 @@ nobackup:
restore_backup: restore_backup:
{ {
/* // If we failed to open the file, we don't need a backup. Throw it
* If we failed to open the file, we don't need a backup. Throw it // away. If we moved or removed the original file try to put the
* away. If we moved or removed the original file try to put the // backup in its place.
* backup in its place.
*/
if (backup != NULL && wfname == fname) { if (backup != NULL && wfname == fname) {
if (backup_copy) { if (backup_copy) {
/* // There is a small chance that we removed the original,
* There is a small chance that we removed the original, // try to move the copy in its place.
* try to move the copy in its place. // This may not work if the vim_rename() fails.
* This may not work if the vim_rename() fails. // In that case we leave the copy around.
* In that case we leave the copy around.
*/
// If file does not exist, put the copy in its place // If file does not exist, put the copy in its place
if (!os_path_exists(fname)) { if (!os_path_exists(fname)) {
vim_rename(backup, fname); vim_rename(backup, fname);
@ -3235,7 +3247,7 @@ restore_backup:
os_remove((char *)backup); os_remove((char *)backup);
} }
} else { } else {
/* try to put the original file back */ // try to put the original file back
vim_rename(backup, fname); vim_rename(backup, fname);
} }
} }
@ -3246,45 +3258,47 @@ restore_backup:
} }
} }
if (wfname != fname) if (wfname != fname) {
xfree(wfname); xfree(wfname);
}
goto fail; goto fail;
} }
write_info.bw_fd = fd;
}
SET_ERRMSG(NULL); SET_ERRMSG(NULL);
write_info.bw_fd = fd;
write_info.bw_buf = buffer; write_info.bw_buf = buffer;
nchars = 0; nchars = 0;
/* use "++bin", "++nobin" or 'binary' */ // use "++bin", "++nobin" or 'binary'
if (eap != NULL && eap->force_bin != 0) if (eap != NULL && eap->force_bin != 0) {
write_bin = (eap->force_bin == FORCE_BIN); write_bin = (eap->force_bin == FORCE_BIN);
else } else {
write_bin = buf->b_p_bin; write_bin = buf->b_p_bin;
}
/* // Skip the BOM when appending and the file already existed, the BOM
* Skip the BOM when appending and the file already existed, the BOM // only makes sense at the start of the file.
* only makes sense at the start of the file.
*/
if (buf->b_p_bomb && !write_bin && (!append || perm < 0)) { if (buf->b_p_bomb && !write_bin && (!append || perm < 0)) {
write_info.bw_len = make_bom(buffer, fenc); write_info.bw_len = make_bom(buffer, fenc);
if (write_info.bw_len > 0) { if (write_info.bw_len > 0) {
/* don't convert */ // don't convert
write_info.bw_flags = FIO_NOCONVERT | wb_flags; write_info.bw_flags = FIO_NOCONVERT | wb_flags;
if (buf_write_bytes(&write_info) == FAIL) if (buf_write_bytes(&write_info) == FAIL) {
end = 0; end = 0;
else } else {
nchars += write_info.bw_len; nchars += write_info.bw_len;
} }
} }
}
write_info.bw_start_lnum = start; write_info.bw_start_lnum = start;
write_undo_file = (buf->b_p_udf && overwriting && !append write_undo_file = (buf->b_p_udf && overwriting && !append
&& !filtering && reset_changed); && !filtering && reset_changed && !checking_conversion);
if (write_undo_file) if (write_undo_file) {
/* Prepare for computing the hash value of the text. */ // Prepare for computing the hash value of the text.
sha256_start(&sha_ctx); sha256_start(&sha_ctx);
}
write_info.bw_len = bufsize; write_info.bw_len = bufsize;
#ifdef HAS_BW_FLAGS #ifdef HAS_BW_FLAGS
@ -3293,26 +3307,27 @@ restore_backup:
fileformat = get_fileformat_force(buf, eap); fileformat = get_fileformat_force(buf, eap);
s = buffer; s = buffer;
len = 0; len = 0;
for (lnum = start; lnum <= end; ++lnum) { for (lnum = start; lnum <= end; lnum++) {
/* // The next while loop is done once for each character written.
* The next while loop is done once for each character written. // Keep it fast!
* Keep it fast! ptr = ml_get_buf(buf, lnum, false) - 1;
*/ if (write_undo_file) {
ptr = ml_get_buf(buf, lnum, FALSE) - 1;
if (write_undo_file)
sha256_update(&sha_ctx, ptr + 1, (uint32_t)(STRLEN(ptr + 1) + 1)); sha256_update(&sha_ctx, ptr + 1, (uint32_t)(STRLEN(ptr + 1) + 1));
}
while ((c = *++ptr) != NUL) { while ((c = *++ptr) != NUL) {
if (c == NL) if (c == NL) {
*s = NUL; /* replace newlines with NULs */ *s = NUL; // replace newlines with NULs
else if (c == CAR && fileformat == EOL_MAC) } else if (c == CAR && fileformat == EOL_MAC) {
*s = NL; /* Mac: replace CRs with NLs */ *s = NL; // Mac: replace CRs with NLs
else } else {
*s = c; *s = c;
++s; }
if (++len != bufsize) s++;
if (++len != bufsize) {
continue; continue;
}
if (buf_write_bytes(&write_info) == FAIL) { if (buf_write_bytes(&write_info) == FAIL) {
end = 0; /* write error: break loop */ end = 0; // write error: break loop
break; break;
} }
nchars += bufsize; nchars += bufsize;
@ -3320,24 +3335,24 @@ restore_backup:
len = 0; len = 0;
write_info.bw_start_lnum = lnum; write_info.bw_start_lnum = lnum;
} }
/* write failed or last line has no EOL: stop here */ // write failed or last line has no EOL: stop here
if (end == 0 if (end == 0
|| (lnum == end || (lnum == end
&& (write_bin || !buf->b_p_fixeol) && (write_bin || !buf->b_p_fixeol)
&& (lnum == buf->b_no_eol_lnum && (lnum == buf->b_no_eol_lnum
|| (lnum == buf->b_ml.ml_line_count && !buf->b_p_eol)))) { || (lnum == buf->b_ml.ml_line_count && !buf->b_p_eol)))) {
++lnum; /* written the line, count it */ lnum++; // written the line, count it
no_eol = TRUE; no_eol = true;
break; break;
} }
if (fileformat == EOL_UNIX) if (fileformat == EOL_UNIX) {
*s++ = NL; *s++ = NL;
else { } else {
*s++ = CAR; /* EOL_MAC or EOL_DOS: write CR */ *s++ = CAR; // EOL_MAC or EOL_DOS: write CR
if (fileformat == EOL_DOS) { /* write CR-NL */ if (fileformat == EOL_DOS) { // write CR-NL
if (++len == bufsize) { if (++len == bufsize) {
if (buf_write_bytes(&write_info) == FAIL) { if (buf_write_bytes(&write_info) == FAIL) {
end = 0; /* write error: break loop */ end = 0; // write error: break loop
break; break;
} }
nchars += bufsize; nchars += bufsize;
@ -3365,11 +3380,24 @@ restore_backup:
} }
if (len > 0 && end > 0) { if (len > 0 && end > 0) {
write_info.bw_len = len; write_info.bw_len = len;
if (buf_write_bytes(&write_info) == FAIL) if (buf_write_bytes(&write_info) == FAIL) {
end = 0; /* write error */ end = 0; // write error
}
nchars += len; nchars += len;
} }
// Stop when writing done or an error was encountered.
if (!checking_conversion || end == 0) {
break;
}
// If no error happened until now, writing should be ok, so loop to
// really write the buffer.
}
// If we started writing, finish writing. Also when an error was
// encountered.
if (!checking_conversion) {
// On many journalling file systems there is a bug that causes both the // On many journalling file systems there is a bug that causes both the
// original and the backup file to be lost when halting the system right // original and the backup file to be lost when halting the system right
// after writing the file. That's because only the meta-data is // after writing the file. That's because only the meta-data is
@ -3385,17 +3413,18 @@ restore_backup:
} }
#ifdef HAVE_SELINUX #ifdef HAVE_SELINUX
/* Probably need to set the security context. */ // Probably need to set the security context.
if (!backup_copy) if (!backup_copy) {
mch_copy_sec(backup, wfname); mch_copy_sec(backup, wfname);
}
#endif #endif
#ifdef UNIX #ifdef UNIX
/* When creating a new file, set its owner/group to that of the original // When creating a new file, set its owner/group to that of the original
* file. Get the new device and inode number. */ // file. Get the new device and inode number.
if (backup != NULL && !backup_copy) { if (backup != NULL && !backup_copy) {
/* don't change the owner when it's already OK, some systems remove // don't change the owner when it's already OK, some systems remove
* permission or ACL stuff */ // permission or ACL stuff
FileInfo file_info; FileInfo file_info;
if (!os_fileinfo((char *)wfname, &file_info) if (!os_fileinfo((char *)wfname, &file_info)
|| file_info.stat.st_uid != file_info_old.stat.st_uid || file_info.stat.st_uid != file_info_old.stat.st_uid
@ -3418,27 +3447,27 @@ restore_backup:
} }
#ifdef UNIX #ifdef UNIX
if (made_writable) if (made_writable) {
perm &= ~0200; /* reset 'w' bit for security reasons */ perm &= ~0200; // reset 'w' bit for security reasons
}
#endif #endif
if (perm >= 0) { // Set perm. of new file same as old file. if (perm >= 0) { // Set perm. of new file same as old file.
(void)os_setperm((const char *)wfname, perm); (void)os_setperm((const char *)wfname, perm);
} }
#ifdef HAVE_ACL #ifdef HAVE_ACL
/* Probably need to set the ACL before changing the user (can't set the // Probably need to set the ACL before changing the user (can't set the
* ACL on a file the user doesn't own). */ // ACL on a file the user doesn't own).
if (!backup_copy) if (!backup_copy) {
mch_set_acl(wfname, acl); mch_set_acl(wfname, acl);
}
#endif #endif
if (wfname != fname) { if (wfname != fname) {
/* // The file was written to a temp file, now it needs to be converted
* The file was written to a temp file, now it needs to be converted // with 'charconvert' to (overwrite) the output file.
* with 'charconvert' to (overwrite) the output file.
*/
if (end != 0) { if (end != 0) {
if (eval_charconvert(enc_utf8 ? "utf-8" : (char *) p_enc, (char *) fenc, if (eval_charconvert(enc_utf8 ? "utf-8" : (char *)p_enc, (char *)fenc,
(char *) wfname, (char *) fname) == FAIL) { (char *)wfname, (char *)fname) == FAIL) {
write_info.bw_conv_error = true; write_info.bw_conv_error = true;
end = 0; end = 0;
} }
@ -3446,8 +3475,10 @@ restore_backup:
os_remove((char *)wfname); os_remove((char *)wfname);
xfree(wfname); xfree(wfname);
} }
}
if (end == 0) { if (end == 0) {
// Error encountered.
if (errmsg == NULL) { if (errmsg == NULL) {
if (write_info.bw_conv_error) { if (write_info.bw_conv_error) {
if (write_info.bw_conv_error_lnum == 0) { if (write_info.bw_conv_error_lnum == 0) {
@ -3470,19 +3501,17 @@ restore_backup:
} }
} }
/* // If we have a backup file, try to put it in place of the new file,
* If we have a backup file, try to put it in place of the new file, // because the new file is probably corrupt. This avoids losing the
* because the new file is probably corrupt. This avoids losing the // original file when trying to make a backup when writing the file a
* original file when trying to make a backup when writing the file a // second time.
* second time. // When "backup_copy" is set we need to copy the backup over the new
* When "backup_copy" is set we need to copy the backup over the new // file. Otherwise rename the backup file.
* file. Otherwise rename the backup file. // If this is OK, don't give the extra warning message.
* If this is OK, don't give the extra warning message.
*/
if (backup != NULL) { if (backup != NULL) {
if (backup_copy) { if (backup_copy) {
/* This may take a while, if we were interrupted let the user // This may take a while, if we were interrupted let the user
* know we got the message. */ // know we got the message.
if (got_int) { if (got_int) {
MSG(_(e_interr)); MSG(_(e_interr));
ui_flush(); ui_flush();
@ -3491,27 +3520,31 @@ restore_backup:
if ((write_info.bw_fd = os_open((char *)fname, if ((write_info.bw_fd = os_open((char *)fname,
O_WRONLY | O_CREAT | O_TRUNC, O_WRONLY | O_CREAT | O_TRUNC,
perm & 0777)) >= 0) { perm & 0777)) >= 0) {
/* copy the file. */ // copy the file.
write_info.bw_buf = smallbuf; write_info.bw_buf = smallbuf;
#ifdef HAS_BW_FLAGS #ifdef HAS_BW_FLAGS
write_info.bw_flags = FIO_NOCONVERT; write_info.bw_flags = FIO_NOCONVERT;
#endif #endif
while ((write_info.bw_len = read_eintr(fd, smallbuf, while ((write_info.bw_len = read_eintr(fd, smallbuf,
SMBUFSIZE)) > 0) SMBUFSIZE)) > 0) {
if (buf_write_bytes(&write_info) == FAIL) if (buf_write_bytes(&write_info) == FAIL) {
break; break;
}
}
if (close(write_info.bw_fd) >= 0 if (close(write_info.bw_fd) >= 0
&& write_info.bw_len == 0) && write_info.bw_len == 0) {
end = 1; /* success */ end = 1; // success
} }
close(fd); /* ignore errors for closing read file */ }
close(fd); // ignore errors for closing read file
} }
} else { } else {
if (vim_rename(backup, fname) == 0) if (vim_rename(backup, fname) == 0) {
end = 1; end = 1;
} }
} }
}
goto fail; goto fail;
} }
@ -4099,6 +4132,10 @@ static int buf_write_bytes(struct bw_info *ip)
# endif # endif
} }
if (ip->bw_fd < 0) {
// Only checking conversion, which is OK if we get here.
return OK;
}
wlen = write_eintr(ip->bw_fd, buf, len); wlen = write_eintr(ip->bw_fd, buf, len);
return (wlen < len) ? FAIL : OK; return (wlen < len) ? FAIL : OK;
} }

18
src/nvim/testdir/test_writefile.vim
View File

@ -32,6 +32,24 @@ func Test_writefile_fails_gently()
call assert_fails('call writefile([], [])', 'E730:') call assert_fails('call writefile([], [])', 'E730:')
endfunc endfunc
func Test_writefile_fails_conversion()
if !has('multi_byte') || !has('iconv')
return
endif
set nobackup nowritebackup
new
let contents = ["line one", "line two"]
call writefile(contents, 'Xfile')
edit Xfile
call setline(1, ["first line", "cannot convert \u010b", "third line"])
call assert_fails('write ++enc=cp932')
call assert_equal(contents, readfile('Xfile'))
call delete('Xfile')
bwipe!
set backup& writebackup&
endfunc
func SetFlag(timer) func SetFlag(timer)
let g:flag = 1 let g:flag = 1
endfunc endfunc