From ac7025cbe1bc46d25dd978138c6b397d77853232 Mon Sep 17 00:00:00 2001 From: Daniel Mueller Date: Wed, 9 Jan 2019 16:34:53 -0800 Subject: Make pinentry::inquire_pin return String directly The inquire_pin function of the pinentry module used to return a vector of bytes, as that is what is ultimately read from the gpg-agent process. All clients of this function, however, work with a string and, hence, convert this vector into a string. As it turns out, for better or worse, the pinentry::parse_pinentry_pin function (which produces the result of inquire_pin) internally already works with a string but then converts it back. That is both not useful and a waste of resources. This change adjusts both functions of interest to simply return a String object instead, removing the need for conversions at the clients. While at it, the patch also removes the need for a bunch of unnecessary allocations caused by sub-par parameter type choice. --- nitrocli/src/commands.rs | 6 ------ nitrocli/src/error.rs | 11 ++++++++-- nitrocli/src/pinentry.rs | 52 +++++++++++++++++++++++++----------------------- 3 files changed, 36 insertions(+), 33 deletions(-) diff --git a/nitrocli/src/commands.rs b/nitrocli/src/commands.rs index 1d5c67c..e719039 100644 --- a/nitrocli/src/commands.rs +++ b/nitrocli/src/commands.rs @@ -186,10 +186,6 @@ where Ok(pin) => pin, Err(err) => return Err((data, err)), }; - let pin = match String::from_utf8(pin) { - Ok(pin) => pin, - Err(err) => return Err((data, Error::from(err))), - }; match op(data, &pin) { Ok(result) => return Ok(result), Err((new_data, err)) => match err { @@ -580,12 +576,10 @@ fn choose_pin(pin_type: pinentry::PinType) -> Result { pinentry::clear_pin(pin_type)?; let new_pin = pinentry::inquire_pin(pin_type, pinentry::Mode::Choose, None)?; pinentry::clear_pin(pin_type)?; - let new_pin = String::from_utf8(new_pin)?; check_pin(pin_type, &new_pin)?; let confirm_pin = pinentry::inquire_pin(pin_type, pinentry::Mode::Confirm, None)?; pinentry::clear_pin(pin_type)?; - let confirm_pin = String::from_utf8(confirm_pin)?; if new_pin != confirm_pin { Err(Error::Error("Entered PINs do not match".to_string())) diff --git a/nitrocli/src/error.rs b/nitrocli/src/error.rs index c2a16a2..ffcc56b 100644 --- a/nitrocli/src/error.rs +++ b/nitrocli/src/error.rs @@ -19,6 +19,7 @@ use std::fmt; use std::io; +use std::str; use std::string; #[derive(Debug)] @@ -26,7 +27,7 @@ pub enum Error { ArgparseError(i32), CommandError(nitrokey::CommandError), IoError(io::Error), - Utf8Error(string::FromUtf8Error), + Utf8Error(str::Utf8Error), Error(String), } @@ -42,9 +43,15 @@ impl From for Error { } } +impl From for Error { + fn from(e: str::Utf8Error) -> Error { + Error::Utf8Error(e) + } +} + impl From for Error { fn from(e: string::FromUtf8Error) -> Error { - Error::Utf8Error(e) + Error::Utf8Error(e.utf8_error()) } } diff --git a/nitrocli/src/pinentry.rs b/nitrocli/src/pinentry.rs index 6c36d73..8ea4298 100644 --- a/nitrocli/src/pinentry.rs +++ b/nitrocli/src/pinentry.rs @@ -1,7 +1,7 @@ // pinentry.rs // ************************************************************************* -// * Copyright (C) 2017-2018 Daniel Mueller (deso@posteo.net) * +// * Copyright (C) 2017-2019 Daniel Mueller (deso@posteo.net) * // * * // * This program is free software: you can redistribute it and/or modify * // * it under the terms of the GNU General Public License as published by * @@ -112,8 +112,11 @@ impl Mode { } } -fn parse_pinentry_pin(response: Vec) -> Result, Error> { - let string = String::from_utf8(response)?; +fn parse_pinentry_pin(response: R) -> Result +where + R: AsRef, +{ + let string = response.as_ref(); let lines: Vec<&str> = string.lines().collect(); // We expect the response to be of the form: @@ -124,7 +127,7 @@ fn parse_pinentry_pin(response: Vec) -> Result, Error> { if lines.len() == 2 && lines[1] == "OK" && lines[0].starts_with("D ") { // We got the only valid answer we accept. let (_, pass) = lines[0].split_at(2); - return Ok(pass.to_string().into_bytes()); + return Ok(pass.to_string()); } // Check if we are dealing with a special "ERR " line and report that @@ -133,7 +136,7 @@ fn parse_pinentry_pin(response: Vec) -> Result, Error> { let (_, error) = lines[0].split_at(4); return Err(Error::Error(error.to_string())); } - Err(Error::Error("Unexpected response: ".to_string() + &string)) + Err(Error::Error(format!("Unexpected response: {}", string))) } /// Inquire a PIN of the given type from the user. @@ -148,7 +151,7 @@ pub fn inquire_pin( pin_type: PinType, mode: Mode, error_msg: Option<&str>, -) -> Result, Error> { +) -> Result { let cache_id = pin_type.cache_id(); let error_msg = error_msg .map(|msg| msg.replace(" ", "+")) @@ -172,18 +175,21 @@ pub fn inquire_pin( .arg(command) .arg("/bye") .output()?; - parse_pinentry_pin(output.stdout) + parse_pinentry_pin(str::from_utf8(&output.stdout)?) } -fn parse_pinentry_response(response: Vec) -> Result<(), Error> { - let string = String::from_utf8(response)?; - let lines: Vec<&str> = string.lines().collect(); +fn parse_pinentry_response(response: R) -> Result<(), Error> +where + R: AsRef, +{ + let string = response.as_ref(); + let lines = string.lines().collect::>(); if lines.len() == 1 && lines[0] == "OK" { // We got the only valid answer we accept. return Ok(()); } - Err(Error::Error("Unexpected response: ".to_string() + &string)) + Err(Error::Error(format!("Unexpected response: {}", string))) } /// Clear the cached pin of the given type. @@ -194,7 +200,7 @@ pub fn clear_pin(pin_type: PinType) -> Result<(), Error> { .arg("/bye") .output()?; - parse_pinentry_response(output.stdout) + parse_pinentry_response(str::from_utf8(&output.stdout)?) } #[cfg(test)] @@ -203,8 +209,8 @@ mod tests { #[test] fn parse_pinentry_pin_good() { - let response = "D passphrase\nOK\n".to_string().into_bytes(); - let expected = "passphrase".to_string().into_bytes(); + let response = "D passphrase\nOK\n"; + let expected = "passphrase"; assert_eq!(parse_pinentry_pin(response).unwrap(), expected) } @@ -215,7 +221,7 @@ mod tests { let response = "ERR ".to_string() + error + "\n"; let expected = error; - let error = parse_pinentry_pin(response.to_string().into_bytes()); + let error = parse_pinentry_pin(response.to_string()); if let Error::Error(ref e) = error.err().unwrap() { assert_eq!(e, &expected); @@ -227,9 +233,8 @@ mod tests { #[test] fn parse_pinentry_pin_unexpected() { let response = "foobar\n"; - let expected = "Unexpected response: ".to_string() + response; - - let error = parse_pinentry_pin(response.to_string().into_bytes()); + let expected = format!("Unexpected response: {}", response); + let error = parse_pinentry_pin(response); if let Error::Error(ref e) = error.err().unwrap() { assert_eq!(e, &expected); @@ -240,22 +245,19 @@ mod tests { #[test] fn parse_pinentry_response_ok() { - let response = "OK\n".to_string().into_bytes(); - assert!(parse_pinentry_response(response).is_ok()) + assert!(parse_pinentry_response("OK\n").is_ok()) } #[test] fn parse_pinentry_response_ok_no_newline() { - let response = "OK".to_string().into_bytes(); - assert!(parse_pinentry_response(response).is_ok()) + assert!(parse_pinentry_response("OK").is_ok()) } #[test] fn parse_pinentry_response_unexpected() { let response = "ERR 42"; - let expected = "Unexpected response: ".to_string() + response; - - let error = parse_pinentry_response(response.to_string().into_bytes()); + let expected = format!("Unexpected response: {}", response); + let error = parse_pinentry_response(response); if let Error::Error(ref e) = error.err().unwrap() { assert_eq!(e, &expected); -- cgit v1.2.3