Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sharing violation between Notepad and renamer on Windows #61

Open
assarbad opened this issue Nov 21, 2022 · 3 comments
Open

Sharing violation between Notepad and renamer on Windows #61

assarbad opened this issue Nov 21, 2022 · 3 comments

Comments

@assarbad
Copy link
Contributor

During the implementation of #59 it turned out that the use of tempfile and how Notepad attempts to (re)open the file for writing back changes causes a sharing violation. I asked for advice over in Stebalien/tempfile#200.

I'll attempt to tackle this, although I cannot assign it to myself.

@assarbad
Copy link
Contributor Author

@mtimkovich @marcusbuffett the suggestion over in Stebalien/tempfile#200 is to use a temporary directory instead and create a file within that.

I also got another idea, but I need to verify if this is viable with tempfile. A cursory look suggested it could, but still the code would deviate between Windows and non-Windows which isn't exactly ideal. Roughly the idea would be that renamer writes the file, then reopens it readonly in order to avoid the sharing violation.

Failing that the only other method I see is that on Windows we could use MoveFileEx to register the file (and even containing folder) for removal at next boot. tempfile itself uses this method in some code paths, I think (at least the latest version of it).

@assarbad
Copy link
Contributor Author

Found a method, but it's clunky.

#![allow(unused_imports)]
use tempfile::tempdir;
use std::fs::{File,OpenOptions};
use std::os::windows::fs::OpenOptionsExt;
use std::os::windows::io::{AsRawHandle, FromRawHandle, RawHandle};
use std::path::Path;
use std::io::{self, Write, BufReader, BufRead, Error};
use std::process::Command;

use windows_sys::Win32::Foundation::{HANDLE, INVALID_HANDLE_VALUE};
use windows_sys::Win32::Storage::FileSystem::{ReOpenFile, FILE_ATTRIBUTE_TEMPORARY, FILE_ATTRIBUTE_NORMAL, FILE_GENERIC_READ, FILE_GENERIC_WRITE, FILE_SHARE_READ, FILE_SHARE_WRITE, FILE_SHARE_DELETE, FILE_FLAG_DELETE_ON_CLOSE};

pub fn create(path: &Path, open_options: &mut OpenOptions) -> io::Result<File> {
    open_options
        .create_new(true)
        .read(true)
        .write(true)
        .share_mode(FILE_SHARE_READ | FILE_SHARE_WRITE)
        .custom_flags(FILE_ATTRIBUTE_TEMPORARY)
        .open(path)
}

pub fn reopen(file: &File, _path: &Path) -> io::Result<File> {
    let handle = file.as_raw_handle();
    unsafe {
        let handle = ReOpenFile(
            handle as HANDLE,
            FILE_GENERIC_READ,
            FILE_SHARE_READ | FILE_SHARE_WRITE,
            0,
        );
        if handle == INVALID_HANDLE_VALUE {
            Err(io::Error::last_os_error())
        } else {
            Ok(FromRawHandle::from_raw_handle(handle as RawHandle))
        }
    }
}

fn open_editor(tmpfile: &Path) -> io::Result<bool>
{
    let child = Command::new("C:\\Windows\\notepad.exe")
        .arg(tmpfile)
        .spawn()?;
    let output = child.wait_with_output()?;
    if output.status.success() {
        return Ok(true);
    }
    Ok(false)
}

fn main() {
    let dirname = tempdir().unwrap();
    let dirname = dirname.path();
    let filepath = dirname.join("tempfile.txt");
    let filepath = filepath.as_path();
    eprintln!("Temporary file path: {}", filepath.display());
    let tmpfile = create(filepath, &mut OpenOptions::new());
    let mut tmpfile = tmpfile.unwrap();
    writeln!(tmpfile, "Foo bar baz").unwrap();
    eprintln!("Opened temporary file");
    let readonly_tmpfile = reopen(&tmpfile, filepath);
    let readonly_tmpfile = readonly_tmpfile.unwrap();
    eprintln!("Opened temporary file again (readonly, sharing mode: read)");
    eprintln!("Dropping tmpfile");
    drop(tmpfile);
    open_editor(filepath).unwrap();
    eprintln!("Dropping readonly_tmpfile");
    drop(readonly_tmpfile);
}

Essentially it would require to specify the sharing mode in tempfile::Builder::new() somehow, and it doesn't seem to offer that. If the sharing mode would be just FILE_SHARE_READ | FILE_SHARE_WRITE it would work out. In that case the above provides a rough outline of what is viable:

  1. create tmpfile with FILE_SHARE_READ | FILE_SHARE_WRITE and open read/write (currently this is the blocker)
  2. retrieve tmpfile.path() into some variable
  3. call ReOpenFile() on the raw handle retrieved from tmpfile.as_file() and open for read-only with the sharing mode as above
  4. drop(tmpfile)
  5. Open Notepad on the path retrieved in 2.)

@assarbad
Copy link
Contributor Author

@marcusbuffett @mtimkovich Any comments? It appears that the suggested solution of using TempDir is the least clunky by far. Security isn't as much a concern for the use case, and the created temporary directory has a pseudo-random name either way.

So the idea would be:

  1. use TempDir instead of the current solution
  2. create the file with the desired sharing mode etc (including making it temporary and thus self-deleting), writing the strings to it
  3. ... on Windows (only?) reopen the file readonly, but setting the same sharing mode as before
  4. execute the editor
  5. read file and close

Comments? Better ideas? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant