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

read_password fails on Windows when stdin is piped #50

Closed
Heliozoa opened this issue Jun 16, 2020 · 2 comments · Fixed by #51
Closed

read_password fails on Windows when stdin is piped #50

Heliozoa opened this issue Jun 16, 2020 · 2 comments · Fixed by #51

Comments

@Heliozoa
Copy link
Contributor

Hi!
I ran into the following issue on Windows 10. Could be related to #49 but I'm not sure. With the main.rs below

fn main() {
    rpassword::read_password().unwrap();
}

running echo "asd" | cargo run results in thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 6, kind: Other, message: "The handle is invalid." }'.

The cause is the GetConsoleMode call at

rpassword/src/lib.rs

Lines 184 to 188 in 096c352

// Get the old mode so we can reset back to it when we are done
let mut mode = 0;
if unsafe { GetConsoleMode(handle, &mut mode as LPDWORD) } == 0 {
return Err(::std::io::Error::last_os_error());
}
which fails because the handle has the wrong type when stdin is piped (I found this question on the topic https://comp.os.ms-windows.programmer.win32.narkive.com/xiPamaWm/getconsolemode-fails-when-input-piped). Subsequent Get/SetConsoleMode calls would also fail.

A simple solution could be to wrap the console mode calls in something like

if unsafe { winapi::um::fileapi::GetFileType(handle) } != winapi::um::winbase::FILE_TYPE_PIPE {
    // ...
}

but I'm not sure if there are any other implications to doing so. If this sounds acceptable, I can make a PR for it. Alternatively it could check that the handle is of type FILE_TYPE_CHAR (related SO answer https://stackoverflow.com/a/3453272) but I felt like making a special case for pipe would be less likely to cause any other issues.

For additional context, I originally bumped into this issue when trying to test a CLI app that asks for a password using rpassword, which tried to run the CLI with Command::new(cli).stdin(Stdio::piped()).spawn() and write into the stdin handle. Also, I don't really know anything about the Windows API so it's possible I've misunderstood or overlooked something. Thanks for your work on rpassword!

@conradkleinespel
Copy link
Owner

Hey @Heliozoa ! Thanks for reporting this and proposing a PR ! 🙏 👍

@equalsraf Since you wrote the GetConsoleMode part, would you by any chance be able to review @Heliozoa 's proposal ? Ideally, you'd both agree on a fix, make a PR and I'd then merge.

@equalsraf
Copy link
Contributor

Sure.

I agree with his proposal. Checking the handle type for FILE_TYPE_PIPE has the least potential for disruption even if later we change our minds on this. Any other option requires some research on how FILE_TYPE_CHAR or other types behave. But since this function targets stdin specifically i dont think that is required.

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

Successfully merging a pull request may close this issue.

3 participants