Maciej W. Rozycki wrote:
On Tue, 24 Jul 2007, Matthew Woehlke wrote:
This rough patch adds a '--ask-password' option to wget. About all that can be
It's a good rule to send patches inlined rather than attached as that
makes them easy to comment on.
It's also a good rule to send patches as attachments so that dumb
mailers don't break them :-). (And some mailers, e.g. Thunderbird, will
inline text attachments when replying anyway.)
said for it is that it works; hopefully it will serve as a useful proof of
concept and possible a starting point for a fully-developed feature.
Hmm, what's there to prove? Anyway...
"Demonstration implementation"?
Index: src/http.c
===================================================================
--- src/http.c (revision 2295)
+++ src/http.c (working copy)
@@ -31,6 +31,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <termios.h> /* FIXME probably not portable? */
#ifdef HAVE_UNISTD_H
# include <unistd.h>
#endif
AC_SYS_POSIX_TERMIOS
@@ -1419,7 +1421,29 @@
passwd = u->passwd;
search_netrc (u->host, (const char **)&user, (const char **)&passwd, 0);
user = user ? user : (opt.http_user ? opt.http_user : opt.user);
+ if (opt.ask_passwd)
+ {
+ int in_fd = fileno(stdin);
Standard input is used for URLs; you probably want to use "stderr" here.
Read from stderr? I admit I've heard stderr is bi-directional but I
can't say I've ever seen it used to read input.
Formatting (ditto all the function calls below).
I both dislike GNU formatting and am not familiar with it, so obviously
someone that understands it will have to fix this.
+ printf("URL \"%s\"\nPassword for user \"%s\": ", u->url, user);
Standard output is used for storing remote files; you definitely want to
use "stderr" here.
Good point.
+ /* TODO check error */
+ tcgetattr(in_fd, &attr);
isatty() beforehand perhaps?
Hmm, good point #2. I did say "it works" not "it's a good patch", right? ;-)
Though I am not sure whether it is
really needed given that many years have passed and nobody wanted such a
feature.
Which strikes me as odd, really, but I would guess not so many people
use wget for sites that need a password (I never did until recently), at
least not on non-private computers. I.e. if they do, they're ok with the
existing security risks.
Otherwise it looks OK, I think.
But the decision is up to the maintainer (once you sort out
technical problems).
I very much doubt this patch will be accepted, there are huge problems
with it yet (like, it should persist the password, work with ftp, etc).
It was meant more as a starting point, and for commentary... and you've
made some good comments, thanks!
--
Matthew
Microsoft: Expect the unexpected