Re: [Bug-wget] [bug #50223] wget 1.19 will not build on MacOS 10.12.3

[Gisle Vanem]

Charles wrote:

> The `make` step fails with this:
> 
> --8<
> /Applications/Xcode.app/Contents/Developer/usr/bin/make  all-am
>   CC   connect.o
>   CC   convert.o
>   CC   cookies.o
>   CC   ftp.o
> ftp.c:1466:19: error: no member named 'rpl_unlink' in 'struct options'
>   if (opt.unlink && file_exists_p (con->target))
>   ~~~ ^
> ../lib/unistd.h:1851:19: note: expanded from macro 'unlink'
> #   define unlink rpl_unlink

Looks like the same issue I had with mswindows.h.
The fix was to put '#include ' before
Gnulib's unlink() got a chance to get defined as
rpl_unlink().

-- 
--gv

Re: [Bug-wget] bug #45790: wget prints it's progress even when background

[Dale R. Worley]

Piotr  writes:
> 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.

That makes sense, though I'd be careful to check for errors returned
from tcgetpgrp().

Dale

Re: [Bug-wget] bug #45790: wget prints it's progress even when background

[Piotr]

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

[Dale R. Worley]

"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

[Wajda, Piotr]

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  [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  [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

Re: [Bug-wget] bug #46584: wget --spider always returns zero exit status

[Giuseppe Scrivano]

Darshit Shah  writes:

> Apart from the patch format, the patch itself looks good.
>
> @Giuseppe: We will require Copyright assignments for Piotr right? This
> patch may be small, but there are a couple others in the pipeline.

yes, I think we will need that.  The sum of all the contributions cannot
be considered trivial.

Regards,
Giuseppe

Re: [Bug-wget] bug #46584: wget --spider always returns zero exit status

[Darshit Shah]

It's correct. That is because `git format-patch` is often used together 
with `git send-email`. It formats the patch as an email that can be 
directly passed to your MUA. What you sent is a correct form of the 
patch that is easy for us to apply.


However, notice that you sent the patch on the wrong thread. I'll review 
the patch on the correct thread.


* pwa...@gmail.net.pl  [160919 19:07]:
I'm not yet fully familiar with git format-patch (weird for me that 
it's adding email-like headers. Is it suppose to be email creation 
tool for patches?), I believe it will work for you.


Thanks
Piotr

W dniu 19.09.2016 o 18:56, Darshit Shah pisze:

Hi Piotr,

How did you create this patch? Because git refuses to accept it.
Patch format detection fails. Please regenerate all your patches 
using `git format-patch` so that we can apply the patches locally.


* Wajda, Piotr  [160916 22:48]:

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)








From f0ccb77460d4bd41b45de7d2ddb54294b91e9e3b Mon Sep 17 00:00:00 2001
From: ja 
Date: Sun, 18 Sep 2016 18:47:37 +0200
Subject: [PATCH] Don't print to stdout in background.

---
src/main.c |   15 +--
1 file changed, 13 insertions(+), 2 deletions(-)

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
-- 
1.7.9.5



--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6


signature.asc
Description: PGP signature

Re: [Bug-wget] bug #45790: wget prints it's progress even when background

[Darshit Shah]

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  [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  [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







--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6


signature.asc
Description: PGP signature

Re: [Bug-wget] bug #45790: wget prints it's progress even when background

[pwa...@gmail.net.pl]

Ok, I'll fix that.

Thanks
Piotr

W dniu 19.09.2016 o 19:30, Darshit Shah pisze:

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  [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  [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

Re: [Bug-wget] bug #46584: wget --spider always returns zero exit status

[Darshit Shah]

Apart from the patch format, the patch itself looks good.

@Giuseppe: We will require Copyright assignments for Piotr right? This 
patch may be small, but there are a couple others in the pipeline.


* Wajda, Piotr  [160916 22:48]:

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)



--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6


signature.asc
Description: PGP signature

Re: [Bug-wget] bug #46584: wget --spider always returns zero exit status

[pwa...@gmail.net.pl]

I'm not yet fully familiar with git format-patch (weird for me that it's 
adding email-like headers. Is it suppose to be email creation tool for 
patches?), I believe it will work for you.


Thanks
Piotr

W dniu 19.09.2016 o 18:56, Darshit Shah pisze:

Hi Piotr,

How did you create this patch? Because git refuses to accept it.
Patch format detection fails. Please regenerate all your patches using 
`git format-patch` so that we can apply the patches locally.


* Wajda, Piotr  [160916 22:48]:

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)





>From f0ccb77460d4bd41b45de7d2ddb54294b91e9e3b Mon Sep 17 00:00:00 2001
From: ja 
Date: Sun, 18 Sep 2016 18:47:37 +0200
Subject: [PATCH] Don't print to stdout in background.

---
 src/main.c |   15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

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
-- 
1.7.9.5

Re: [Bug-wget] bug #46584: wget --spider always returns zero exit status

[Darshit Shah]

Hi Piotr,

How did you create this patch? Because git refuses to accept it.
Patch format detection fails. Please regenerate all your patches using 
`git format-patch` so that we can apply the patches locally.


* Wajda, Piotr  [160916 22:48]:

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)



--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6


signature.asc
Description: PGP signature

Re: [Bug-wget] bug #45790: wget prints it's progress even when background

[pwa...@gmail.net.pl]

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  [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

Re: [Bug-wget] bug #45790: wget prints it's progress even when background

[Darshit Shah]

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  [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



--
Thanking You,
Darshit Shah
PGP Fingerprint: 7845 120B 07CB D8D6 ECE5 FF2B 2A17 43ED A91A 35B6


signature.asc
Description: PGP signature

Re: [Bug-wget] [bug #47408] Wget sends malformed SNI host names

[Daniel Stenberg]

On Wed, 16 Mar 2016, Tim Ruehsen wrote:

Here is a patch for both openssl and gnutls. Please comment, I'll push it 
tomorrow.


The bug report says the SNI field should be different than the Host: header, 
but I question the sensibility in that. What would be the point? (pun not 
intended =B))


When requesting contents from an HTTPS site, the SNI field will tell the 
server which particular virtual server to get the data from and when the 
trailing dot gets stripped the two strings with and without dot will end up on 
the same virtual server. Sending a Host: header that doesn't match the virtual 
server name then is then likely to either get ignored or to cause the HTTP 
backend to complain.


It will also make it behave a bit different for HTTP than for HTTPS since then 
there's no SNI field and the Host: header is what will be used and then they 
clearly are different servers.


And incidentally, curl strips the trailing dot off from both SNI and Host: =)

--

 / daniel.haxx.se

Re: [Bug-wget] [bug #47408] Wget sends malformed SNI host names

[Tim Ruehsen]

Great. Thanks !

Tim

On Wednesday 16 March 2016 13:06:23 Daniel Stenberg wrote:
> On Wed, 16 Mar 2016, Tim Ruehsen wrote:
> > Should we follow the browsers or curl ?
> 
> I brought this subject to the http-wg mailing list, possibly we can clear it
> up on a wider scale:
> 
>   https://lists.w3.org/Archives/Public/ietf-http-wg/2016JanMar/0430.html

Re: [Bug-wget] [bug #47408] Wget sends malformed SNI host names

[Tim Ruehsen]

On Wednesday 16 March 2016 11:59:04 Daniel Stenberg wrote:
> On Wed, 16 Mar 2016, Tim Ruehsen wrote:
> > Here is a patch for both openssl and gnutls. Please comment, I'll push it
> > tomorrow.
> 
> The bug report says the SNI field should be different than the Host: header,
> but I question the sensibility in that. What would be the point? (pun not
> intended =B))
> 
> When requesting contents from an HTTPS site, the SNI field will tell the
> server which particular virtual server to get the data from and when the
> trailing dot gets stripped the two strings with and without dot will end up
> on the same virtual server. Sending a Host: header that doesn't match the
> virtual server name then is then likely to either get ignored or to cause
> the HTTP backend to complain.
> 
> It will also make it behave a bit different for HTTP than for HTTPS since
> then there's no SNI field and the Host: header is what will be used and
> then they clearly are different servers.
> 
> And incidentally, curl strips the trailing dot off from both SNI and Host:
> =)

That is what I would like to do as well. It seems consistent. And the patch 
introduces non-elegant code (not really my favor).

And for DNS lookups... is there a difference between dot and not-dot (e.g. 
example.com vs. example.com.) ?

The patch follows Yst Dawson's conclusion, though:
"That means that the SNI host name and HTTP Host header do not always match.
The SNI host name must never have a trailing dot, but the HTTP Host header
must reflect a host name that is identical to the host name of the URI, so if
the URI's host has a trailing dot, the HTTP Host header must include that
trailing dot."

Also what Jay Satiro says:
"I tried this in Firefox, Chrome and IE and all send the trailing dot for 
SNI."

Should we follow the browsers or curl ?

Tim

Re: [Bug-wget] [bug #47408] Wget sends malformed SNI host names

[Daniel Stenberg]

On Wed, 16 Mar 2016, Tim Ruehsen wrote:


Should we follow the browsers or curl ?


I brought this subject to the http-wg mailing list, possibly we can clear it 
up on a wider scale:


 https://lists.w3.org/Archives/Public/ietf-http-wg/2016JanMar/0430.html

--

 / daniel.haxx.se

Re: [Bug-wget] [bug #47408] Wget sends malformed SNI host names

[Tim Ruehsen]

Here is a patch for both openssl and gnutls.
Please comment, I'll push it tomorrow.

BTW, when fixing the gnutls code I stumbled upon a bug in 3.4.x.
I reported it as https://gitlab.com/gnutls/gnutls/issues/78

Tim

On Monday 14 March 2016 17:21:15 Yst Dawson wrote:
> URL:
>   
>
>  Summary: Wget sends malformed SNI host names
>  Project: GNU Wget
> Submitted by: yst
> Submitted on: Mon 14 Mar 2016 05:21:14 PM GMT
> Category: Program Logic
> Severity: 3 - Normal
> Priority: 5 - Normal
>   Status: None
>  Privacy: Public
>  Assigned to: None
>  Originator Name:
> Originator Email:
>  Open/Closed: Open
>  Discussion Lock: Any
>  Release: 1.16
> Operating System: GNU/Linux
>  Reproducibility: Every Time
>Fixed Release: None
>  Planned Release: None
>   Regression: None
>Work Required: None
>   Patch Included: None
>
> ___
>
> Details:
>
> To quote a couple specifications:
>  (SNI)
>   "HostName" contains the fully qualified DNS hostname of the server,
>   as understood by the client.  The hostname is represented as a byte
>   string using ASCII encoding without a trailing dot.
>
>  (HTTP)
>   A client MUST send a Host header field in all HTTP/1.1 request
>   messages.  If the target URI includes an authority component, then a
>   client MUST send a field-value for Host that is identical to that
>   authority component, excluding any userinfo subcomponent and its "@"
>   delimiter (Section 2.7.1).
>
> That means that the SNI host name and HTTP Host header do not always match.
> The SNI host name must never have a trailing dot, but the HTTP Host header
> must reflect a host name that is identical to the host name of the URI, so
> if the URI's host has a trailing dot, the HTTP Host header must include
> that trailing dot.
>
> For example, if the URI of a page is , the
> following values should be sent by the Web browser:
> SNI host: alice.sni.velox.ch
> HTTP host: alice.sni.velox.ch.
>
> However, Wget sends "alice.sni.velox.ch." as the SNI host name. In some
> cases, malformed SNI host names can cause the server to throw an error, an
> example of which can be seen at  or
> .
>
> Other information:
>  * version: 1.16
>  * invoked by running "wget --no-check-certificate
> https://alice.sni.velox.ch./;
>  * expected result: Wget should send an SNI host name that conforms to RFC
> 6066 or no SNI host name, while still sending an HTTP Host header that
> includes the trailing dot, as per RFC 7230.
>  * actual result: Wget sent a malformed SNI host name
>  * The output, in case relevant, has been attached as a file upload.
>
>
>
> ___
>
> File Attachments:
>
>
> ---
> Date: Mon 14 Mar 2016 05:21:14 PM GMT  Name: index.html  Size: 5kB   By: yst
>
> 
>
> ___
>
> Reply to this item at:
>
>   
>
> ___
>   Message sent via/by Savannah
>   http://savannah.gnu.org/From d7726f8a1366efcd09329ee20beefb7e8ece9078 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tim Rühsen?= 
Date: Wed, 16 Mar 2016 11:23:51 +0100
Subject: [PATCH] Fix SNI server names with trailing dot(s)

* src/gnutls.c (ssl_connect_wget, ssl_check_certificate): Fix SNI server name
* src/openssl.c (ssl_connect_wget, ssl_check_certificate): Fix SNI server name

Fixes #47408
---
 src/gnutls.c  | 32 
 src/openssl.c | 35 +++
 2 files changed, 59 insertions(+), 8 deletions(-)

diff --git a/src/gnutls.c b/src/gnutls.c
index d39371f..3e1596a 100644
--- a/src/gnutls.c
+++ b/src/gnutls.c
@@ -36,6 +36,7 @@ as that of the covered work.  */
 #include 
 #include 
 #include 
+#include 

 #include 
 #include 
@@ -518,6 +519,22 @@ _do_handshake (gnutls_session_t session, int fd, double timeout)
   return err;
 }

+static const char *
+_sni_hostname(const char *hostname)
+{
+  size_t len = strlen(hostname);
+
+  char *sni_hostname = xmemdup(hostname, len + 1);
+
+  /* Remove trailing dot(s) to fix #47408.
+   * Regarding RFC 6066 (SNI): The hostname is represented as a byte
+   * string using ASCII encoding without a trailing dot. */
+  while (len && sni_hostname[--len] == '.')
+sni_hostname[len] = 0;
+
+  return sni_hostname;
+}
+
 bool
 

Re: [Bug-wget] [bug #47408] Wget sends malformed SNI host names

[Daniel Stenberg]

On Wed, 16 Mar 2016, Jay Satiro wrote:

I tried this in Firefox, Chrome and IE and all send the trailing dot for 
SNI. curl doesn't though, it strips the trailing dot and also it won't 
appear in the host header.


And the associated Firefox bug report:

   https://bugzilla.mozilla.org/show_bug.cgi?id=1008120

--

 / daniel.haxx.se

Re: [Bug-wget] [bug #43799] wget should implement OCSP + OCSP stapling

[Tim Ruehsen]

On Wednesday 19 August 2015 18:19:16 Petr Pisar wrote:
 On Wed, Aug 19, 2015 at 03:37:06PM +, Tim Ruehsen wrote:
  Regarding MITM and other attacks... did you notice that OCSP responder
  URLs
  are HTTP (plain text) will all the insecurity ? I never saw a HTTPS URL,
  did you ?

 
 There is no need for HTTPS. The OCSP response is signed by the CA's OCSP
 responder. So the problem of OCSP response integrity reduces to verifying
 the OCSP response signature. Of course to verify the signature, one needs
 to verify OCSP responder's certificate. But this is the same story as with
 CRLs.

A signature makes it possible to verify the delivered content (answer from 
OCSP responder here), and - as you say - is as trustworthy as the CA certs you 
have.
But there also is a privacy concern involved when using plain text 
communication. The OCSP request data holds unique parts of the server 
certificate(s). Anyone (between me and the OCSP responder) can see and assign 
my IP with the domain (or at least with the server IP) that I want to access. 
OCSP stapling is a big win here, but is potential useless if the server has a 
cert chain. That's why I want to see OCSP multi-stapling in the near future 
(RFC6961 TLS Multiple Certificate Status Request Extension). That's also a big 
HTTPS connect speedup. AFAIK, there is no TLS library currently supporting it.

Tim


signature.asc
Description: This is a digitally signed message part.

Re: [Bug-wget] [bug #45790] wget prints it's progress even when background

[Giuseppe Scrivano]

Darshit Shah dar...@gmail.com writes:

 This affects an invokation using the shell's background operator () too.

 E.g.: wget 
 http://cdimage.debian.org/debian-cd/current/multi-arch/iso-cd/debian-8.1.0-amd64-i386-netinst.iso
 
 will cause the logging output and progress bar to be displayed on the
 terminal as explained in the bug report.

 However, I am not willing to fix that behaviour. A huge number of
 people copy URLs and paste them in their terminals for Wget to
 download without double-quoting them. A large number of these URLs
 have the  character which causes the shell to background the
 process. They tend to realise that something went wrong when the
 screen is garbled by a background process spewing messages to stdout
 and stderr. If this behaviour is changed, many people won't realise
 their error and un-necessarily invoke multiple instances of
 backgrounded Wget processes, eventually coming back here with new bug
 reports.

 The bahviour has remained so for a long time and I'm inclined to
 retain the status quo.

I agree with you here, the behavior should not be changed.  It would be
a bug if wget stops reporting errors when resumed.

Regards,
Giuseppe

Re: [Bug-wget] [bug #45790] wget prints it's progress even when background

[Darshit Shah]

This affects an invokation using the shell's background operator () too.

E.g.: wget 
http://cdimage.debian.org/debian-cd/current/multi-arch/iso-cd/debian-8.1.0-amd64-i386-netinst.iso

will cause the logging output and progress bar to be displayed on the
terminal as explained in the bug report.

However, I am not willing to fix that behaviour. A huge number of
people copy URLs and paste them in their terminals for Wget to
download without double-quoting them. A large number of these URLs
have the  character which causes the shell to background the
process. They tend to realise that something went wrong when the
screen is garbled by a background process spewing messages to stdout
and stderr. If this behaviour is changed, many people won't realise
their error and un-necessarily invoke multiple instances of
backgrounded Wget processes, eventually coming back here with new bug
reports.

The bahviour has remained so for a long time and I'm inclined to
retain the status quo.

On Tue, Aug 18, 2015 at 2:33 PM, NoëlKöthe invalid.nore...@gnu.org wrote:
 URL:
   http://savannah.gnu.org/bugs/?45790

  Summary: wget prints it's progress even when background
  Project: GNU Wget
 Submitted by: nok
 Submitted on: Di 18 Aug 2015 11:03:31 CEST
 Category: User Interface
 Severity: 3 - Normal
 Priority: 5 - Normal
   Status: None
  Privacy: Public
  Assigned to: None
  Originator Name:
 Originator Email:
  Open/Closed: Open
  Discussion Lock: Any
  Release: None
 Operating System: None
  Reproducibility: None
Fixed Release: None
  Planned Release: None
   Regression: None
Work Required: None
   Patch Included: None

 ___

 Details:

 Hello,

 an old forgotten bug report:

 --8--
 When wget is suspended in command line and then
 send into background (eg using bash bg command), it continues
 to print it's progress messages. This leads to either stopping
 wget or to garbling terminal with wget messages (depending
 on the TOSTOP terminal setting).
 --8--
 My suggestion is to stop printing verbose progress messages
 when the job is resumed in background. It could be checked
 by (successful) getpgrp() not equal to (successful) tcgetprp(1)
 in SIGCONT signal handler.
  And something like this is used in some console applications,
 for example, in lftp.
 --8--
 https://bugs.debian.org/281201

 As an example:

 # wget
 http://cdimage.debian.org/debian-cd/current/multi-arch/iso-cd/debian-8.1.0-amd64-i386-netinst.iso
 Press ctrl+z
 [1]+  Stopped  wget
 http://cdimage.debian.org/debian-cd/current/multi-arch/iso-cd/debian-8.1.0-amd64-i386-netinst.iso
 nk@pro:/tmp/test$ bg
 garbling the terminal but commands work as expected

 Regards

 Noël




 ___

 Reply to this item at:

   http://savannah.gnu.org/bugs/?45790

 ___
   Nachricht gesendet von/durch Savannah
   http://savannah.gnu.org/




-- 
Thanking You,
Darshit Shah

Re: [Bug-wget] [bug #45778] wget and unicode

[Tim Ruehsen]

There is a current patch in discussion.

tim@blitz-lx:~/src/wget$ src/wget -nv 
http://zh.wikipedia.org/wiki/%E9%A6%96%E9%A1%B5
2015-08-17 14:47:52 URL:http://zh.wikipedia.org/wiki/%E9%A6%96%E9%A1%B5 
[36245] - 首页 [1]

Tim


signature.asc
Description: This is a digitally signed message part.

Re: [Bug-wget] [bug #45037] wget -O foo ftp://ftp.funet.fi/pub/Linux/mirrors/debian/README deletes README symlink

[Ángel González]

On 07/05/15 06:10, Darshit Shah wrote:
Given that this is a clearly documented feature of Wget, that -O is 
supposed to work as shell redirection, I think it should not pose too 
many issues. Yes, there is the security risk where another file may be 
overwritten, but I think the user should be careful about those.
I was thinking in a recursive download creating a symlink and then 
following it, using the same code path. If the user explicitely 
requested wget -O symlink, it's the user responsability.

Re: [Bug-wget] [bug #45037] wget -O foo ftp://ftp.funet.fi/pub/Linux/mirrors/debian/README deletes README symlink

[Darshit Shah]

Aah! It was a quick patch I wrote and had tested only one scenario.
Which is why I wanted some reviews before pushing it.

Seems like it'll take slightly more effort to fix this. I'll take a
look again today evening.

On Thu, May 7, 2015 at 1:04 PM, Tim Ruehsen tim.rueh...@gmx.de wrote:
 Hi Darshit,

 your patch doesn't work as expected.

 $ cat abc xxx
 $ ln -s xxx foo
 $ ln -s /etc/passwd README
 $ ../src/wget -O foo ftp://ftp.funet.fi/pub/Linux/mirrors/debian/README
 ...

 $ ls -la
 lrwxrwxrwx 1 oms users   11 May  7 09:28 README - /etc/passwd
 -rw-r--r-- 1 oms users 1495 May  7 09:30 xxx

 'foo' doesn't exist and 'xxx' has been overwritten !!!

 Regards, Tim


 On Thursday 07 May 2015 10:06:55 Darshit Shah wrote:
 joey@darkstar:~/tmp/yln -s /etc/passwd README
 joey@darkstar:~/tmp/yls
 README@
 joey@darkstar:~/tmp/ywget -O foo
 ftp://ftp.funet.fi/pub/Linux/mirrors/debian/README
 --2015-05-05 13:17:22--  ftp://ftp.funet.fi/pub/Linux/mirrors/debian/README
 
= 'foo'
 
 Resolving ftp.funet.fi (ftp.funet.fi)... 193.166.3.2, 2001:708:10:9::20:2
 Connecting to ftp.funet.fi (ftp.funet.fi)|193.166.3.2|:21... connected.
 Logging in as anonymous ... Logged in!
 == SYST ... done.== PWD ... done.
 == TYPE I ... done.  == CWD (1) /pub/Linux/mirrors/debian ... done.
 == SIZE README ... 1495
 == PASV ... done.== RETR README ... done.
 Length: 1495 (1.5K) (unauthoritative)
 
 README  100%[=]   1.46K  5.45KB/s   in
 0.3s
 
 
 2015-05-05 13:17:28 (5.45 KB/s) - 'foo' saved [1495]
 
 joey@darkstar:~/tmp/yls
 foo
 
 Doesn't happen if README is a file rather than a symlink, doesn't happen
 when using http. The ftp downloader apparently has a bug..

 This definitely is a bug. I've attached a small fix for this particular
 issue. If no one has any issues with it, I'll push it in a day.

 Although, I think we should also look into the issue Giuseppe found, and
 also the point that Angel raised about compatibility with shell like
 redirection.



-- 
Thanking You,
Darshit Shah

Re: [Bug-wget] [bug #45037] wget -O foo ftp://ftp.funet.fi/pub/Linux/mirrors/debian/README deletes README symlink

[Tim Ruehsen]

Hi Darshit,

your patch doesn't work as expected.

$ cat abc xxx
$ ln -s xxx foo
$ ln -s /etc/passwd README
$ ../src/wget -O foo ftp://ftp.funet.fi/pub/Linux/mirrors/debian/README
...

$ ls -la
lrwxrwxrwx 1 oms users   11 May  7 09:28 README - /etc/passwd
-rw-r--r-- 1 oms users 1495 May  7 09:30 xxx

'foo' doesn't exist and 'xxx' has been overwritten !!!

Regards, Tim


On Thursday 07 May 2015 10:06:55 Darshit Shah wrote:
 joey@darkstar:~/tmp/yln -s /etc/passwd README
 joey@darkstar:~/tmp/yls
 README@
 joey@darkstar:~/tmp/ywget -O foo
 ftp://ftp.funet.fi/pub/Linux/mirrors/debian/README
 --2015-05-05 13:17:22--  ftp://ftp.funet.fi/pub/Linux/mirrors/debian/README
 
= ‘foo’
 
 Resolving ftp.funet.fi (ftp.funet.fi)... 193.166.3.2, 2001:708:10:9::20:2
 Connecting to ftp.funet.fi (ftp.funet.fi)|193.166.3.2|:21... connected.
 Logging in as anonymous ... Logged in!
 == SYST ... done.== PWD ... done.
 == TYPE I ... done.  == CWD (1) /pub/Linux/mirrors/debian ... done.
 == SIZE README ... 1495
 == PASV ... done.== RETR README ... done.
 Length: 1495 (1.5K) (unauthoritative)
 
 README  100%[=]   1.46K  5.45KB/s   in
 0.3s
 
 
 2015-05-05 13:17:28 (5.45 KB/s) - ‘foo’ saved [1495]
 
 joey@darkstar:~/tmp/yls
 foo
 
 Doesn't happen if README is a file rather than a symlink, doesn't happen
 when using http. The ftp downloader apparently has a bug..
 
 This definitely is a bug. I've attached a small fix for this particular
 issue. If no one has any issues with it, I'll push it in a day.
 
 Although, I think we should also look into the issue Giuseppe found, and
 also the point that Angel raised about compatibility with shell like
 redirection.


signature.asc
Description: This is a digitally signed message part.

Re: [Bug-wget] [bug #45037] wget -O foo ftp://ftp.funet.fi/pub/Linux/mirrors/debian/README deletes README symlink

[Darshit Shah]

On 05/06, Ángel wrote:

Follow-up Comment #3, bug #45037 (project wget):

It's clearly a bug. If we are downloading README to local file foo, there's no
reason to remove the local symlink README.


I agree. This is definitely a bug.


When we want to save into a name where there's currently a symlink,
consistency with shell  would dictate just to follow the link on writing, but
as that may lead to security issues, replacing with a file may also be
acceptable.

My guess is, we should default to shell like behaviour and provide a command 
line option to instead replace the file, if the user wishes so.


Given that this is a clearly documented feature of Wget, that -O is supposed to 
work as shell redirection, I think it should not pose too many issues. Yes, 
there is the security risk where another file may be overwritten, but I think 
the user should be careful about those.


   ___

Reply to this item at:

 http://savannah.gnu.org/bugs/?45037

___
 Mensaje enviado vía/por Savannah
 http://savannah.gnu.org/



--
Thanking You,
Darshit Shah


pgpIYWXEEy9Gs.pgp
Description: PGP signature

Re: [Bug-wget] [bug #45037] wget -O foo ftp://ftp.funet.fi/pub/Linux/mirrors/debian/README deletes README symlink

[Darshit Shah]

joey@darkstar:~/tmp/yln -s /etc/passwd README
joey@darkstar:~/tmp/yls
README@
joey@darkstar:~/tmp/ywget -O foo
ftp://ftp.funet.fi/pub/Linux/mirrors/debian/README
--2015-05-05 13:17:22--  ftp://ftp.funet.fi/pub/Linux/mirrors/debian/README
  = ‘foo’
Resolving ftp.funet.fi (ftp.funet.fi)... 193.166.3.2, 2001:708:10:9::20:2
Connecting to ftp.funet.fi (ftp.funet.fi)|193.166.3.2|:21... connected.
Logging in as anonymous ... Logged in!
== SYST ... done.== PWD ... done.
== TYPE I ... done.  == CWD (1) /pub/Linux/mirrors/debian ... done.
== SIZE README ... 1495
== PASV ... done.== RETR README ... done.
Length: 1495 (1.5K) (unauthoritative)

README  100%[=]   1.46K  5.45KB/s   in 0.3s


2015-05-05 13:17:28 (5.45 KB/s) - ‘foo’ saved [1495]

joey@darkstar:~/tmp/yls
foo

Doesn't happen if README is a file rather than a symlink, doesn't happen when
using http. The ftp downloader apparently has a bug..


This definitely is a bug. I've attached a small fix for this particular issue. 
If no one has any issues with it, I'll push it in a day.


Although, I think we should also look into the issue Giuseppe found, and also 
the point that Angel raised about compatibility with shell like redirection.


--
Thanking You,
Darshit Shah
From 7537c7e341948ee6e0ff5170a5a056d80a82cfde Mon Sep 17 00:00:00 2001
From: Darshit Shah dar...@reniac.com
Date: Thu, 7 May 2015 09:59:25 +0530
Subject: [PATCH] Fix #45037: Wget deletes wrong Symlink in ftp

* ftp.c (ftp_loop_internal): Call remove_link() on the actual file
name to be retrieved, not the filename in .LISTING.

Reported-by: Joey Hess i...@joeyh.name
 Noel Koethe n...@debian.org
---
 src/ftp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/ftp.c b/src/ftp.c
index 68f1a33..0abd543 100644
--- a/src/ftp.c
+++ b/src/ftp.c
@@ -1589,7 +1589,7 @@ ftp_loop_internal (struct url *u, struct fileinfo *f, ccon *con, char **local_fi
 }
 
   /* Remove it if it's a link.  */
-  remove_link (con-target);
+  remove_link (locf);
 
   count = 0;
 
-- 
2.3.7



pgpZazmMNNHam.pgp
Description: PGP signature

Re: [Bug-wget] [Bug-Wget] Wget closes connection despite Keep-Alive

[Darshit Shah]

On Wed, Nov 5, 2014 at 3:50 PM, Giuseppe Scrivano gscriv...@gnu.org wrote:
 Hi Darshit,

 Darshit Shah dar...@gmail.com writes:

 * Off-topic:
 Looking at the condition, there's one comparison that we can avoid:
 hs-restval  0
 contlen = 0
 hs-restval = contlen

 In this case, hs-restval  0 is redundant and un-needed.

 good catch, we can surely drop the first comparison.

Pushed

 The reason why I came across this was that I needed Wget to maintain a
 persistent connection, and that was exactly what the server
 expected. However, Wget went ahead and closed the connection. This
 *missing feature* was precisely what I was looking for.

 Also, this behaviour will become a bug with SPDY and HTTP/2 since the
 server will try to use the open connection to suggest other resources
 which the client may ask for.


 I'm waiting for Giuseppe's views on this. Else, I'll split the
 condition into two parts, with the first one called CLOSE_FINISH
 instead of CLOSE_INVALIDATE

 feel free to split it and leave the connection open on a
 HTTP_STATUS_RANGE_NOT_SATISFIABLE.

Pushed

 Regards,
 Giuseppe



-- 
Thanking You,
Darshit Shah

Re: [Bug-wget] [Bug-Wget] Wget closes connection despite Keep-Alive

[Darshit Shah]

On 11/04, Tim Rühsen wrote:

Am Dienstag, 4. November 2014, 23:25:40 schrieb Darshit Shah:

While looking at some debug output from Wget, I noticed that in case of a
416 Range Not Satisfiable response, Wget forces the connection close
despite the fact that the server explicitly sent a `Connection: Keep-Alive`
header.

Looking at the code, I found this at http.c:2775


  CLOSE_INVALIDATE (sock);/* would be CLOSE_FINISH, but there
   might be more bytes in the body. */

I'm not sure what exactly we're trying to protect in this case. Why is
CLOSE_INVALIDATE required instead of CLOSE_FINISH? If the server responds
with a 416 status, there should be no more data in the stream and we should
be able to reuse that connection without any issues.

What am I missing here?


The code is with an if construct with || :
if (statcode == HTTP_STATUS_RANGE_NOT_SATISFIABLE
 || (!opt.timestamping  hs-restval  0  statcode == HTTP_STATUS_OK
  contrange == 0  contlen = 0  hs-restval = contlen))
   {

So if statcode == HTTP_STATUS_RANGE_NOT_SATISFIABLE i guess we could use
CLOSE_FINISH. But I can't judge that for the other part of the expression.
If in doubt, CLOSE_INVALIDATE might be a better choice than CLOSE_FINISH.
Wget simply opens a new connection for the next request in this case.

I'm pretty sure that in case of `statcode == HTTP_STATUS_RANGE_NOT_SATISFIABLE` 
we can use CLOSE_FINISH(). The other condition is what I am confused about.


* Off-topic:
Looking at the condition, there's one comparison that we can avoid:
hs-restval  0
contlen = 0
hs-restval = contlen

In this case, hs-restval  0 is redundant and un-needed.






I wanted to write a test case to demonstrate this, but detecting a closed
socket will take more than a quick test case. I'll sit down and write the
required functionality to generate the relevant test case when I can.


Don't spend too much time here. It is not a bug, more of a missing feature.
And also a very seldom used one. And how I read the comment, servers might
behave buggy. And again, in that case it seems more reliable when Wget closes
the connection and opens a new one.

The reason why I came across this was that I needed Wget to maintain a 
persistent connection, and that was exactly what the server expected. However, 
Wget went ahead and closed the connection. This *missing feature* was precisely 
what I was looking for.


Also, this behaviour will become a bug with SPDY and HTTP/2 since the server 
will try to use the open connection to suggest other resources which the client 
may ask for.



I'm waiting for Giuseppe's views on this. Else, I'll split the condition into 
two parts, with the first one called CLOSE_FINISH instead of CLOSE_INVALIDATE



--
Thanking You,
Darshit Shah


pgpZztcuk8cLV.pgp
Description: PGP signature

Re: [Bug-wget] [Bug-Wget] Wget closes connection despite Keep-Alive

[Tim Ruehsen]

On Wednesday 05 November 2014 13:49:56 Darshit Shah wrote:
 * Off-topic:
 Looking at the condition, there's one comparison that we can avoid:
 hs-restval  0
 contlen = 0
 hs-restval = contlen
 
 In this case, hs-restval  0 is redundant and un-needed.

ACK.

 I'm waiting for Giuseppe's views on this. Else, I'll split the condition
 into two parts, with the first one called CLOSE_FINISH instead of
 CLOSE_INVALIDATE

Splitting sounds reasonable to me.

Tim


signature.asc
Description: This is a digitally signed message part.

Re: [Bug-wget] [Bug-Wget] Wget closes connection despite Keep-Alive

[Tim Rühsen]

Am Dienstag, 4. November 2014, 23:25:40 schrieb Darshit Shah:
 While looking at some debug output from Wget, I noticed that in case of a
 416 Range Not Satisfiable response, Wget forces the connection close
 despite the fact that the server explicitly sent a `Connection: Keep-Alive`
 header.
 
 Looking at the code, I found this at http.c:2775
 
 
   CLOSE_INVALIDATE (sock);/* would be CLOSE_FINISH, but there
might be more bytes in the body. */
 
 I'm not sure what exactly we're trying to protect in this case. Why is
 CLOSE_INVALIDATE required instead of CLOSE_FINISH? If the server responds
 with a 416 status, there should be no more data in the stream and we should
 be able to reuse that connection without any issues.
 
 What am I missing here?

The code is with an if construct with || :
if (statcode == HTTP_STATUS_RANGE_NOT_SATISFIABLE
  || (!opt.timestamping  hs-restval  0  statcode == HTTP_STATUS_OK
   contrange == 0  contlen = 0  hs-restval = contlen))
{

So if statcode == HTTP_STATUS_RANGE_NOT_SATISFIABLE i guess we could use 
CLOSE_FINISH. But I can't judge that for the other part of the expression.
If in doubt, CLOSE_INVALIDATE might be a better choice than CLOSE_FINISH.
Wget simply opens a new connection for the next request in this case.


 I wanted to write a test case to demonstrate this, but detecting a closed
 socket will take more than a quick test case. I'll sit down and write the
 required functionality to generate the relevant test case when I can.

Don't spend too much time here. It is not a bug, more of a missing feature. 
And also a very seldom used one. And how I read the comment, servers might 
behave buggy. And again, in that case it seems more reliable when Wget closes 
the connection and opens a new one.


Tim


signature.asc
Description: This is a digitally signed message part.

Re: [Bug-wget] Bug in WGET?

[Giuseppe Scrivano]

Patrick Steil patr...@churchbuzz.org writes:

 Also, if I use wget in spider mode, it will at the end of the log
 output tell me about all the broken links... but I also need to know
 what page those broken links are created on (if the broken link) is on
 the site I am getting... this will help me find the 404 on my site... 

 I have a vision for how this should work to make it awesome... 

 Any way to do that, or anyone want to add this functionality?

I don't think it is possible at the moment, but add this feature
shouldn't take much time.

The feature seems interesting but I don't think it is going to be
implemented before the next release.

You can wait until someone is going to implement it, or you can take
advantage of the fact wget is Free software and implement it by yourself
or hire someone to do it for you.

Cheers,
Giuseppe

Re: [Bug-wget] Bug in WGET?

[Giuseppe Scrivano]

Hello,

Patrick Steil patr...@churchbuzz.org writes:

 If I run this command:

 wget www.domain.org/news?page=1 options= -r --no-clobber --html-extension
 --convert-links -np --include-directories=news

 Here is what it does today:

 1.  When --html-extension is turned on, the --noclobber is not changing the
 name of the downloaded files, but it DOES rewrite the file as the date/time
 stamp changes every time I run the above command.

I couldn't reproduce it.  I have `strace'd but I can't see any syscall
which could modify the time stamp.  Can you please attach the strace
and the wget debug log?  You can get it by:

strace -o strace.log wget args -d -o wget.log



 2.  If I turn off --html-extension, then as soon as WGET sees that the first
 file has already been downloaded it stops and does not continue to
 spider/download any further pages.

AFAICS, the behaviour you get using --no-clobber and -r is documented,
and it should work exactly as you described it (a newer version is
ignored).  The old version is still traversed for links.

Cheers,
Giuseppe