Re: [Bug-wget] [PATCH 10/14] Do not depend on always defined macros

[Darshit Shah]

On Thu, Jun 12, 2014 at 8:59 PM, Steven M. Schweda  wrote:
> From: Giuseppe Scrivano 
>
>> ops, thanks to have catched it.  I've fixed it in my local copy, this is
>> how it looks now:
>>
>> diff --git a/vms/vms.h b/vms/vms.h
>> index d65aeda..6053df1 100644
>> --- a/vms/vms.h
>> +++ b/vms/vms.h
>> @@ -49,16 +49,6 @@ int utime( const char *path, const struct utimbuf *times);
>
>There was also that comment near the beginning:
>
>> ALP $ gdiff wget-1_14/vms/vms.h_orig wget-1_14/vms/vms.h
>> 10,12d9
>> <  *  Emergency substitution of stat() for lstat() for VAX and VMS CRTL
>> <  *  before V7.3-1.
>> <  *
>
>It might make sense to fetch one of my 1.15 source kits, and see what
> else has changed since 1.12.  (Probably many things.)
>
>I don't know how much sense it makes to include VMS-specific files in
> the official sources.  Without a VMS system, you really can't
> maintain/test them, and I usually don't do anything until a real release
> occurs, so it's hard for you to do a release with good VMS stuff in it.
> And, recent "tar" kits have not included any VMS-specific files, anyway.
>
I'm not sure about the release tarballs, but if we're not shipping the
VMS files in the tarballs, then maybe the git sources shouldn't carry
them either.

Anyways, as Steven states we are unable to test/maintain any of the
VMS specific code, then keeping build documentation doesn't help
either.
>SMS.
>



-- 
Thanking You,
Darshit Shah

Re: [Bug-wget] [PATCH 02/14] Do not use exit() with a magic number

[Giuseppe Scrivano]

"Gisle Vanem"  writes:

>  wrote:
>
> ...
>
>> diff --git a/src/mswindows.c b/src/mswindows.c
>> index 179773e..c0d9be6 100644
>> --- a/src/mswindows.c
>> +++ b/src/mswindows.c
>> @@ -308,7 +308,7 @@ cleanup:
>>
>>   /* We're the parent.  If all is well, terminate.  */
>>   if (rv)
>> -exit (0);
>> +exit (WGET_EXIT_SUCCESS);
>
> This breaks compilation of mswindows.c as I wrote to Guiseppe
> privately; add "exits.h" at top.

thanks, I've fixed locally the source file, as you reported.

Regards,
Giuseppe

Re: [Bug-wget] [bug-wget] Libpsl for cookie domain checking in Wget

[Darshit Shah]

That patch works perfectly for me.

Maybe we should update the rest of the file along the same lines? It
would help to reduce some fragmentation.

On Thu, Jun 12, 2014 at 8:53 PM, Tim Rühsen  wrote:
> Am Donnerstag, 12. Juni 2014, 13:24:02 schrieb Giuseppe Scrivano:
>> Darshit Shah  writes:
>> > On Wed, Jun 11, 2014 at 5:20 PM, Tim Rühsen  wrote:
>> >> Am Freitag, 6. Juni 2014, 13:39:32 schrieb Darshit Shah:
>> >>> I'm facing an issue with the patch I submitted for libpsl and would be
>> >>> glad if someone could help me.
>> >>>
>> >>> The configure.ac file does not work as expected. When libpsl is not
>> >>> installed on a system, the LDFLAGS does not contain -lpsl flag, but
>> >>> the configure summary shows LIBPSL: Yes.
>> >>>
>> >>> There is some discrepency in the output that I'd like to fix. The
>> >>> build completes successfully because the HAVE_LIBPSL variable isn't
>> >>> set, and Wget compiles without libpsl support. This should however
>> >>> happen only when --without-libpsl was explicitly specified as a
>> >>> configure option.
>> >>
>> >> This should do it:
>> >>
>> >> AC_ARG_WITH(libpsl,
>> >>
>> >> AS_HELP_STRING([--without-libpsl], [disable support for libpsl cookie
>> >> checking.]), [
>> >>
>> >>   with_libpsl=no
>> >>
>> >> ], [
>> >>
>> >>   AC_CHECK_LIB(psl, psl_builtin,
>> >>
>> >>[with_libpsl=yes; AC_DEFINE([WITH_LIBPSL], [1], [PSL
>> >>support enabled]) LIBS="${LIBS} -lpsl"],
>> >>[with_libpsl=no; AC_MSG_WARN(*** libpsl was not
>> >>found. Fallback to Wget builtin cookie checking.)])>>
>> >> ])
>>
>> I've not tested it but I think this version should fix the problem we
>> had before.  Just one observation, can we use a different name instead
>> of "with_libpsl"?  AFAICS, it is used only to display a message at the
>> end of the configure script, so I would prefer we don't mess with
>> variables set already by autoconf.
>> We will then need to set it to no before the AC_CHECK_LIB is used.
>
> Just read that today AC_SEARCH_LIBS should be used instead of AC_CHECK_LIBS.
>
> That would be something like:
>
> AC_ARG_WITH(libpsl,
> AS_HELP_STRING([--without-libpsl], [disable support for libpsl cookie 
> checking.]),
> [],
> [
>   AC_SEARCH_LIBS(psl_builtin, psl,
>  [AC_DEFINE([WITH_LIBPSL], [1], [PSL support enabled])],
>  [AC_MSG_WARN(*** libpsl was not found. Fallback to Wget 
> builtin cookie checking.)])
> ])
> AS_IF([test x$ac_cv_search_psl_builtin != "x-lpsl"], [ ENABLE_PSL=no ], [ 
> ENABLE_PSL=yes ])
>
> ...
> PSL:   $ENABLE_PSL
> ...
>
> Tim
>



-- 
Thanking You,
Darshit Shah

Re: [Bug-wget] [PATCH 10/14] Do not depend on always defined macros

[Steven M. Schweda]

From: Giuseppe Scrivano 

> ops, thanks to have catched it.  I've fixed it in my local copy, this is
> how it looks now:
> 
> diff --git a/vms/vms.h b/vms/vms.h
> index d65aeda..6053df1 100644
> --- a/vms/vms.h
> +++ b/vms/vms.h
> @@ -49,16 +49,6 @@ int utime( const char *path, const struct utimbuf *times);

   There was also that comment near the beginning:

> ALP $ gdiff wget-1_14/vms/vms.h_orig wget-1_14/vms/vms.h
> 10,12d9
> <  *  Emergency substitution of stat() for lstat() for VAX and VMS CRTL
> <  *  before V7.3-1.
> <  *

   It might make sense to fetch one of my 1.15 source kits, and see what
else has changed since 1.12.  (Probably many things.)

   I don't know how much sense it makes to include VMS-specific files in
the official sources.  Without a VMS system, you really can't
maintain/test them, and I usually don't do anything until a real release
occurs, so it's hard for you to do a release with good VMS stuff in it. 
And, recent "tar" kits have not included any VMS-specific files, anyway.

   SMS.

Re: [Bug-wget] [bug-wget] Libpsl for cookie domain checking in Wget

[Tim Rühsen]

Am Donnerstag, 12. Juni 2014, 13:24:02 schrieb Giuseppe Scrivano:
> Darshit Shah  writes:
> > On Wed, Jun 11, 2014 at 5:20 PM, Tim Rühsen  wrote:
> >> Am Freitag, 6. Juni 2014, 13:39:32 schrieb Darshit Shah:
> >>> I'm facing an issue with the patch I submitted for libpsl and would be
> >>> glad if someone could help me.
> >>> 
> >>> The configure.ac file does not work as expected. When libpsl is not
> >>> installed on a system, the LDFLAGS does not contain -lpsl flag, but
> >>> the configure summary shows LIBPSL: Yes.
> >>> 
> >>> There is some discrepency in the output that I'd like to fix. The
> >>> build completes successfully because the HAVE_LIBPSL variable isn't
> >>> set, and Wget compiles without libpsl support. This should however
> >>> happen only when --without-libpsl was explicitly specified as a
> >>> configure option.
> >> 
> >> This should do it:
> >> 
> >> AC_ARG_WITH(libpsl,
> >> 
> >> AS_HELP_STRING([--without-libpsl], [disable support for libpsl cookie
> >> checking.]), [
> >> 
> >>   with_libpsl=no
> >> 
> >> ], [
> >> 
> >>   AC_CHECK_LIB(psl, psl_builtin,
> >>   
> >>[with_libpsl=yes; AC_DEFINE([WITH_LIBPSL], [1], [PSL
> >>support enabled]) LIBS="${LIBS} -lpsl"],
> >>[with_libpsl=no; AC_MSG_WARN(*** libpsl was not
> >>found. Fallback to Wget builtin cookie checking.)])>>   
> >>   
> >> ])
> 
> I've not tested it but I think this version should fix the problem we
> had before.  Just one observation, can we use a different name instead
> of "with_libpsl"?  AFAICS, it is used only to display a message at the
> end of the configure script, so I would prefer we don't mess with
> variables set already by autoconf.
> We will then need to set it to no before the AC_CHECK_LIB is used.

Just read that today AC_SEARCH_LIBS should be used instead of AC_CHECK_LIBS.

That would be something like:

AC_ARG_WITH(libpsl,
AS_HELP_STRING([--without-libpsl], [disable support for libpsl cookie 
checking.]),
[],
[
  AC_SEARCH_LIBS(psl_builtin, psl,
 [AC_DEFINE([WITH_LIBPSL], [1], [PSL support enabled])],
 [AC_MSG_WARN(*** libpsl was not found. Fallback to Wget 
builtin cookie checking.)])
])
AS_IF([test x$ac_cv_search_psl_builtin != "x-lpsl"], [ ENABLE_PSL=no ], [ 
ENABLE_PSL=yes ])

...
PSL:   $ENABLE_PSL
...

Tim

Re: [Bug-wget] [PATCH 02/14] Do not use exit() with a magic number

[Gisle Vanem]

 wrote:

...


diff --git a/src/mswindows.c b/src/mswindows.c
index 179773e..c0d9be6 100644
--- a/src/mswindows.c
+++ b/src/mswindows.c
@@ -308,7 +308,7 @@ cleanup:

  /* We're the parent.  If all is well, terminate.  */
  if (rv)
-exit (0);
+exit (WGET_EXIT_SUCCESS);


This breaks compilation of mswindows.c as I wrote to Guiseppe
privately; add "exits.h" at top.

--gv

Re: [Bug-wget] [bug-wget] Libpsl for cookie domain checking in Wget

[Tim Rühsen]

Am Donnerstag, 12. Juni 2014, 07:16:13 schrieb Darshit Shah:
> Yes, the configure statements given by Tim work. I found out that the issue
> on machine was caching of configure values. Deleting the configure cache
> fixed the issue.
> 
> I also agree with Giuseppe's point about not using the autoconf variables.
> Let's fix the rest of them too. Tim's patch however seems to add -lpsl
> twice for me. Removing the line that explicitly adds it to LIBS does the
> trick for me.

Your configure.ac adds -lpsl later again. Did you use/check/diff the 
configure.ac 
that i sent you ? I removed the second AC_CHECK_LIB... 

Tim

Re: [Bug-wget] Issue in cookie path checking

[yasuhisa . ishikawa]

Hi Darshit,

 Sorry to be late. I have never used git so it takes long for me to learn
how to generate path using git.

 I’m not sure I have done it well. If there are something wrong with this
attachment, please correct.




0001-2014-06-12-Yasuhisa-Ishikawa-yasuhisa.ishikawa-kuman.patch
Description: Binary data



Regards,
Yasuhisa Ishikawa

2014/06/04 3:09、Darshit Shah  のメール：

> Hi Yasuhisa,
> 
> Thanks for the patch. The cookie domain patch checking code in Wget
> was old and only based on a heuristic. However, we are currently in
> the process of using libpsl a library that handles this for us. I have
> submitted a patch which is currently in the pipeline that adds support
> for using libpsl to perform cookie domain name checking.
> 
> Your code may stil be useful in the fallback mechanism. Could you
> please send a patch file as generated by `git format-patch` against
> the current HEAD of the tree and also add the details to the relevant
> ChangeLog file? It would make applying the patch so much easier for
> us.
> 
> On Thu, May 8, 2014 at 8:12 PM, yasuhisa.ishik...@kumaneko.org
>  wrote:
>> Hi all,
>> 
>> I found two issues in path checking code in cookie.c.
>> 
>> In cookie_handle_set_cookie(), path in Set-Cookie header should be
>> checked so as not to be accepted when it is upper than that of
>> requested document.
>> 
>> However, current implementation works as:
>> 
>> - check_path_match() validate the path of requested document
>>  when its prefix is same with cookie_path.
>>  path_matches(full_path, prefix) checks if full_path starts with prefix.
>>  Current code allows /foo/bar/test.html to issue path=/ cookie.
>>  Expected behavior is opposite. cookie_path must be child of current path.
>> 
>> - cookie->path is compared with path(full document path including filename)
>>  in stead of its parent path.
>> 
>> I applied following fix, and it works as expected. Please consider to merge 
>> this fix in next release.
>> 
>> $ diff -c wget-1.15/src/cookies.c.orig wget-1.15/src/cookies.c
>> *** wget-1.15/src/cookies.c.orig2013-10-21 23:50:12.0 +0900
>> --- wget-1.15/src/cookies.c 2014-05-08 22:47:57.317467164 +0900
>> ***
>> *** 634,640 
>>  static bool
>>  check_path_match (const char *cookie_path, const char *path)
>>  {
>> !   return path_matches (path, cookie_path) != 0;
>>  }
>> 
>>  /* Prepend '/' to string S.  S is copied to fresh stack-allocated
>> --- 634,640 
>>  static bool
>>  check_path_match (const char *cookie_path, const char *path)
>>  {
>> !   return path_matches (cookie_path, path) != 0;
>>  }
>> 
>>  /* Prepend '/' to string S.  S is copied to fresh stack-allocated
>> ***
>> *** 707,713 
>>else
>>  {
>>/* The cookie sets its own path; verify that it is legal. */
>> !   if (!check_path_match (cookie->path, path))
>>  {
>>DEBUGP (("Attempt to fake the path: %s, %s\n",
>> cookie->path, path));
>> --- 707,714 
>>else
>>  {
>>/* The cookie sets its own path; verify that it is legal. */
>> !   char *trailing_slash = strrchr (path, '/');
>> !   if (!check_path_match (cookie->path, trailing_slash ? strdupdelim 
>> (path, trailing_slash + 1) : '/'))
>>  {
>>DEBUGP (("Attempt to fake the path: %s, %s\n",
>> cookie->path, path));
>> $
>> 
>> Thanks,
>> Yasuhisa Ishikawa
> 
> 
> 
> -- 
> Thanking You,
> Darshit Shah

Re: [Bug-wget] [bug-wget] Libpsl for cookie domain checking in Wget

[Darshit Shah]

Yes, the configure statements given by Tim work. I found out that the issue on 
machine was caching of configure values. Deleting the configure cache fixed the 
issue. 

I also agree with Giuseppe's point about not using the autoconf variables. 
Let's fix the rest of them too. 
Tim's patch however seems to add -lpsl twice for me. Removing the line that 
explicitly adds it to LIBS does the trick for me. 

P.S. Currently on mobile. Sorry for any formatting errors. 

—
Thanking you, 
Darshit Shah 
Sent from mobile. Please excuse any errors.

On Thu, Jun 12, 2014 at 4:54 PM, Giuseppe Scrivano 
wrote:

> Darshit Shah  writes:
>> On Wed, Jun 11, 2014 at 5:20 PM, Tim Rühsen  wrote:
>>> Am Freitag, 6. Juni 2014, 13:39:32 schrieb Darshit Shah:
 I'm facing an issue with the patch I submitted for libpsl and would be
 glad if someone could help me.

 The configure.ac file does not work as expected. When libpsl is not
 installed on a system, the LDFLAGS does not contain -lpsl flag, but
 the configure summary shows LIBPSL: Yes.

 There is some discrepency in the output that I'd like to fix. The
 build completes successfully because the HAVE_LIBPSL variable isn't
 set, and Wget compiles without libpsl support. This should however
 happen only when --without-libpsl was explicitly specified as a
 configure option.
>>>
>>> This should do it:
>>>
>>> AC_ARG_WITH(libpsl,
>>> AS_HELP_STRING([--without-libpsl], [disable support for libpsl cookie 
>>> checking.]),
>>> [
>>>   with_libpsl=no
>>> ], [
>>>   AC_CHECK_LIB(psl, psl_builtin,
>>>[with_libpsl=yes; AC_DEFINE([WITH_LIBPSL], [1], [PSL 
>>> support enabled]) LIBS="${LIBS} -lpsl"],
>>>[with_libpsl=no; AC_MSG_WARN(*** libpsl was not found. 
>>> Fallback to Wget builtin cookie checking.)])
>>> ])
>>>
> I've not tested it but I think this version should fix the problem we
> had before.  Just one observation, can we use a different name instead
> of "with_libpsl"?  AFAICS, it is used only to display a message at the
> end of the configure script, so I would prefer we don't mess with
> variables set already by autoconf.
> We will then need to set it to no before the AC_CHECK_LIB is used.
> Thanks,
> Giuseppe

Re: [Bug-wget] [bug-wget] Libpsl for cookie domain checking in Wget

[Giuseppe Scrivano]

Darshit Shah  writes:

> On Wed, Jun 11, 2014 at 5:20 PM, Tim Rühsen  wrote:
>> Am Freitag, 6. Juni 2014, 13:39:32 schrieb Darshit Shah:
>>> I'm facing an issue with the patch I submitted for libpsl and would be
>>> glad if someone could help me.
>>>
>>> The configure.ac file does not work as expected. When libpsl is not
>>> installed on a system, the LDFLAGS does not contain -lpsl flag, but
>>> the configure summary shows LIBPSL: Yes.
>>>
>>> There is some discrepency in the output that I'd like to fix. The
>>> build completes successfully because the HAVE_LIBPSL variable isn't
>>> set, and Wget compiles without libpsl support. This should however
>>> happen only when --without-libpsl was explicitly specified as a
>>> configure option.
>>
>> This should do it:
>>
>> AC_ARG_WITH(libpsl,
>> AS_HELP_STRING([--without-libpsl], [disable support for libpsl cookie 
>> checking.]),
>> [
>>   with_libpsl=no
>> ], [
>>   AC_CHECK_LIB(psl, psl_builtin,
>>[with_libpsl=yes; AC_DEFINE([WITH_LIBPSL], [1], [PSL 
>> support enabled]) LIBS="${LIBS} -lpsl"],
>>[with_libpsl=no; AC_MSG_WARN(*** libpsl was not found. 
>> Fallback to Wget builtin cookie checking.)])
>> ])
>>

I've not tested it but I think this version should fix the problem we
had before.  Just one observation, can we use a different name instead
of "with_libpsl"?  AFAICS, it is used only to display a message at the
end of the configure script, so I would prefer we don't mess with
variables set already by autoconf.
We will then need to set it to no before the AC_CHECK_LIB is used.

Thanks,
Giuseppe

Re: [Bug-wget] [bug-wget] Libpsl for cookie domain checking in Wget

[Tim Rühsen]

Am Mittwoch, 11. Juni 2014, 18:57:13 schrieb Darshit Shah:
> On Wed, Jun 11, 2014 at 5:20 PM, Tim Rühsen  wrote:
> > Am Freitag, 6. Juni 2014, 13:39:32 schrieb Darshit Shah:
> >> I'm facing an issue with the patch I submitted for libpsl and would be
> >> glad if someone could help me.
> >> 
> >> The configure.ac file does not work as expected. When libpsl is not
> >> installed on a system, the LDFLAGS does not contain -lpsl flag, but
> >> the configure summary shows LIBPSL: Yes.
> >> 
> >> There is some discrepency in the output that I'd like to fix. The
> >> build completes successfully because the HAVE_LIBPSL variable isn't
> >> set, and Wget compiles without libpsl support. This should however
> >> happen only when --without-libpsl was explicitly specified as a
> >> configure option.
> > 
> > This should do it:
> > 
> > AC_ARG_WITH(libpsl,
> > 
> > AS_HELP_STRING([--without-libpsl], [disable support for libpsl cookie
> > checking.]), [
> > 
> >   with_libpsl=no
> > 
> > ], [
> > 
> >   AC_CHECK_LIB(psl, psl_builtin,
> >   
> >[with_libpsl=yes; AC_DEFINE([WITH_LIBPSL], [1], [PSL
> >support enabled]) LIBS="${LIBS} -lpsl"],
> >[with_libpsl=no; AC_MSG_WARN(*** libpsl was not found.
> >Fallback to Wget builtin cookie checking.)])> 
> > ])
> > 
> > But I can't compile the gnulib when having WITH_LIBPSL=1.
> > ./configure sets FTELLO_BROKEN_AFTER_SWITCHING_FROM_READ_TO_WRITE to 1
> > which causes undefined fp_ which causes headaches with some SOLARIS code
> > :-(
> 
> Hi Tim,
> 
> Thanks for the patch. But it didn't help solve the problem. When
> libpsl isn't installed, by default ./configure succeeds but make fails
> because it somehoe detects libpsl even through it isn't installed on
> the system.

I tested it and it works ok.
Perhaps you made a mistake somewhere !?

I attached my current configure.ac - please give it a try. Don't forget 
'autoreconf' after changing configure.ac. After that I do
./configure
make clean
make

If libpsl is still detected, make sure you really uninstalled libpsl.
Also 'grep -i PSL src/config.h config.log', HAVE_LIBPSL should be unset in 
src/config.h.

Further steps include looking into config.log (search for psl) to see what is 
going on.

Shouldn't be too hard to find.

Timdnl Template file for GNU Autoconf
dnl Copyright (C) 1995, 1996, 1997, 2001, 2007, 2008, 2009, 2010, 2011, 2012,
dnl 2013, 2014 Free Software Foundation, Inc.

dnl This program is free software; you can redistribute it and/or modify
dnl it under the terms of the GNU General Public License as published by
dnl the Free Software Foundation; either version 3 of the License, or
dnl (at your option) any later version.

dnl This program is distributed in the hope that it will be useful,
dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
dnl GNU General Public License for more details.

dnl You should have received a copy of the GNU General Public License
dnl along with this program.  If not, see .

dnl Additional permission under GNU GPL version 3 section 7

dnl If you modify this program, or any covered work, by linking or
dnl combining it with the OpenSSL project's OpenSSL library (or a
dnl modified version of that library), containing parts covered by the
dnl terms of the OpenSSL or SSLeay licenses, the Free Software Foundation
dnl grants you additional permission to convey the resulting work.
dnl Corresponding Source for a non-source form of such a combination
dnl shall include the source code for the parts of OpenSSL used as well
dnl as that of the covered work.

dnl
dnl Process this file with autoconf to produce a configure script.
dnl

AC_INIT([wget],
m4_esyscmd([build-aux/git-version-gen .tarball-version]),
[bug-wget@gnu.org])
AC_PREREQ(2.61)

dnl
dnl What version of Wget are we building?
dnl
AC_MSG_NOTICE([configuring for GNU Wget $PACKAGE_VERSION])

AC_CONFIG_MACRO_DIR([m4])
AC_CONFIG_AUX_DIR([build-aux])

AC_CONFIG_SRCDIR([src/wget.h])

dnl
dnl Automake setup
dnl
AM_INIT_AUTOMAKE([1.9])

dnl
dnl Get cannonical host
dnl
AC_CANONICAL_HOST
AC_DEFINE_UNQUOTED([OS_TYPE], "$host_os",
   [Define to be the name of the operating system.])

dnl
dnl Process features.
dnl

AC_ARG_WITH(libpsl,
AS_HELP_STRING([--without-libpsl], [disable support for libpsl cookie 
checking.]),
[
  with_libpsl=no
], [
  AC_CHECK_LIB(psl, psl_builtin,
   [with_libpsl=yes; AC_DEFINE([WITH_LIBPSL], [1], [PSL support 
enabled]) LIBS="${LIBS} -lpsl"], 
   [with_libpsl=no; AC_MSG_WARN(*** libpsl was not found. 
Fallback to Wget builtin cookie checking.)])
])

AC_ARG_WITH(ssl,
[[  --without-ssl   disable SSL autodetection
  --with-ssl={gnutls,openssl} specify the SSL backend.  GNU TLS is the 
default.]])

AC_ARG_WITH