Bug 43066 - get_password on Windows is kludgy
Summary: get_password on Windows is kludgy
Status: NEW
Alias: None
Product: APR
Classification: Unclassified
Component: APR (show other bugs)
Version: HEAD
Hardware: All Windows XP
: P2 normal (vote)
Target Milestone: ---
Assignee: Apache Portable Runtime bugs mailinglist
Keywords: PatchAvailable
Depends on:
Reported: 2007-08-08 17:07 UTC by Ondra Hosek
Modified: 2007-12-26 12:43 UTC (History)
0 users

Proposed solution (2.90 KB, patch)
2007-08-08 17:10 UTC, Ondra Hosek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ondra Hosek 2007-08-08 17:07:05 UTC
The Windows implementation of get_password in passwd/apr_getpass.c is a kludge.
The same problem can be solved by an API very akin to termios -- using the
GetConsoleMode and SetConsoleMode system calls. These are available since
Windows 95 (according to MSDN).
Comment 1 Ondra Hosek 2007-08-08 17:10:41 UTC
Created attachment 20617 [details]
Proposed solution

The patch basically replaces the current Windows implementation of get_password
with one copied from the termios version and edited to work on Windows. It
should provide behavior comparable to that on Unix (in terms of arrow key,
backspace, etc. behaviour).

It has not been extensively tested.
Comment 2 Davi Arnaut 2007-08-08 17:50:09 UTC
A few comments: the last printf("\n") goes to stdout, GetStdHandle() might
return NULL, the return is "wrong". Otherwise looks good.
Comment 3 Ondra Hosek 2007-08-09 02:53:56 UTC
If the printf going to stdout and the return are wrong, then it's probably a
problem with the termios implementation a few lines up too. I confess I was
aggressively copypasting.

As for the NULL on GetStdHandle, yeah, sorry for that oversight. I overlooked
the corresponding paragraph on MSDN.

Anyway, thanks for the positive feedback. It's the first time I'm contributing
to an Apache project.
Comment 4 William A. Rowe Jr. 2007-12-26 12:43:05 UTC
AIUI the unix termios flavor actually lets you redirect password input from the
current stream, correct?

This patch would break that concept horridly, if applicable.