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

simplewallet: Add Unicode input_line [Ryo backport] #4390

Merged
merged 2 commits into from
Oct 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 23 additions & 11 deletions src/common/password.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ namespace

bool read_from_tty(epee::wipeable_string& pass, bool hide_input)
{
static constexpr const char BACKSPACE = 8;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea why someone chose to do this instead of using \b

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most likely for symmetry with the Unix routine (which used BACKSPACE = 127, ASCII DEL). I'd suggest preserving this line. Although I would have named them both DELETE, not BACKSPACE.


HANDLE h_cin = ::GetStdHandle(STD_INPUT_HANDLE);

DWORD mode_old;
Expand All @@ -67,32 +65,46 @@ namespace

bool r = true;
pass.reserve(tools::password_container::max_password_size);
std::vector<int> chlen;
chlen.reserve(tools::password_container::max_password_size);
while (pass.size() < tools::password_container::max_password_size)
{
DWORD read;
char ch;
r = (TRUE == ::ReadConsoleA(h_cin, &ch, 1, &read, NULL));
wchar_t ucs2_ch;
r = (TRUE == ::ReadConsoleW(h_cin, &ucs2_ch, 1, &read, NULL));
r &= (1 == read);

if (!r)
{
break;
}
else if (ch == '\n' || ch == '\r')
else if (ucs2_ch == L'\n' || ucs2_ch == L'\r')
{
std::cout << std::endl;
break;
}
else if (ch == BACKSPACE)
else if (ucs2_ch == L'\b')
{
if (!pass.empty())
{
pass.pop_back();
int len = chlen.back();
chlen.pop_back();
while(len-- > 0)
pass.pop_back();
}
continue;
}
else
{
pass.push_back(ch);
}

char utf8_ch[8] = {0};
int len;
if((len = WideCharToMultiByte(CP_UTF8, 0, &ucs2_ch, 1, utf8_ch, sizeof(utf8_ch), NULL, NULL)) <= 0)
break;

if(pass.size() + len >= tools::password_container::max_password_size)
break;

chlen.push_back(len);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible for the UTF8 char to push pass.size() over the max_password_size. Should check the length and break out of the loop here, before appending the new character.

I think it's bad practice to just silently return with whatever was read so far, but the existing code already does this. And I suppose the likelihood of anyone entering such a long password is low to begin with, so maybe not a serious problem.

pass += utf8_ch;
}

::SetConsoleMode(h_cin, mode_old);
Expand Down
28 changes: 28 additions & 0 deletions src/common/util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -939,4 +939,32 @@ std::string get_nix_version_display_string()
}
return newval;
}

#ifdef _WIN32
std::string input_line_win()
{
HANDLE hConIn = CreateFileW(L"CONIN$", GENERIC_READ | GENERIC_WRITE, FILE_SHARE_READ | FILE_SHARE_WRITE, nullptr, OPEN_EXISTING, 0, nullptr);
DWORD oldMode;

FlushConsoleInputBuffer(hConIn);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't we just use STD_INPUT_HANDLE here?

The Unix version will work with input redirection. I don't see why we should break that for Windows.

GetConsoleMode(hConIn, &oldMode);
SetConsoleMode(hConIn, ENABLE_LINE_INPUT | ENABLE_ECHO_INPUT | ENABLE_PROCESSED_INPUT);

wchar_t buffer[1024];
DWORD read;

ReadConsoleW(hConIn, buffer, sizeof(buffer)/sizeof(wchar_t)-1, &read, nullptr);
buffer[read] = 0;

SetConsoleMode(hConIn, oldMode);
CloseHandle(hConIn);

int size_needed = WideCharToMultiByte(CP_UTF8, 0, buffer, -1, NULL, 0, NULL, NULL);
std::string buf(size_needed, '\0');
WideCharToMultiByte(CP_UTF8, 0, buffer, -1, &buf[0], size_needed, NULL, NULL);
buf.pop_back(); //size_needed includes null that we needed to have space for
return buf;
}
#endif

}
3 changes: 3 additions & 0 deletions src/common/util.h
Original file line number Diff line number Diff line change
Expand Up @@ -235,4 +235,7 @@ namespace tools
boost::optional<std::pair<uint32_t, uint32_t>> parse_subaddress_lookahead(const std::string& str);

std::string glob_to_regex(const std::string &val);
#ifdef _WIN32
std::string input_line_win();
#endif
}
50 changes: 3 additions & 47 deletions src/simplewallet/simplewallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,47 +138,6 @@ namespace

const command_line::arg_descriptor< std::vector<std::string> > arg_command = {"command", ""};

#ifdef WIN32
// Translate from CP850 to UTF-8;
// std::getline for a Windows console returns a string in CP437 or CP850; as simplewallet,
// like all of Monero, is assumed to work internally with UTF-8 throughout, even on Windows
// (although only implemented partially), a translation to UTF-8 is needed for input.
//
// Note that if a program is started inside the MSYS2 shell somebody already translates
// console input to UTF-8, but it's not clear how one could detect that in order to avoid
// double-translation; this code here thus breaks UTF-8 input within a MSYS2 shell,
// unfortunately.
//
// Note also that input for passwords is NOT translated, to remain compatible with any
// passwords containing special characters that predate this switch to UTF-8 support.
template<typename T>
static T cp850_to_utf8(const T &cp850_str)
{
boost::locale::generator gen;
gen.locale_cache_enabled(true);
std::locale loc = gen("en_US.CP850");
const boost::locale::conv::method_type how = boost::locale::conv::default_method;
T result;
const char *begin = cp850_str.data();
const char *end = begin + cp850_str.size();
result.reserve(end-begin);
typedef std::back_insert_iterator<T> inserter_type;
inserter_type inserter(result);
boost::locale::utf::code_point c;
while(begin!=end) {
c=boost::locale::utf::utf_traits<char>::template decode<char const *>(begin,end);
if(c==boost::locale::utf::illegal || c==boost::locale::utf::incomplete) {
if(how==boost::locale::conv::stop)
throw boost::locale::conv::conversion_error();
}
else {
boost::locale::utf::utf_traits<char>::template encode<inserter_type>(c,inserter);
}
}
return result;
}
#endif

std::string input_line(const std::string& prompt)
{
#ifdef HAVE_READLINE
Expand All @@ -187,9 +146,10 @@ namespace
std::cout << prompt;

std::string buf;
#ifdef _WIN32
buf = tools::input_line_win();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should have emulated std::getline() semantics instead, e.g.
tools::getline(buf);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This paradigm is actually faster in C++11 onwards (copy elision).

#else
std::getline(std::cin, buf);
#ifdef WIN32
buf = cp850_to_utf8(buf);
#endif

return epee::string_tools::trim(buf);
Expand All @@ -209,10 +169,6 @@ namespace

epee::wipeable_string buf = pwd_container->password();

#ifdef WIN32
buf = cp850_to_utf8(buf);
#endif

buf.trim();
return buf;
}
Expand Down