From ac7025cbe1bc46d25dd978138c6b397d77853232 Mon Sep 17 00:00:00 2001
From: Daniel Mueller <deso@posteo.net>
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<String> {
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<io::Error> for Error {
}
}
+impl From<str::Utf8Error> for Error {
+ fn from(e: str::Utf8Error) -> Error {
+ Error::Utf8Error(e)
+ }
+}
+
impl From<string::FromUtf8Error> 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<u8>) -> Result<Vec<u8>, Error> {
- let string = String::from_utf8(response)?;
+fn parse_pinentry_pin<R>(response: R) -> Result<String, Error>
+where
+ R: AsRef<str>,
+{
+ 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<u8>) -> Result<Vec<u8>, 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<u8>) -> Result<Vec<u8>, 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<Vec<u8>, Error> {
+) -> Result<String, Error> {
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<u8>) -> Result<(), Error> {
- let string = String::from_utf8(response)?;
- let lines: Vec<&str> = string.lines().collect();
+fn parse_pinentry_response<R>(response: R) -> Result<(), Error>
+where
+ R: AsRef<str>,
+{
+ let string = response.as_ref();
+ let lines = string.lines().collect::<Vec<_>>();
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