Hi, here is patch to avoid the crash, please review.
@Darshit As you say, we probably should discuss the program logic in http_loop() regarding if-modified-since !? Regards, Tim Am Samstag, 21. November 2015, 00:10:44 schrieb Darshit Shah: > Another thing that I just remembered, this issue seems to pop up when the > file being downloaded already exists on disk. Maybe, that is why you're > seeing the different behaviour? > > Try downloading the file when it already exists and see if the problem can > be reproduced on the newer system. > > On 11/20, Darshit Shah wrote: > >This looks similar to another segfault I've seen. I'm not sure since > >when it exists in the code, but I did come across one recently. That > >case was caused when --trust-server-names -N and --content-disposition > >were all provided to Wget. A wrong logic condition causes Wget to work > >on output_filename = NULL which eventually resents in a segfault. > > > >I'm assuming the case you've provided is very similar to what I've > >seen. I was using Clang when I saw this issue. Unfortunately, I'm > >currently swamped with other work and am unable to look more deeply > >into this. > > > >The stack trace provided does match my explanation above, so we should > >be able to track it down and fix it. Some interplay of multiple > >options is causing this bug, though I can't explain why it works with > >one GCC version and not with another. The way I see it, it was a logic > >bug in the code. > > > >Unfortunately, On 11/20, Schleusener, Jens wrote: > >>Hi, > >> > >>under some conditions I get with a self-compiled wget 1.17 binary on > >>a 64-bit openSUSE 13.1 Linux system a segmentation fault (but the > >>self-compiled wget 1.16.3 works correctly). > >> > >>I could reduce the problem to this usage case: > >> > >>wget -N --content-disposition http://ftp.gnu.org/gnu/wget/ > >> > >>while the usage of only "-N" or "--content-disposition" let wget work. > >> > >>Sorry, I am not a C expert but nevertheless I tried to use gdb on > >>the resulting core dump as best I could with the following result: > >> > >>Program terminated with signal SIGSEGV, Segmentation fault. > >>#0 0x00007f5b0899d42a in strlen () from /lib64/libc.so.6 > >>(gdb) bt > >>#0 0x00007f5b0899d42a in strlen () from /lib64/libc.so.6 > >>#1 0x0000000000420cdf in set_file_timestamp () > >>#2 0x00000000004241e0 in http_loop () > >>#3 0x0000000000433619 in retrieve_url () > >>#4 0x000000000042c64b in main () > >> > >>The used gcc version is > >>4.8.1 > >>and a "rpm -qf /lib64/libc.so.6" issues > >>glibc-2.18-4.38.1.x86_64 > >> > >>On a newer openSUSE Leap 42.1 Linux system the "identically" > >>compiled wget doesn't segfault and works ok. On that system the gcc > >>version is > >>4.8.5 > >>and a "rpm -qf /lib64/libc.so.6" issues > >>glibc-2.19-17.4.x86_64 > >> > >>Regards > >> > >>Jens
From 0eacdbfc1b3341fce2fcb8239dcd81d6dd3969f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim Rühsen?= <[email protected]> Date: Sat, 21 Nov 2015 21:44:11 +0100 Subject: [PATCH] Fix SIGSEGV in -N / --content-disposition combination * src/http.c (http_loop): Fix SIGSEGV Reported-by: "Schleusener, Jens" <[email protected]> --- src/http.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/http.c b/src/http.c index 355ff53..9d71483 100644 --- a/src/http.c +++ b/src/http.c @@ -3794,7 +3794,6 @@ http_loop (struct url *u, struct url *original_url, char **newloc, struct http_stat hstat; /* HTTP status */ struct_stat st; bool send_head_first = true; - char *file_name; bool force_full_retrieve = false; @@ -3864,11 +3863,6 @@ http_loop (struct url *u, struct url *original_url, char **newloc, if (opt.content_disposition && opt.always_rest) send_head_first = true; - if (!opt.output_document) - file_name = url_file_name (opt.trustservernames ? u : original_url, NULL); - else - file_name = xstrdup (opt.output_document); - #ifdef HAVE_METALINK if (opt.metalink_over_http) { @@ -3881,7 +3875,7 @@ http_loop (struct url *u, struct url *original_url, char **newloc, { /* Use conditional get request if requested * and if timestamp is known at this moment. */ - if (opt.if_modified_since && file_exists_p (file_name) && !send_head_first) + if (opt.if_modified_since && !send_head_first && got_name && file_exists_p (hstat.local_file)) { *dt |= IF_MODIFIED_SINCE; { @@ -3892,12 +3886,10 @@ http_loop (struct url *u, struct url *original_url, char **newloc, } /* Send preliminary HEAD request if -N is given and we have existing * destination file or content disposition is enabled. */ - else if (file_exists_p (file_name) || opt.content_disposition) + else if (opt.content_disposition || file_exists_p (hstat.local_file)) send_head_first = true; } - xfree (file_name); - /* THE loop */ do { -- 2.6.2
signature.asc
Description: This is a digitally signed message part.
