Re: [Bug-wget] [bug #48811] netrc password wins over interactive --ask-password
Hi All, I've sent patch for this before already, but it hasn't been reviewed yet. Besides logic correction, I've rebuilt/cleaned up code a bit. Piotr On 20/12/16 00:12, Vijeth Aradhya wrote: Follow-up Comment #2, bug #48811 (project wget): Hi Basin, I'm new to the wget community, but I have looked through the program logic for ftp and http password and realized it is wrong. BEFORE changes Initially, search_netrc function was called first, and if it existed, the password was assigned to passwd, and used in the next logic step ( this is WRONG because if the user entered the password with --ask-password, then that should be given priority ). passwd = passwd ? passwd : .. AFTER changes I have changed the logic such that it first sees if there is an --ask-password passwd stored, and if it is not there, then it uses the netrc password. I have attached the corresponding patch file to this. Please have a look :-) This would be my first contribution to wget (GNU) as well and so eagerly waiting for the code review! (file #39280) ___ Additional Item Attachment: File name: 0001-Corrected-the-logic-for-ftp-and-http-password-usage-auth_varadhya.patch Size:2 KB ___ Reply to this item at: <http://savannah.gnu.org/bugs/?48811> ___ Message sent via/by Savannah http://savannah.gnu.org/
Re: [Bug-wget] [PATCH v2] bug #48811: netrc password wins over interactive --ask-password
Hi Tim,
Just sent new version of this patch.
Shouldn't be any memleaks now.
Piotr
On 21/10/16 12:36, Tim Rühsen wrote:
Hi Piotr,
please include netrc.h in utils.c.
In getftp():
struct net_credentials *ftp_cred = malloc (sizeof *ftp_cred);
...
ftp_cred = pick_credentials()
This looks like a memleak. Also, where do you free ftp_cred ?
Is it really necessary to allocate ftp_cred on each call to getftp (just a
question, maybe it is) ?
Basically the same in gethttp()...
Try 'make check-valgrind' to test for some kinds of memleaks.
Regards, Tim
On Mittwoch, 19. Oktober 2016 11:53:22 CEST losgrandes wrote:
* src/ftp.c: Leverage new struct net_credentials and function
pick_credentials. pick_credentials is responsible for taking proper order
when selecting source of credentials. * src/http.c: Leverage new struct
net_credentials and function pick_credentials. * src/utils.c: New function
pick_credentials.
* src/utils.h: New struct net_credentials.
---
src/ftp.c | 19 +++
src/http.c | 18 +++---
src/utils.c | 19 +++
src/utils.h | 11 +++
4 files changed, 44 insertions(+), 23 deletions(-)
diff --git a/src/ftp.c b/src/ftp.c
index 39f20fa..cc98ca3 100644
--- a/src/ftp.c
+++ b/src/ftp.c
@@ -327,7 +327,8 @@ getftp (struct url *u, struct url *original_url,
uerr_t err = RETROK; /* appease the compiler */
FILE *fp = NULL;
char *respline, *tms;
- const char *user, *passwd, *tmrate;
+ const char *tmrate;
+ struct net_credentials *ftp_cred = malloc (sizeof *ftp_cred);
int cmd = con->cmd;
wgint expected_bytes = 0;
bool got_expected_bytes = false;
@@ -359,13 +360,7 @@ getftp (struct url *u, struct url *original_url,
*qtyread = restval;
- user = u->user;
- passwd = u->passwd;
- search_netrc (u->host, (const char **), (const char **), 1);
- user = user ? user : (opt.ftp_user ? opt.ftp_user : opt.user);
- if (!user) user = "anonymous";
- passwd = passwd ? passwd : (opt.ftp_passwd ? opt.ftp_passwd :
opt.passwd); - if (!passwd) passwd = "-wget@";
+ ftp_cred = pick_credentials (u, opt.ftp_user, opt.ftp_passwd, opt.user,
opt.passwd, 1);
dtsock = -1;
local_sock = -1;
@@ -461,18 +456,18 @@ getftp (struct url *u, struct url *original_url,
/* Second: Login with proper USER/PASS sequence. */
logprintf (LOG_VERBOSE, _("Logging in as %s ... "),
- quotearg_style (escape_quoting_style, user));
+ quotearg_style (escape_quoting_style, ftp_cred->user));
if (opt.server_response)
logputs (LOG_ALWAYS, "\n");
if (con->proxy)
{
/* If proxy is in use, log in as username@target-site. */
- char *logname = concat_strings (user, "@", u->host, (char *) 0);
- err = ftp_login (csock, logname, passwd);
+ char *logname = concat_strings (ftp_cred->user, "@", u->host,
(char *) 0); + err = ftp_login (csock, logname, ftp_cred->passwd);
xfree (logname);
}
else
-err = ftp_login (csock, user, passwd);
+err = ftp_login (csock, ftp_cred->user, ftp_cred->passwd);
/* FTPRERR, FTPSRVERR, WRITEFAILED, FTPLOGREFUSED, FTPLOGINC */
switch (err)
diff --git a/src/http.c b/src/http.c
index 7e2c4ec..41eaa42 100644
--- a/src/http.c
+++ b/src/http.c
@@ -1813,7 +1813,7 @@ time_to_rfc1123 (time_t time, char *buf, size_t
bufsize) static struct request *
initialize_request (const struct url *u, struct http_stat *hs, int *dt,
struct url *proxy, bool inhibit_keep_alive, bool *basic_auth_finished, -
wgint *body_data_size, char **user, char **passwd, uerr_t
*ret) +wgint *body_data_size, struct net_credentials
**http_cred, uerr_t *ret) {
bool head_only = !!(*dt & HEAD_ONLY);
struct request *req;
@@ -1876,20 +1876,16 @@ initialize_request (const struct url *u, struct
http_stat *hs, int *dt, struct u request_set_header (req,
"Accept-Encoding", "identity", rel_none);
/* Find the username and password for authentication. */
- *user = u->user;
- *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);
- *passwd = *passwd ? *passwd : (opt.http_passwd ? opt.http_passwd :
opt.passwd); + *http_cred = pick_credentials (u, opt.http_user,
opt.http_passwd, opt.user, opt.passwd, 0);
/* We only do "site-wide" authentication with "global" user/password
* values unless --auth-no-challange has been requested; URL
user/password * info overrides. */
- if (*user && *passwd && (!u->user || opt.auth_without_challenge))
+ if ((*http_cred)->user && (*http_cred)->passwd && (!u->user ||
opt.auth_without_challenge)) {
/* If this is a host for wh
[Bug-wget] [PATCH v3] bug #48811: netrc password wins over interactive --ask-password
* utils.h: New struct net_credentials and pick_credentials function
* utils.c: pick_credentials for selecting FTP/HTTP user/passwd according to
importance
* retr.c: define net_cred structure to hold user/passwd pair
* ftp.c: call pick_credentials before getftp
* http.c: call pick_credentials before gethttp
---
src/ftp.c | 25 ++---
src/http.c | 26 +++---
src/retr.c | 3 +++
src/utils.c | 20
src/utils.h | 11 +++
5 files changed, 55 insertions(+), 30 deletions(-)
diff --git a/src/ftp.c b/src/ftp.c
index 947d8f2..42c84ad 100644
--- a/src/ftp.c
+++ b/src/ftp.c
@@ -81,6 +81,7 @@ typedef struct
struct url *proxy;/* FTWK-style proxy */
} ccon;
+extern struct net_credentials net_cred;
/* Look for regexp "( *[0-9]+ *byte" (literal parenthesis) anywhere in
the string S, and return the number converted to wgint, if found, 0
@@ -321,13 +322,13 @@ static uerr_t
getftp (struct url *u, struct url *original_url,
wgint passed_expected_bytes, wgint *qtyread,
wgint restval, ccon *con, int count, wgint *last_expected_bytes,
-FILE *warc_tmp)
+FILE *warc_tmp, struct net_credentials *ftp_cred)
{
int csock, dtsock, local_sock, res;
uerr_t err = RETROK; /* appease the compiler */
FILE *fp = NULL;
char *respline, *tms;
- const char *user, *passwd, *tmrate;
+ const char *tmrate;
int cmd = con->cmd;
wgint expected_bytes = 0;
bool got_expected_bytes = false;
@@ -359,14 +360,6 @@ getftp (struct url *u, struct url *original_url,
*qtyread = restval;
- user = u->user;
- passwd = u->passwd;
- search_netrc (u->host, (const char **), (const char **), 1);
- user = user ? user : (opt.ftp_user ? opt.ftp_user : opt.user);
- if (!user) user = "anonymous";
- passwd = passwd ? passwd : (opt.ftp_passwd ? opt.ftp_passwd : opt.passwd);
- if (!passwd) passwd = "-wget@";
-
dtsock = -1;
local_sock = -1;
con->dltime = 0;
@@ -461,18 +454,18 @@ getftp (struct url *u, struct url *original_url,
/* Second: Login with proper USER/PASS sequence. */
logprintf (LOG_VERBOSE, _("Logging in as %s ... "),
- quotearg_style (escape_quoting_style, user));
+ quotearg_style (escape_quoting_style, ftp_cred->user));
if (opt.server_response)
logputs (LOG_ALWAYS, "\n");
if (con->proxy)
{
/* If proxy is in use, log in as username@target-site. */
- char *logname = concat_strings (user, "@", u->host, (char *) 0);
- err = ftp_login (csock, logname, passwd);
+ char *logname = concat_strings (ftp_cred->user, "@", u->host, (char
*) 0);
+ err = ftp_login (csock, logname, ftp_cred->passwd);
xfree (logname);
}
else
-err = ftp_login (csock, user, passwd);
+err = ftp_login (csock, ftp_cred->user, ftp_cred->passwd);
/* FTPRERR, FTPSRVERR, WRITEFAILED, FTPLOGREFUSED, FTPLOGINC */
switch (err)
@@ -1937,10 +1930,12 @@ ftp_loop_internal (struct url *u, struct url
*original_url, struct fileinfo *f,
else
len = 0;
+ pick_credentials (u, opt.ftp_user, opt.ftp_passwd, opt.user, opt.passwd,
1, _cred);
+
/* If we are working on a WARC record, getftp should also write
to the warc_tmp file. */
err = getftp (u, original_url, len, , restval, con, count,
-_expected_bytes, warc_tmp);
+_expected_bytes, warc_tmp, _cred);
if (con->csock == -1)
con->st &= ~DONE_CWD;
diff --git a/src/http.c b/src/http.c
index 67b3686..fbd10dd 100644
--- a/src/http.c
+++ b/src/http.c
@@ -79,6 +79,7 @@ as that of the covered work. */
# include "vms.h"
#endif /* def __VMS */
+extern struct net_credentials net_cred;
/* Forward decls. */
struct http_stat;
@@ -1813,7 +1814,7 @@ time_to_rfc1123 (time_t time, char *buf, size_t bufsize)
static struct request *
initialize_request (const struct url *u, struct http_stat *hs, int *dt, struct
url *proxy,
bool inhibit_keep_alive, bool *basic_auth_finished,
-wgint *body_data_size, char **user, char **passwd, uerr_t
*ret)
+wgint *body_data_size, struct net_credentials **http_cred,
uerr_t *ret)
{
bool head_only = !!(*dt & HEAD_ONLY);
struct request *req;
@@ -1875,21 +1876,14 @@ initialize_request (const struct url *u, struct
http_stat *hs, int *dt, struct u
request_set_header (req, "Accept", "*/*", rel_none);
request_set_header (req, "Accept-Encoding", "identity", rel_none);
- /* Find the username and password for authentication. */
- *user = u->user;
- *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);
- *passwd = *passwd ? *passwd : (opt.http_passwd ? opt.http_passwd :
opt.passwd);
-
/* We only do
Re: [Bug-wget] Does -o work correctly now?
Patch sent, should be good now. Piotr On 08/11/16 22:29, Tim Rühsen wrote: On Dienstag, 8. November 2016 15:26:14 CET Dale R. Worley wrote: I've been getting a script working, and as part of it, it appears that -o does not work correctly. Specifically -o is supposed to send all "messages" to a specified log file rather than stderr. But what I see is that no messages are sent to the log file. The command in question is: wget --no-verbose \ -o ~/temp/log-file \ --mirror --trust-server-names --convert-links --page-requisites \ --include-directories=/assignments \ --limit-rate=20k \ http://www.iana.org/assignments/index.html With the (now obsolete) wget distributed with my OS, GNU Wget 1.16.1, -o behaves as documented. With wget from commit 00ae9b4 (which I think is the latest), which reports itself as GNU Wget 1.18.88-00ae-dirty, -o seems to have no effect. There are any number of mistakes I could have made in this test, but since the symptom is so simple, I figured I'd ask if anyone else has noticed whether -o works or does not in the latest commits. Hi Dale, you are right. Looks like commit dd5c549f6af8e1143e1a6ef66725eea4bcd9ad50 broke it. Sadly, the test suite doesn't catch it. @Piotr Could you take a look ? Regards, Tim
[Bug-wget] [PATCH] Respect -o parameter
* log.c: don't choose log output dynamically when opt.lfilename is set
---
src/log.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/log.c b/src/log.c
index e068acf..51f30c4 100644
--- a/src/log.c
+++ b/src/log.c
@@ -956,11 +956,11 @@ static void
check_redirect_output (void)
{
#ifndef WINDOWS
- /* If it was redirected already to log file by SIGHUP or SIGUSR1,
- * it was permanent and since that redirect_request_signal_name is set.
+ /* If it was redirected already to log file by SIGHUP, SIGUSR1 or -o
parameter,
+ * it was permanent.
* If there was no SIGHUP or SIGUSR1 and shell is interactive
* we check if process is fg or bg before every line is printed.*/
- if (!redirect_request_signal_name && shell_is_interactive)
+ if (!redirect_request_signal_name && shell_is_interactive && !opt.lfilename)
{
if (tcgetpgrp (STDIN_FILENO) != getpgrp ())
{
--
1.9.1
Re: [Bug-wget] Does -o work correctly now?
Yeah, I'll look into it. Piotr On 08/11/16 22:29, Tim Rühsen wrote: On Dienstag, 8. November 2016 15:26:14 CET Dale R. Worley wrote: I've been getting a script working, and as part of it, it appears that -o does not work correctly. Specifically -o is supposed to send all "messages" to a specified log file rather than stderr. But what I see is that no messages are sent to the log file. The command in question is: wget --no-verbose \ -o ~/temp/log-file \ --mirror --trust-server-names --convert-links --page-requisites \ --include-directories=/assignments \ --limit-rate=20k \ http://www.iana.org/assignments/index.html With the (now obsolete) wget distributed with my OS, GNU Wget 1.16.1, -o behaves as documented. With wget from commit 00ae9b4 (which I think is the latest), which reports itself as GNU Wget 1.18.88-00ae-dirty, -o seems to have no effect. There are any number of mistakes I could have made in this test, but since the symptom is so simple, I figured I'd ask if anyone else has noticed whether -o works or does not in the latest commits. Hi Dale, you are right. Looks like commit dd5c549f6af8e1143e1a6ef66725eea4bcd9ad50 broke it. Sadly, the test suite doesn't catch it. @Piotr Could you take a look ? Regards, Tim
[Bug-wget] --unlink is ineffective
Hi, there seem to be an issue with wget. Whenever you run it with --unlink and the file already exists locally the file is not overwritten. It appears that the decision whether to unlink is made after a new numbered name is generated and stored in hs->local_file. As before unlink() a check is done vs the new name the file with original name is never deleted. Per wget manual: --unlink Force Wget to unlink file instead of clobbering existing file. This option is useful for downloading to the directory with hardlinks. I'd expect it to unlink the file instead of creating a new name for it. This was reproduced with http on wget 1.15 and 1.17.1.
[Bug-wget] Fwd: Re: [PATCH v3] bug #45790: wget prints it's progress even when background
Forgot to include mailinglist. Forwarded Message Subject: Re: [Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background Date: Wed, 19 Oct 2016 12:18:13 +0200 From: Wajda, Piotr <pwa...@gmail.net.pl> To: Eli Zaretskii <e...@gnu.org> For CTRL+Break we could probably go to background on windows by forking process using current fake_fork method. Child process should be then started with -c and -b. Piotr On 19/10/16 12:12, Eli Zaretskii wrote: From: "Wajda, Piotr" <pwa...@gmail.net.pl> Date: Wed, 19 Oct 2016 11:57:06 +0200 My only confusion was that during testing on windows, when sending CTRL+C or CTRL+Break it immediately terminates, which is basically what I think it should do for CTRL+C. Not sure about CTRL+Break. What else is reasonable for CTRL+Break? We can arrange for them to produce different effects, if there are two alternative behaviors that would make sense. Thanks.
Re: [Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background
Sorry, I had the impression there was some discussion going on !? My only confusion was that during testing on windows, when sending CTRL+C or CTRL+Break it immediately terminates, which is basically what I think it should do for CTRL+C. Not sure about CTRL+Break. But the same behaviour was with current wget version (without my patches), so I think it's correct. But apart from that: - please add a space after if statements. - please add a space between function name and (. - please make local commits with appropriate commit messages (see git log for examples), generate your patches with 'git format-patch -3' (-3 for your last 3 commits) and send them as attachments. Corrected and sent. Thanks Piotr
Re: [Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background
Can you please review my 3 patches? Piotr On 07/10/16 10:10, Wajda, Piotr wrote: From: losgrandes <pwa...@gmail.net.pl> Date: Thu, 6 Oct 2016 09:47:01 +0200 Fortunately I tested wget.exe in normal mode and background mode (-b). Was ok. Unfortunately I haven't tested wget.exe with CTRL+Break/CTRL+C (is it really works on windows?). Yes, CTRL-C/CTRL-BREAK should work on Windows. What didn't work in your case? I've tested ctrl+c/ctrl+break with current wget and my patched version. In all cases process is terminated. I think patch is ready to be reviewed. Piotr
Re: [Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background
From: losgrandes <pwa...@gmail.net.pl> Date: Thu, 6 Oct 2016 09:47:01 +0200 Fortunately I tested wget.exe in normal mode and background mode (-b). Was ok. Unfortunately I haven't tested wget.exe with CTRL+Break/CTRL+C (is it really works on windows?). Yes, CTRL-C/CTRL-BREAK should work on Windows. What didn't work in your case? I've tested ctrl+c/ctrl+break with current wget and my patched version. In all cases process is terminated. I think patch is ready to be reviewed. Piotr
Re: [Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background
Eli, you're awesome! :) I tried many combinations (I think so), including LIBS=-liconv, but without success. I've tried now and it works! Now I'll test on my own. Thanks Piotr On 07/10/16 09:08, Eli Zaretskii wrote: From: losgrandes <pwa...@gmail.net.pl> Date: Thu, 6 Oct 2016 09:47:01 +0200 Fortunately I tested wget.exe in normal mode and background mode (-b). Was ok. Unfortunately I haven't tested wget.exe with CTRL+Break/CTRL+C (is it really works on windows?). Yes, CTRL-C/CTRL-BREAK should work on Windows. What didn't work in your case? 1. Could you be so kind and test my wget.exe with CTRL+Break? Send your test instructions, and I will try to build and test it here. 2. Advise me with error I get while compiling: main.o:main.c:(.text+0x579): undefined reference to `pipe' The Windows runtime doesn't have 'pipe', it has '_pipe' (with a slightly different argument list). I believe we need the Gnulib pipe module to get this to compile. However, just as a quick hack, replace the call to 'pipe' with a corresponding call to '_pipe', you can find its documentation here: https://msdn.microsoft.com/en-us/library/edze9h7e.aspx (This problem is unrelated to your changes, the call to 'pipe' is already in the repository.) url.o:url.c:(.text+0x1e78): undefined reference to `libiconv_open' url.o:url.c:(.text+0x1f25): undefined reference to `libiconv' url.o:url.c:(.text+0x1f57): undefined reference to `libiconv' url.o:url.c:(.text+0x1f7e): undefined reference to `libiconv_close' url.o:url.c:(.text+0x20ee): undefined reference to `libiconv_close' collect2: error: ld returned 1 exit status This was generated by: ./configure --host=i686-w64-mingw32 --without-ssl --without-libidn --without-metalink --with-gpgme-prefix=/dev/null CFLAGS=-I$BUILDDIR/tmp/include LDFLAGS=-L$BUILDDIR/tmp/lib --with-libiconv-prefix=$BUILDDIR/tmp CFLAGS=-liconv I think you need to add -liconv to LIBS, not to CFLAGS. GNU ld is a one-pass linker, so it needs to see -liconv _after_ all the object files, not before.
Re: [Bug-wget] [PATCH v3] bug #45790: wget prints it's progress even when background
Sorry for incorrect Date email header in that email, I've send it from
previously suspended virtual machine that I forgot to ntpupdate.
Piotr
On 06/10/16 09:47, losgrandes wrote:
I've updated patch to not run tcgetpgrp, getpgrp and isatty (for
shell_is_interactive test) on windows.
I have compiled and tested on linux and (mostly*) on windows. Should be ok.
* - I was able to create working mingw env on linux, but cross-compilation
worked for me only few times (I know, silly, I broke it somehow).
Fortunately I tested wget.exe in normal mode and background mode (-b). Was ok.
Unfortunately I haven't tested wget.exe with CTRL+Break/CTRL+C (is it really
works on windows?).
I'm struggling with mingw, but no luck so far.
So here're my asks for you:
1. Could you be so kind and test my wget.exe with CTRL+Break?
and/or
2. Advise me with error I get while compiling:
main.o:main.c:(.text+0x579): undefined reference to `pipe'
url.o:url.c:(.text+0x1e78): undefined reference to `libiconv_open'
url.o:url.c:(.text+0x1f25): undefined reference to `libiconv'
url.o:url.c:(.text+0x1f57): undefined reference to `libiconv'
url.o:url.c:(.text+0x1f7e): undefined reference to `libiconv_close'
url.o:url.c:(.text+0x20ee): undefined reference to `libiconv_close'
collect2: error: ld returned 1 exit status
This was generated by:
./configure --host=i686-w64-mingw32 --without-ssl --without-libidn
--without-metalink --with-gpgme-prefix=/dev/null CFLAGS=-I$BUILDDIR/tmp/include
LDFLAGS=-L$BUILDDIR/tmp/lib --with-libiconv-prefix=$BUILDDIR/tmp CFLAGS=-liconv
make
Thanks
Piotr
---
src/log.c | 125 +---
src/log.h | 1 +
src/main.c | 2 +-
src/mswindows.c | 5 +--
4 files changed, 85 insertions(+), 48 deletions(-)
diff --git a/src/log.c b/src/log.c
index a1338ca..9a84c85 100644
--- a/src/log.c
+++ b/src/log.c
@@ -80,6 +80,18 @@ as that of the covered work. */
logging is inhibited, logfp is set back to NULL. */
static FILE *logfp;
+/* Descriptor of the stdout|stderr */
+static FILE *stdlogfp;
+
+/* Descriptor of the wget.log* file (if created) */
+static FILE *filelogfp;
+
+/* Name of log file */
+static char *logfile;
+
+/* Is interactive shell ? */
+static int shell_is_interactive;
+
/* A second file descriptor pointing to the temporary log file for the
WARC writer. If WARC writing is disabled, this is NULL. */
static FILE *warclogfp;
@@ -611,16 +623,18 @@ log_init (const char *file, bool appendp)
{
if (HYPHENP (file))
{
- logfp = stdout;
+ stdlogfp = stdout;
+ logfp = stdlogfp;
}
else
{
- logfp = fopen (file, appendp ? "a" : "w");
- if (!logfp)
+ filelogfp = fopen (file, appendp ? "a" : "w");
+ if (!filelogfp)
{
fprintf (stderr, "%s: %s: %s\n", exec_name, file, strerror
(errno));
exit (WGET_EXIT_GENERIC_ERROR);
}
+ logfp = filelogfp;
}
}
else
@@ -631,7 +645,8 @@ log_init (const char *file, bool appendp)
stderr only if the user actually specifies `-O -'. He says
this inconsistency is harder to document, but is overall
easier on the user. */
- logfp = stderr;
+ stdlogfp = stderr;
+ logfp = stdlogfp;
if (1
#ifdef HAVE_ISATTY
@@ -646,6 +661,11 @@ log_init (const char *file, bool appendp)
save_context_p = true;
}
}
+
+#ifndef WINDOWS
+ /* Initialize this values so we don't have to ask every time we print line */
+ shell_is_interactive = isatty (STDIN_FILENO);
+#endif
}
/* Close LOGFP (only if we opened it, not if it's stderr), inhibit
@@ -880,59 +900,78 @@ log_cleanup (void)
/* When SIGHUP or SIGUSR1 are received, the output is redirected
elsewhere. Such redirection is only allowed once. */
-static enum { RR_NONE, RR_REQUESTED, RR_DONE } redirect_request = RR_NONE;
static const char *redirect_request_signal_name;
-/* Redirect output to `wget-log'. */
+/* Redirect output to `wget-log' or back to stdout/stderr. */
-static void
-redirect_output (void)
+void
+redirect_output (bool to_file, const char *signal_name)
{
- char *logfile;
- logfp = unique_create (DEFAULT_LOGFILE, false, );
- if (logfp)
+ if(to_file && logfp != filelogfp)
{
- fprintf (stderr, _("\n%s received, redirecting output to %s.\n"),
- redirect_request_signal_name, quote (logfile));
- xfree (logfile);
- /* Dump the context output to the newly opened log. */
- log_dump_context ();
+ if (signal_name)
+{
+ fprintf(stderr, "\n%s received.", signal_name);
+}
+ if (!filelogfp)
+{
+ filelogfp = unique_create (DEFAULT_LOGFILE, false, );
+ if (filelogfp)
+{
+ fprintf
[Bug-wget] C developer keen to help
Hi all, My name is Piotr Zacharzewski, I am searching for a free software project to get involved in. You can read more about me on https://savannah.gnu.org/users/pzach. I must say I've never contributed to a project this way, although I have experience in collaborating with people on software projects. I have around 2 years of practical C experience but only very basic TCP/IP knowledge from classes. I tend to learn fast though and I have some spare time in the evenings, weekends regularly. Do you think I could join in and help out? kind regards Peter signature.asc Description: OpenPGP digital signature
[Bug-wget] [PATCH v2] bug #45790: wget prints it's progress even when background
Hi, Reworked recent patch to behave correctly on fg and bg. Now user can switch
from fg to bg and vice versa and wget will select fd accordingly.
Please review.
Thanks
Piotr
---
src/log.c | 116 +++--
src/log.h | 1 +
src/main.c | 2 +-
3 files changed, 76 insertions(+), 43 deletions(-)
diff --git a/src/log.c b/src/log.c
index a1338ca..f93b02c 100644
--- a/src/log.c
+++ b/src/log.c
@@ -80,6 +80,18 @@ as that of the covered work. */
logging is inhibited, logfp is set back to NULL. */
static FILE *logfp;
+/* Descriptor of the stdout|stderr */
+static FILE *stdlogfp;
+
+/* Descriptor of the wget.log* file (if created) */
+static FILE *filelogfp;
+
+/* Name of log file */
+static char *logfile;
+
+/* Is interactive shell ? */
+static int shell_is_interactive;
+
/* A second file descriptor pointing to the temporary log file for the
WARC writer. If WARC writing is disabled, this is NULL. */
static FILE *warclogfp;
@@ -611,16 +623,18 @@ log_init (const char *file, bool appendp)
{
if (HYPHENP (file))
{
- logfp = stdout;
+ stdlogfp = stdout;
+ logfp = stdlogfp;
}
else
{
- logfp = fopen (file, appendp ? "a" : "w");
+ filelogfp = fopen (file, appendp ? "a" : "w");
if (!logfp)
{
fprintf (stderr, "%s: %s: %s\n", exec_name, file, strerror
(errno));
exit (WGET_EXIT_GENERIC_ERROR);
}
+ logfp = filelogfp;
}
}
else
@@ -631,7 +645,8 @@ log_init (const char *file, bool appendp)
stderr only if the user actually specifies `-O -'. He says
this inconsistency is harder to document, but is overall
easier on the user. */
- logfp = stderr;
+ stdlogfp = stderr;
+ logfp = stdlogfp;
if (1
#ifdef HAVE_ISATTY
@@ -646,6 +661,9 @@ log_init (const char *file, bool appendp)
save_context_p = true;
}
}
+
+ /* Initialize this values so we don't have to ask every time we print line */
+ shell_is_interactive = isatty (STDIN_FILENO);
}
/* Close LOGFP (only if we opened it, not if it's stderr), inhibit
@@ -880,59 +898,73 @@ log_cleanup (void)
/* When SIGHUP or SIGUSR1 are received, the output is redirected
elsewhere. Such redirection is only allowed once. */
-static enum { RR_NONE, RR_REQUESTED, RR_DONE } redirect_request = RR_NONE;
static const char *redirect_request_signal_name;
-/* Redirect output to `wget-log'. */
+/* Redirect output to `wget-log' or back to stdout/stderr. */
-static void
-redirect_output (void)
+void
+redirect_output (bool to_file, const char *signal_name)
{
- char *logfile;
- logfp = unique_create (DEFAULT_LOGFILE, false, );
- if (logfp)
+ if(to_file && logfp != filelogfp)
{
- fprintf (stderr, _("\n%s received, redirecting output to %s.\n"),
- redirect_request_signal_name, quote (logfile));
- xfree (logfile);
- /* Dump the context output to the newly opened log. */
- log_dump_context ();
+ if (signal_name)
+{
+ fprintf(stderr, "\n%s received.", signal_name);
+}
+ if (!filelogfp)
+{
+ filelogfp = unique_create (DEFAULT_LOGFILE, false, );
+ if (filelogfp)
+{
+ fprintf (stderr, _("\nRedirecting output to %s.\n"),
+ quote (logfile));
+ /* Store signal name to tell wget it's permanent redirect to log
file */
+ redirect_request_signal_name = signal_name;
+ logfp = filelogfp;
+ /* Dump the context output to the newly opened log. */
+ log_dump_context ();
+}
+ else
+{
+ /* Eek! Opening the alternate log file has failed. Nothing we
+can do but disable printing completely. */
+ fprintf (stderr, _("%s: %s; disabling logging.\n"),
+ (logfile) ? logfile : DEFAULT_LOGFILE, strerror (errno));
+ inhibit_logging = true;
+}
+}
+ else
+{
+ fprintf (stderr, _("\nRedirecting output to %s.\n"),
+ quote (logfile));
+ logfp = filelogfp;
+ log_dump_context ();
+}
}
- else
+ else if (!to_file && logfp != stdlogfp)
{
- /* Eek! Opening the alternate log file has failed. Nothing we
- can do but disable printing completely. */
- fprintf (stderr, _("\n%s received.\n"), redirect_request_signal_name);
- fprintf (stderr, _("%s: %s; disabling logging.\n"),
- (logfile) ? logfile : DEFAULT_LOGFILE, strerror (errno));
- inhibit_logging = true;
+ logfp = stdlogfp;
+ log_dump_context ();
}
-
Re: [Bug-wget] bug #45790: wget prints it's progress even when background
I would like to avoid forcing users to hack like this ;). Wget should print to std* when in fg and print to wget.log when in bg, no matter how user gets there. I don't think getpgrp() == tcgetpgrp(STDOUT_FILENO) is heavy and should probaby be ok to check it when printing lines. Piotr 28 wrz 2016 17:47 wor...@alum.mit.edu napisał(a): > > "Wajda, Piotr" writes: > > The case with stopping wget is obvious. CTRL+Z and bg should make wget > > write to file and I can catch bg with SIGCONT. > > But I wonder what to do when after CTRL+Z and bg, user runs fg. In this > > case there's no signal between bg anf fg, > > Though the user could, instead of just "fg", do "fg", then Ctrl-Z, then > "fg" again. The second "fg" would cause a SIGCONT, and wget could at > that point theck that it had been foregrounded. Not elegant, but fairly > simple. > > Dale
Re: [Bug-wget] bug #45790: wget prints it's progress even when background
Hi,
The case with stopping wget is obvious. CTRL+Z and bg should make wget
write to file and I can catch bg with SIGCONT.
But I wonder what to do when after CTRL+Z and bg, user runs fg. In this
case there's no signal between bg anf fg, and I can only check for
getpgrp() == tcgetpgrp(STDOUT_FILENO) in e.g. check_redirect_output(),
right?
In other words, is there better approach than checking getpgrp() ==
tcgetpgrp(STDOUT_FILENO) before printing every line?
Thanks
Piotr
On 19/09/16 19:30, Darshit Shah wrote:
Hi Piotr,
The patch looks fine. However, when Wget is foregrounded again, the
progress bar remains invisible. When the process is foregrounded again,
you should undo the effects of `redirect_output_signal()`
* pwa...@gmail.net.pl <pwa...@gmail.net.pl> [160919 18:01]:
Hi Darshit,
Sorry for pasting patch into email incorrectly. I've send 2 other
patches before, but as attachments, so they should be fine.
Thanks
Piotr
W dniu 19.09.2016 o 17:28, Darshit Shah pisze:
Hi Piotr,
Thanks for your interest in Wget. I shall review your patch soon.
However, for future reference please do not send patches that are
pasted into the mail like this. It makes it extremely difficult for
us to apply the patch. I was unable to apply the provided diff after
simply saving your email.
Either use `git format-patch` to create a patch file that you attach
to the emails, or use `git send-email` to correctly send an inline diff.
* pwa...@gmail.net.pl <pwa...@gmail.net.pl> [160918 18:41]:
Hi,
I've implemented fix for bug #45790. Basically I used approach Noel
showed in comment.
Please check below diff if it's sane.
diff --git a/src/main.c b/src/main.c
index ac6ee2c..f324253 100644
--- a/src/main.c
+++ b/src/main.c
@@ -113,7 +113,7 @@ int numurls = 0;
setting up gettext's message catalog using bindtextdomain and
textdomain. Does nothing if NLS is disabled or missing. */
-#if defined(SIGHUP) || defined(SIGUSR1)
+#if defined(SIGHUP) || defined(SIGUSR1) || defined(SIGCONT)
/* Hangup signal handler. When wget receives SIGHUP or SIGUSR1, it
will proceed operation as usual, trying to write into a log file.
If that is impossible, the output will be turned off. */
@@ -131,12 +131,20 @@ redirect_output_signal (int sig)
if (sig == SIGUSR1)
signal_name = "SIGUSR1";
#endif
+#ifdef SIGCONT
+ if(sig == SIGCONT) {
+/* If process goes to foreground, don't redirect output */
+if(getpgrp() == tcgetpgrp(STDOUT_FILENO))
+ return;
+signal_name = "SIGCONT";
+ }
+#endif
log_request_redirect_output (signal_name);
progress_schedule_redirect ();
signal (sig, redirect_output_signal);
}
-#endif /* defined(SIGHUP) || defined(SIGUSR1) */
+#endif /* defined(SIGHUP) || defined(SIGUSR1) || defined(SIGCONT)*/
static void
i18n_initialize (void)
@@ -2003,6 +2011,9 @@ only if outputting to a regular file.\n"));
#ifdef SIGUSR1
signal (SIGUSR1, redirect_output_signal);
#endif
+#ifdef SIGCONT
+ signal (SIGCONT, redirect_output_signal);
+#endif
#ifdef SIGPIPE
/* Writing to a closed socket normally signals SIGPIPE, and the
process exits. What we want is to ignore SIGPIPE and just check
Thanks
Piotr
[Bug-wget] [PATCH] bug #46584: wget --spider always returns zero exit status
Sending correctly created patch.
Ps. Not sure if I chose FTPNSFOD as a correct exit value name.
Piotr
---
src/ftp.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/ftp.c b/src/ftp.c
index 39f20fa..e05d57b 100644
--- a/src/ftp.c
+++ b/src/ftp.c
@@ -1191,6 +1191,7 @@ Error in server response, closing control
connection.\n"));
if (opt.spider)
{
bool exists = false;
+ bool all_exist = true;
struct fileinfo *f;
uerr_t _res = ftp_get_listing (u, original_url, con, );
/* Set the DO_RETR command flag again, because it gets unset when
@@ -1206,6 +1207,8 @@ Error in server response, closing control
connection.\n"));
{
exists = true;
break;
+} else {
+ all_exist = false;
}
f = f->next;
}
@@ -1226,7 +1229,11 @@ Error in server response, closing control
connection.\n"));
con->csock = -1;
fd_close (dtsock);
fd_close (local_sock);
- return RETRFINISHED;
+ if(all_exist) {
+ return RETRFINISHED;
+ } else {
+ return FTPNSFOD;
+ }
}
if (opt.verbose)
--
1.9.1
[Bug-wget] bug #48811: netrc password wins over interactive --ask-password
I've tried to tackle another bug. I've moved reading from .netrc file to the end, when no user and no passwd is defined (for both http and ftp). Please review. Thanks Piotr diff --git a/src/ftp.c b/src/ftp.c index 39f20fa..359cbce 100644 --- a/src/ftp.c +++ b/src/ftp.c @@ -360,11 +360,10 @@ getftp (struct url *u, struct url *original_url, *qtyread = restval; user = u->user; - passwd = u->passwd; - search_netrc (u->host, (const char **), (const char **), 1); user = user ? user : (opt.ftp_user ? opt.ftp_user : opt.user); + passwd = opt.passwd ? opt.passwd : (u->passwd ? u->passwd : opt.ftp_passwd); + if(!user && !passwd) search_netrc (u->host, (const char **), (const char **), 1); if (!user) user = "anonymous"; - passwd = passwd ? passwd : (opt.ftp_passwd ? opt.ftp_passwd : opt.passwd); if (!passwd) passwd = "-wget@"; dtsock = -1; diff --git a/src/http.c b/src/http.c index 7e2c4ec..9de5cc0 100644 --- a/src/http.c +++ b/src/http.c @@ -1877,10 +1877,9 @@ initialize_request (const struct url *u, struct http_stat *hs, int *dt, struct u /* Find the username and password for authentication. */ *user = u->user; - *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); - *passwd = *passwd ? *passwd : (opt.http_passwd ? opt.http_passwd : opt.passwd); + *passwd = opt.passwd ? opt.passwd : (u->passwd ? u->passwd : opt.http_passwd); + if (!*user && !*passwd) search_netrc (u->host, (const char **)user, (const char **)passwd, 0); /* We only do "site-wide" authentication with "global" user/password * values unless --auth-no-challange has been requested; URL user/password
[Bug-wget] bug #46584: wget --spider always returns zero exit status
Hi,
I'd like to start contributing to wget. I've chosen
http://savannah.gnu.org/bugs/index.php?46584 for a good start.
Please let me know if attached patch is sane.
Thanks
Piotr
diff --git a/src/ftp.c b/src/ftp.c
index 39f20fa..e05d57b 100644
--- a/src/ftp.c
+++ b/src/ftp.c
@@ -1191,6 +1191,7 @@ Error in server response, closing control
connection.\n"));
if (opt.spider)
{
bool exists = false;
+ bool all_exist = true;
struct fileinfo *f;
uerr_t _res = ftp_get_listing (u, original_url, con, );
/* Set the DO_RETR command flag again, because it gets unset when
@@ -1206,6 +1207,8 @@ Error in server response, closing control
connection.\n"));
{
exists = true;
break;
+} else {
+ all_exist = false;
}
f = f->next;
}
@@ -1226,7 +1229,11 @@ Error in server response, closing control
connection.\n"));
con->csock = -1;
fd_close (dtsock);
fd_close (local_sock);
- return RETRFINISHED;
+ if(all_exist) {
+ return RETRFINISHED;
+ } else {
+ return FTPNSFOD;
+ }
}
if (opt.verbose)
