Hi, "Misra, Deapesh" <[email protected]> writes:
> Yes - I whole heartedly agree with the following: > >> >> To cite myself :) >> "But there is also non-obvious wget behavior in creating those (temp) files >> in >> the filesystem." >> >> The Wget docs just don't make clear that these files come into existence for >> a >> while. Of course we could amend the docs and lean back... but it still is >> not >> an intuitive behavior and I fear people might trap into that pit. And we >> could >> easily prevent it with some lines of code... >> >> Regards, Tim > > Although I am late to this thread, I would like to elucidate the basic issue > I had with the current scenario with an analogy: > > If I assign a guard to a room and order the guard not to allow (say) > people wearing yellow shirts, I intuitively expect that the people > with yellow shirts will be prevented from entering the room and not > that everyone will be allowed into the room and then the yellow > shirted people will be asked to leave. > > When I had thought about the possible solutions, I had thought of storing the > files in a temporary location. But you guys (developers) are on the right > track with all your solutions and the ensuing discussion, IMHO. If nobody is going to complain, I will push these patches shortly:
>From 1904fbfb40d817075a809124cb03640b9b7234df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20R=C3=BChsen?= <[email protected]> Date: Sun, 14 Aug 2016 21:04:58 +0200 Subject: [PATCH 1/2] Limit file mode to u=rw on temp. downloaded files * bootstrap.conf: Add gnulib modules fopen, open. * src/http.c (open_output_stream): Limit file mode to u=rw on temporary downloaded files. Reported-by: "Misra, Deapesh" <[email protected]> Discovered by: Dawid Golunski (http://legalhackers.com) --- bootstrap.conf | 2 ++ src/http.c | 13 ++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/bootstrap.conf b/bootstrap.conf index 2b225b7..d9a5f90 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -40,6 +40,7 @@ dirname fcntl flock fnmatch +fopen futimens ftello getaddrinfo @@ -71,6 +72,7 @@ crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 +open quote quotearg recv diff --git a/src/http.c b/src/http.c index 56b8669..d463f29 100644 --- a/src/http.c +++ b/src/http.c @@ -39,6 +39,7 @@ as that of the covered work. */ #include <errno.h> #include <time.h> #include <locale.h> +#include <fcntl.h> #include "hash.h" #include "http.h" @@ -2471,7 +2472,17 @@ open_output_stream (struct http_stat *hs, int count, FILE **fp) open_id = 22; *fp = fopen (hs->local_file, "wb", FOPEN_OPT_ARGS); #else /* def __VMS */ - *fp = fopen (hs->local_file, "wb"); + if (opt.delete_after + || opt.spider /* opt.recursive is implicitely true */ + || !acceptable (hs->local_file)) + { + *fp = fdopen (open (hs->local_file, O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | S_IWUSR), "wb"); + } + else + { + *fp = fopen (hs->local_file, "wb"); + } + #endif /* def __VMS [else] */ } else -- 2.7.4
>From 7013f8260fc0a2b71f2414593aedb45c77972d98 Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano <[email protected]> Date: Sun, 21 Aug 2016 15:21:44 +0200 Subject: [PATCH 2/2] Append .tmp to temporary files * src/http.c (struct http_stat): Add `temporary` flag. (check_file_output): Append .tmp to temporary files. (open_output_stream): Refactor condition to use hs->temporary instead. Reported-by: "Misra, Deapesh" <[email protected]> Discovered by: Dawid Golunski (http://legalhackers.com) --- NEWS | 6 ++++++ src/http.c | 14 +++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/NEWS b/NEWS index 5073d7e..56c21a5 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,12 @@ See the end for copying conditions. Please send GNU Wget bug reports to <[email protected]>. +* Changes in Wget X.Y.Z + +* On a recursive download, append a .tmp suffix to temporary files + that will be deleted after being parsed, and create them + readable/writable only by the owner. + * Changes in Wget 1.18 * By default, on server redirects to a FTP resource, use the original diff --git a/src/http.c b/src/http.c index d463f29..43bc2cb 100644 --- a/src/http.c +++ b/src/http.c @@ -1569,6 +1569,7 @@ struct http_stat #ifdef HAVE_METALINK metalink_t *metalink; #endif + bool temporary; /* downloading a temporary file */ }; static void @@ -2259,6 +2260,15 @@ check_file_output (struct url *u, struct http_stat *hs, xfree (local_file); } + hs->temporary = opt.delete_after || opt.spider || !acceptable (hs->local_file); + if (hs->temporary) + { + char *tmp = NULL; + asprintf (&tmp, "%s.tmp", hs->local_file); + xfree (hs->local_file); + hs->local_file = tmp; + } + /* TODO: perform this check only once. */ if (!hs->existence_checked && file_exists_p (hs->local_file)) { @@ -2472,9 +2482,7 @@ open_output_stream (struct http_stat *hs, int count, FILE **fp) open_id = 22; *fp = fopen (hs->local_file, "wb", FOPEN_OPT_ARGS); #else /* def __VMS */ - if (opt.delete_after - || opt.spider /* opt.recursive is implicitely true */ - || !acceptable (hs->local_file)) + if (hs->temporary) { *fp = fdopen (open (hs->local_file, O_CREAT | O_TRUNC | O_WRONLY, S_IRUSR | S_IWUSR), "wb"); } -- 2.7.4
Regards, Giuseppe
