Re: [Bug-wget] [PATCH 01/14] main: make program_name not static
Darshit Shah dar...@gmail.com writes: diff --git a/src/ChangeLog b/src/ChangeLog index 5ebeaf3..f93cd70 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,5 +1,7 @@ 2014-06-08 Giuseppe Scrivano gscri...@redhat.com + * main.c: Make `program_name' not static. + * cookies.c [HAVE_PSL]: Include libpsl.h only when HAVE_PSL is defined. You probably wanted to add a new ChangeLog entry here. I think there should not be duplicate header lines (date-author) in the ChangeLog file, and that changes related to the same commit are grouped without any empty line. At least this is what I can understand from: http://www.gnu.org/prep/standards/standards.html#Change-Logs But the truth is that I use Emacs C-x 4 a to add a changelog entry then ensure there are not empty lines for changes related to the same commit :-) Regards, Giuseppe
Re: [Bug-wget] [PATCH 10/14] Do not depend on always defined macros
Steven M. Schweda s...@antinode.info writes: diff --git a/vms/vms.h b/vms/vms.h index d65aeda..19efdef 100644 --- a/vms/vms.h +++ b/vms/vms.h @@ -54,8 +54,6 @@ int utime( const char *path, const struct utimbuf *times); */ #if defined(__VAX) || __CRTL_VER 70301000 -#define lstat( __p1, __p2) stat( __p1, __p2) - #endif /* defined(__VAX) || __CRTL_VER 70301000 */ That section disappeared (in my code) in version 1.14. Note that after removing the #define, there's nothing left in the #if-#endif block, and the related comments are similarly pointless. 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); #endif /* __CRTL_VER 7030 */ -/* Emergency substitution of stat() for lstat() for VAX and VMS CRTL - before V7.3-1. -*/ -#if defined(__VAX) || __CRTL_VER 70301000 - -#define lstat( __p1, __p2) stat( __p1, __p2) - -#endif /* defined(__VAX) || __CRTL_VER 70301000 */ - - /* Global storage. */ /*VMS destination file system type. 0: unset/unknown Thanks, Giuseppe
Re: [Bug-wget] [bug-wget] Libpsl for cookie domain checking in Wget
Am Mittwoch, 11. Juni 2014, 18:57:13 schrieb Darshit Shah:
On Wed, Jun 11, 2014 at 5:20 PM, Tim Rühsen tim.rueh...@gmx.de 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 http://www.gnu.org/licenses/.
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(zlib,
[[ --without-zlib disable zlib ]])
AC_ARG_ENABLE(opie,
[
Re: [Bug-wget] [bug-wget] Libpsl for cookie domain checking in Wget
Darshit Shah dar...@gmail.com writes:
On Wed, Jun 11, 2014 at 5:20 PM, Tim Rühsen tim.rueh...@gmx.de 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
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 gscriv...@gnu.org
wrote:
Darshit Shah dar...@gmail.com writes:
On Wed, Jun 11, 2014 at 5:20 PM, Tim Rühsen tim.rueh...@gmx.de 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] Issue in cookie path checking
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 dar...@gmail.com のメール：
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
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
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] [PATCH 02/14] Do not use exit() with a magic number
gscriv...@gnu.org 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
Am Donnerstag, 12. Juni 2014, 13:24:02 schrieb Giuseppe Scrivano:
Darshit Shah dar...@gmail.com writes:
On Wed, Jun 11, 2014 at 5:20 PM, Tim Rühsen tim.rueh...@gmx.de 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 10/14] Do not depend on always defined macros
From: Giuseppe Scrivano gscriv...@gnu.org 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
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 tim.rueh...@gmx.de wrote:
Am Donnerstag, 12. Juni 2014, 13:24:02 schrieb Giuseppe Scrivano:
Darshit Shah dar...@gmail.com writes:
On Wed, Jun 11, 2014 at 5:20 PM, Tim Rühsen tim.rueh...@gmx.de 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 02/14] Do not use exit() with a magic number
Gisle Vanem gva...@yahoo.no writes: gscriv...@gnu.org 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] [PATCH 10/14] Do not depend on always defined macros
On Thu, Jun 12, 2014 at 8:59 PM, Steven M. Schweda s...@antinode.info wrote: From: Giuseppe Scrivano gscriv...@gnu.org 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
