Re: [Bug-wget] [Bug-Wget] Wget closes connection despite Keep-Alive
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] Send Content-Length with POST 0 length body
Matthew Atkinson mutley...@ntlworld.com writes: I don't think this ever got pushed. thanks to have checked it. I've pushed it right now. Regards, Giuseppe
Re: [Bug-wget] [Bug-Wget] Wget closes connection despite Keep-Alive
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] certificate revocation lists (CRLs) #43501
On Wednesday 05 November 2014 12:24:06 Noël Köthe wrote: Hello, wget does not support CRLs. There is a bug report about this here: https://savannah.gnu.org/bugs/?43501 The first step could to document (IMHO prefered in the manpage) this behavior (see attached first ugly patch because I don't know where to place this better). The next and better step might be to implement this by loading CRLs files (reporter points to curl where this is possible) then this patch should be removed again. Maybe you agree and apply this minor documentation patch. Thank you, Noël. On 24th Oct I pushed a change to Mget that allows to specify a CRL file via --crl-file. If nobody complains, I would fit that patch to Wget's GnuTLS code. BTW, does Debian meanwhile has a CRL infrastructure (something like /etc/ssl/certs/) or is planning something like it ? Also, OCSP certificate status checking might be interesting for Wget. Tim signature.asc Description: This is a digitally signed message part.
Re: [Bug-wget] certificate revocation lists (CRLs) #43501
Hello Debian,;) wget developers are working on CRL support and raised the following questions which somebody of you guys have a better answer: Am Mittwoch, den 05.11.2014, 12:48 +0100 schrieb Tim Ruehsen: BTW, does Debian meanwhile has a CRL infrastructure (something like /etc/ssl/certs/) or is planning something like it ? I'm aware of fetch-crl https://packages.debian.org/unstable/main/fetch-crl but maybe there is more anything planed like CRL support for the ca-certificates package? Thx. Regards Noël -- Noël Köthe noel debian.org Debian GNU/Linux, www.debian.org signature.asc Description: This is a digitally signed message part
Re: [Bug-wget] certificate revocation lists (CRLs) #43501
Hello Tim, Am Mittwoch, den 05.11.2014, 12:48 +0100 schrieb Tim Ruehsen: https://savannah.gnu.org/bugs/?43501 The first step could to document (IMHO prefered in the manpage) this behavior (see attached first ugly patch because I don't know where to place this better). The next and better step might be to implement this by loading CRLs files (reporter points to curl where this is possible) then this patch should be removed again. On 24th Oct I pushed a change to Mget that allows to specify a CRL file via --crl-file. If nobody complains, I would fit that patch to Wget's GnuTLS code. :) That you be perfect for this request. BTW, does Debian meanwhile has a CRL infrastructure (something like /etc/ssl/certs/) or is planning something like it ? I'm not aware of an infrastructure but asked the people who might know this (CC: to this list). Also, OCSP certificate status checking might be interesting for Wget. :) ACK. Thank you. Regards Noël -- Noël Köthe noel debian.org Debian GNU/Linux, www.debian.org signature.asc Description: This is a digitally signed message part
Re: [Bug-wget] [PATCH] fixing clock_gettime link error
On Tuesday 04 November 2014 10:36:41 Tim Ruehsen wrote: 2. make check says clock_gettime undefined: I need to specify LIBS=-lrt with configure. (regression to wget-1.15) I set up a Debian lenny with glibc 2.11 and could reproduce the problem. This patch fixes it for me (using same method as in src/Makefile.am). Pushed it. Tim signature.asc Description: This is a digitally signed message part.
Re: [Bug-wget] certificate revocation lists (CRLs) #43501
On Wed, 2014-11-05 at 13:51 +0100, Noël Köthe wrote: I'm aware of fetch-crl https://packages.debian.org/unstable/main/fetch-crl but maybe there is more anything planed like CRL support for the ca-certificates package? My personal experience with this (and we massively use it in the LCG, where fetch-crl actually comes from),... it doesn't work well. Not talking about fetch-crl itslef, but rather the CAs,... their CRLs are every now and then (read: suprisingly often) not downloadble, for whatever miscellaneous and strange reasons. So in the end, you either make all that voluntary (i.e. not fail, even if the CRL is out of date), which makes it of course completely useless... or you make it mandatory, but then you see rather often failures. Of course this doesn't mean one shouldn't use CRLs, but as long as the major players don't use them we have at least little chance to make pressure on the CAs to fix their server infrastructure :-( Cheers, Chris. smime.p7s Description: S/MIME cryptographic signature
Re: [Bug-wget] [PATCH] use python test suite only if python3 found
Am Dienstag, 4. November 2014, 17:19:42 schrieb Giuseppe Scrivano: Tim Ruehsen tim.rueh...@gmx.de writes: * configure.ac: check for python3 * Makefile.am: only use python test suite if python3 found On system without python3 the test suite will fail since the tests are designed for python3 only. Please review. Tim * configure.ac: Fix check for libpsl diff --git a/Makefile.am b/Makefile.am index a059794..8bf7def 100644 --- a/Makefile.am +++ b/Makefile.am @@ -41,7 +41,10 @@ distuninstallcheck_listfiles = find . -type f | \ ACLOCAL_AMFLAGS = -I m4 # subdirectories in the distribution -SUBDIRS = lib src doc po tests util testenv +SUBDIRS = lib src doc po tests util +if HAVE_PYTHON + SUBDIRS += testenv +endif I-ve not tried it, but I am afraid this will break make dist on systems without python3 as testenv is not even included in Makefile.in. We should check for HAVE_PYTHON (that ideally is HAVE_PYTHON3) inside the testenv/Makefile.am itself and skip tests if not present. You are right. I amended the patch to reflect your idea. I tested make distcheck with and without python3 available. Please have a look. Tim From 3855dc0f6b480624d6150836c3829fb09d05bccc Mon Sep 17 00:00:00 2001 From: Tim Ruehsen tim.rueh...@gmx.de Date: Wed, 5 Nov 2014 21:35:13 +0100 Subject: [PATCH] Skip python test suite if python3 is not available --- ChangeLog | 5 + configure.ac| 6 ++ testenv/Makefile.am | 6 -- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index a7357aa..e9595f7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2014-11-05 Tim Ruehsen tim.rueh...@gmx.de + + * configure.ac: check for python3 + * Makefile.am: only use python test suite if python3 found + 2014-11-05 Giuseppe Scrivano gscri...@redhat.com * contrib/tsocked-wget (TSOCKS_CONF_FILE): Remove empty new-line. diff --git a/configure.ac b/configure.ac index 01d3eef..7028481 100644 --- a/configure.ac +++ b/configure.ac @@ -136,6 +136,12 @@ AC_AIX gl_EARLY dnl +dnl Find python3 +dnl +AM_PATH_PYTHON([3.0],,[:]) +AM_CONDITIONAL([HAVE_PYTHON3], [test $PYTHON != :]) + +dnl dnl Gettext dnl AM_GNU_GETTEXT([external],[need-ngettext]) diff --git a/testenv/Makefile.am b/testenv/Makefile.am index 33604bc..f745bdb 100644 --- a/testenv/Makefile.am +++ b/testenv/Makefile.am @@ -29,7 +29,8 @@ AUTOMAKE_OPTIONS = parallel-tests AM_TESTS_ENVIRONMENT = export WGETRC=/dev/null; MAKE_CHECK=True; export MAKE_CHECK;\ export PYTHONPATH=$$PYTHONPATH:$(srcdir); export VALGRIND_TESTS=@VALGRIND_TESTS@; -TESTS = Test-auth-basic-fail.py \ +if HAVE_PYTHON3 + TESTS = Test-auth-basic-fail.py \ Test-auth-basic.py \ Test-auth-both.py \ Test-auth-digest.py \ @@ -50,7 +51,8 @@ TESTS = Test-auth-basic-fail.py \ Test-Post.py\ Test--spider-r.py -XFAIL_TESTS = Test-auth-both.py + XFAIL_TESTS = Test-auth-both.py +endif EXTRA_DIST = certs conf exc misc server test README $(TESTS) $(XFAIL_TESTS) -- 2.1.1 signature.asc Description: This is a digitally signed message part.
Re: [Bug-wget] [PATCH] wget --local-encoding=blorp crashes, prints wrong error message
Am Dienstag, 4. November 2014, 12:57:43 schrieb Tim Ruehsen: On Tuesday 04 November 2014 12:02:08 Mikael Magnusson wrote: % wget --local-encoding=blorp google.com Conversion from 'blorp' to 'blorp' isn't supported zsh: segmentation fault wget --local-encoding=blorp google.com (the message should say to 'UTF-8'). % wget --local-encoding= google.com converted 'http://google.com' () - 'http://google.com' (UTF-8) ... 2014-11-04 09:00:22 (590 KB/s) - 'index.html' saved [17800] It's unclear what this conversion does. Thanks for reporting, Mikael. The 'converted' messages were meant as debug messages. The somehow dropped into a commit. The appended patch fixes the sigfault und changes the 'converted' messages into DEBUG ones. Also the 'Conversion from...' message has been fixed. I pushed it. Tim signature.asc Description: This is a digitally signed message part.
Re: [Bug-wget] [PATCH] use python test suite only if python3 found
Tim Rühsen tim.rueh...@gmx.de writes: Am Dienstag, 4. November 2014, 17:19:42 schrieb Giuseppe Scrivano: Tim Ruehsen tim.rueh...@gmx.de writes: * configure.ac: check for python3 * Makefile.am: only use python test suite if python3 found On system without python3 the test suite will fail since the tests are designed for python3 only. Please review. Tim * configure.ac: Fix check for libpsl diff --git a/Makefile.am b/Makefile.am index a059794..8bf7def 100644 --- a/Makefile.am +++ b/Makefile.am @@ -41,7 +41,10 @@ distuninstallcheck_listfiles = find . -type f | \ ACLOCAL_AMFLAGS = -I m4 # subdirectories in the distribution -SUBDIRS = lib src doc po tests util testenv +SUBDIRS = lib src doc po tests util +if HAVE_PYTHON + SUBDIRS += testenv +endif I-ve not tried it, but I am afraid this will break make dist on systems without python3 as testenv is not even included in Makefile.in. We should check for HAVE_PYTHON (that ideally is HAVE_PYTHON3) inside the testenv/Makefile.am itself and skip tests if not present. You are right. I amended the patch to reflect your idea. I tested make distcheck with and without python3 available. Please have a look. yes, now the patch looks correct since we list all tests in EXTRA_DIST and there is no risk to miss them in a make dist. Regards, Giuseppe
