Re: [patch] ftp(1): change mtime for http/https links

2017-10-05 Thread Jesper Wallin
...and another bump. :-) *ping sthen@*

On Sun, Sep 24, 2017 at 07:25:33PM +0200, Jesper Wallin wrote:
> On Sat, Sep 23, 2017 at 11:50:46PM +0200, Jesper Wallin wrote:
> > Woups, seems like I managed to break ftp(1) in the installer due to
> > pledge being a bit too tight.  Here is an updated version of the patch
> > and with Philips changes as well.
> 
> ...and hopefully a final version, sorry for the noise.
> 
> Changed the order of the pledge promises to their canonical order as
> given by the manual and removed some empty lines. (thanks anton@)
> 
> I will of course still bump this once we're out of beta.
> 
> 
> Index: fetch.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
> retrieving revision 1.163
> diff -u -p -r1.163 fetch.c
> --- fetch.c   7 Mar 2017 08:00:23 -   1.163
> +++ fetch.c   24 Sep 2017 08:06:26 -
> @@ -210,6 +210,7 @@ url_get(const char *origline, const char
>   int status;
>   int save_errno;
>   const size_t buflen = 128 * 1024;
> + time_t mtime = -1;
>  
>   direction = "received";
>  
> @@ -647,7 +648,7 @@ noslash:
>   if (pledge("stdio rpath inet dns tty",  NULL) == -1)
>   err(1, "pledge");
>   } else {
> - if (pledge("stdio rpath wpath cpath inet dns tty", 
> NULL) == -1)
> + if (pledge("stdio rpath wpath cpath inet fattr dns 
> tty", NULL) == -1)
>   err(1, "pledge");
>   }
>   }
> @@ -860,6 +861,12 @@ noslash:
>   if (restart_point)
>   filesize += restart_point;
>  #endif /* !SMALL */
> +#define LASTMOD "Last-Modified: "
> + } else if (strncasecmp(cp, LASTMOD, sizeof(LASTMOD) - 1) == 0) {
> + struct tm tm;
> + cp += sizeof(LASTMOD) - 1;
> + if (strptime(cp, "%a, %d %b %Y %T %z", &tm) != NULL)
> + mtime = mktime(&tm);
>  #define LOCATION "Location: "
>   } else if (isredirect &&
>   strncasecmp(cp, LOCATION, sizeof(LOCATION) - 1) == 0) {
> @@ -1043,8 +1050,19 @@ cleanup_url_get:
>   fclose(fin);
>   else if (s != -1)
>   close(s);
> - if (out >= 0 && out != fileno(stdout))
> + if (out >= 0 && out != fileno(stdout)) {
> + if (mtime != -1) {
> + struct timespec tv[2];
> + tv[0].tv_nsec = UTIME_NOW;
> + tv[1].tv_sec = mtime;
> + tv[1].tv_nsec = 0;
> + if (futimens(out, tv) == -1)
> + fprintf(ttyout,
> + "Can't change modification time on %s to %s\n",
> + savefile, ctime(&mtime));
> + }
>   close(out);
> + }
>   free(buf);
>   free(proxyhost);
>   free(proxyurl);
> Index: ftp.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
> retrieving revision 1.100
> diff -u -p -r1.100 ftp.c
> --- ftp.c 22 Aug 2016 16:27:00 -  1.100
> +++ ftp.c 24 Sep 2017 08:06:26 -
> @@ -1217,8 +1217,8 @@ break2:
>   ut.modtime = mtime;
>   if (utime(local, &ut) == -1)
>   fprintf(ttyout,
> - "Can't change modification time on %s to %s",
> - local, asctime(localtime(&mtime)));
> + "Can't change modification time on %s to %s\n",
> + local, ctime(&mtime));
>   }
>   }
>   }



Re: [patch] ftp(1): change mtime for http/https links

2017-09-24 Thread Jesper Wallin
On Sat, Sep 23, 2017 at 11:50:46PM +0200, Jesper Wallin wrote:
> Woups, seems like I managed to break ftp(1) in the installer due to
> pledge being a bit too tight.  Here is an updated version of the patch
> and with Philips changes as well.

...and hopefully a final version, sorry for the noise.

Changed the order of the pledge promises to their canonical order as
given by the manual and removed some empty lines. (thanks anton@)

I will of course still bump this once we're out of beta.


Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.163
diff -u -p -r1.163 fetch.c
--- fetch.c 7 Mar 2017 08:00:23 -   1.163
+++ fetch.c 24 Sep 2017 08:06:26 -
@@ -210,6 +210,7 @@ url_get(const char *origline, const char
int status;
int save_errno;
const size_t buflen = 128 * 1024;
+   time_t mtime = -1;
 
direction = "received";
 
@@ -647,7 +648,7 @@ noslash:
if (pledge("stdio rpath inet dns tty",  NULL) == -1)
err(1, "pledge");
} else {
-   if (pledge("stdio rpath wpath cpath inet dns tty", 
NULL) == -1)
+   if (pledge("stdio rpath wpath cpath inet fattr dns 
tty", NULL) == -1)
err(1, "pledge");
}
}
@@ -860,6 +861,12 @@ noslash:
if (restart_point)
filesize += restart_point;
 #endif /* !SMALL */
+#define LASTMOD "Last-Modified: "
+   } else if (strncasecmp(cp, LASTMOD, sizeof(LASTMOD) - 1) == 0) {
+   struct tm tm;
+   cp += sizeof(LASTMOD) - 1;
+   if (strptime(cp, "%a, %d %b %Y %T %z", &tm) != NULL)
+   mtime = mktime(&tm);
 #define LOCATION "Location: "
} else if (isredirect &&
strncasecmp(cp, LOCATION, sizeof(LOCATION) - 1) == 0) {
@@ -1043,8 +1050,19 @@ cleanup_url_get:
fclose(fin);
else if (s != -1)
close(s);
-   if (out >= 0 && out != fileno(stdout))
+   if (out >= 0 && out != fileno(stdout)) {
+   if (mtime != -1) {
+   struct timespec tv[2];
+   tv[0].tv_nsec = UTIME_NOW;
+   tv[1].tv_sec = mtime;
+   tv[1].tv_nsec = 0;
+   if (futimens(out, tv) == -1)
+   fprintf(ttyout,
+   "Can't change modification time on %s to %s\n",
+   savefile, ctime(&mtime));
+   }
close(out);
+   }
free(buf);
free(proxyhost);
free(proxyurl);
Index: ftp.c
===
RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
retrieving revision 1.100
diff -u -p -r1.100 ftp.c
--- ftp.c   22 Aug 2016 16:27:00 -  1.100
+++ ftp.c   24 Sep 2017 08:06:26 -
@@ -1217,8 +1217,8 @@ break2:
ut.modtime = mtime;
if (utime(local, &ut) == -1)
fprintf(ttyout,
-   "Can't change modification time on %s to %s",
-   local, asctime(localtime(&mtime)));
+   "Can't change modification time on %s to %s\n",
+   local, ctime(&mtime));
}
}
}



Re: [patch] ftp(1): change mtime for http/https links

2017-09-23 Thread Jesper Wallin
> On Sat, Sep 23, 2017 at 09:43:12AM +0200, Jesper Wallin wrote:
> Yeah, fully understandable.  I'll bump this once 6.2 is out.
> 
> I'll apply the changes Philip suggested (thanks!) and then build a
> release to verify that everything works.
> 
>
Woups, seems like I managed to break ftp(1) in the installer due to
pledge being a bit too tight.  Here is an updated version of the patch
and with Philips changes as well.


Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.163
diff -u -p -r1.163 fetch.c
--- fetch.c 7 Mar 2017 08:00:23 -   1.163
+++ fetch.c 23 Sep 2017 21:28:49 -
@@ -210,6 +210,7 @@ url_get(const char *origline, const char
int status;
int save_errno;
const size_t buflen = 128 * 1024;
+   time_t mtime = -1;
 
direction = "received";
 
@@ -647,7 +648,7 @@ noslash:
if (pledge("stdio rpath inet dns tty",  NULL) == -1)
err(1, "pledge");
} else {
-   if (pledge("stdio rpath wpath cpath inet dns tty", 
NULL) == -1)
+   if (pledge("stdio rpath wpath cpath fattr inet dns 
tty", NULL) == -1)
err(1, "pledge");
}
}
@@ -860,6 +861,12 @@ noslash:
if (restart_point)
filesize += restart_point;
 #endif /* !SMALL */
+#define LASTMOD "Last-Modified: "
+   } else if (strncasecmp(cp, LASTMOD, sizeof(LASTMOD) - 1) == 0) {
+   struct tm tm;
+   cp += sizeof(LASTMOD) - 1;
+   if (strptime(cp, "%a, %d %b %Y %T %z", &tm) != NULL)
+   mtime = mktime(&tm);
 #define LOCATION "Location: "
} else if (isredirect &&
strncasecmp(cp, LOCATION, sizeof(LOCATION) - 1) == 0) {
@@ -1043,8 +1050,21 @@ cleanup_url_get:
fclose(fin);
else if (s != -1)
close(s);
-   if (out >= 0 && out != fileno(stdout))
+   if (out >= 0 && out != fileno(stdout)) {
+
+   if (mtime != -1) {
+   struct timespec tv[2];
+   tv[0].tv_nsec = UTIME_NOW;
+   tv[1].tv_sec = mtime;
+   tv[1].tv_nsec = 0;
+   if (futimens(out, tv) == -1)
+   fprintf(ttyout,
+   "Can't change modification time on %s to %s\n",
+   savefile, ctime(&mtime));
+   }
+
close(out);
+   }
free(buf);
free(proxyhost);
free(proxyurl);
Index: ftp.c
===
RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
retrieving revision 1.100
diff -u -p -r1.100 ftp.c
--- ftp.c   22 Aug 2016 16:27:00 -  1.100
+++ ftp.c   23 Sep 2017 21:28:49 -
@@ -1217,8 +1217,8 @@ break2:
ut.modtime = mtime;
if (utime(local, &ut) == -1)
fprintf(ttyout,
-   "Can't change modification time on %s to %s",
-   local, asctime(localtime(&mtime)));
+   "Can't change modification time on %s to %s\n",
+   local, ctime(&mtime));
}
}
}



Re: [patch] ftp(1): change mtime for http/https links

2017-09-23 Thread Raf Czlonka
On Sat, Sep 23, 2017 at 10:12:32AM BST, Ted Unangst wrote:
> Raf Czlonka wrote:
> > This is unrelated to your diff but what I do instead is to check
> > the BUILDINFO file - it's tiny and all the information you need,
> > is already there.
> 
> While we're on the subject, I'll point out that sometimes only the base sets
> change, but not the x sets. If you download SHA256 you can download only files
> that differ. (This also may work better if you switch mirrors or the
> timestamps drift for other reasons.) It's just a few lines of shell, though I
> don't have them handy atm.
> 
> (And for bonus points, you can use SHA256.sig and verify it's correct, then
> verify that BUILDINFO hasn't been tampered with, etc...)
> 
> ftp/http should still do the right thing, of course.

Hi Ted et al.

I've mentioned BUILDINFO only as an example of the smallest file
to download which, at the same time contains very useful information.

This is what I use:

BSD="bsd.rd bsd"
test "$(sysctl -n hw.ncpu)" -ge "2" && BSD="$BSD bsd.mp"
ftp -o BUILDINFO.new -V ${SNAP_PATH}/BUILDINFO > /dev/null 2>&1 &&
cmp -s BUILDINFO.new BUILDINFO || mv BUILDINFO.new BUILDINFO &&
ftp -V ${SNAP_PATH}/SHA256.sig > /dev/null 2>&1 &&
for i in INSTALL.${arch} $BSD $SETS BUILDINFO
do
signify -q -C -p $KEY -x SHA256.sig $i > /dev/null 2>&1 ||
ftp -V ${SNAP_PATH}/${i} > /dev/null 2>&1
done

I don't think $KEY, $SNAP_PATH and $SETS require any explanation.

Regards,

Raf



Re: [patch] ftp(1): change mtime for http/https links

2017-09-23 Thread Jesper Wallin
Yeah, the SHA256.sig should be enough and just veify that everything is
correct.  If not, fetch the incorrect sets again.  I just wanted to give
a brief explaination on how I noticed this inconsistency.  But yeah,
thanks for pointing me to a better solution.


On Sat, Sep 23, 2017 at 05:12:32AM -0400, Ted Unangst wrote:
> Raf Czlonka wrote:
> > This is unrelated to your diff but what I do instead is to check
> > the BUILDINFO file - it's tiny and all the information you need,
> > is already there.
> 
> While we're on the subject, I'll point out that sometimes only the base sets
> change, but not the x sets. If you download SHA256 you can download only files
> that differ. (This also may work better if you switch mirrors or the
> timestamps drift for other reasons.) It's just a few lines of shell, though I
> don't have them handy atm.
> 
> (And for bonus points, you can use SHA256.sig and verify it's correct, then
> verify that BUILDINFO hasn't been tampered with, etc...)
> 
> ftp/http should still do the right thing, of course.



Re: [patch] ftp(1): change mtime for http/https links

2017-09-23 Thread Ted Unangst
Raf Czlonka wrote:
> This is unrelated to your diff but what I do instead is to check
> the BUILDINFO file - it's tiny and all the information you need,
> is already there.

While we're on the subject, I'll point out that sometimes only the base sets
change, but not the x sets. If you download SHA256 you can download only files
that differ. (This also may work better if you switch mirrors or the
timestamps drift for other reasons.) It's just a few lines of shell, though I
don't have them handy atm.

(And for bonus points, you can use SHA256.sig and verify it's correct, then
verify that BUILDINFO hasn't been tampered with, etc...)

ftp/http should still do the right thing, of course.



Re: [patch] ftp(1): change mtime for http/https links

2017-09-23 Thread Jesper Wallin
Yeah, fully understandable.  I'll bump this once 6.2 is out.

I'll apply the changes Philip suggested (thanks!) and then build a
release to verify that everything works.


On Fri, Sep 22, 2017 at 11:35:03PM +0100, Stuart Henderson wrote:
> I like this, but it's rather late in the release cycle to be adding
> features to ftp(1), could you send a reminder when we're at 6.2-current?
> (Also, if you haven't already done so, check that "make release" and the
> installer still work).
> 



Re: [patch] ftp(1): change mtime for http/https links

2017-09-23 Thread Anton Lindqvist
On Fri, Sep 22, 2017 at 11:56:51PM +0100, Raf Czlonka wrote:
> On Fri, Sep 22, 2017 at 11:01:57PM BST, Jesper Wallin wrote:
> > Hi all,
> > 
> > My morning routine consists of downloading the latest snapshot files and
> > running the upgrade.  To avoid wasting bandwidth and time, I check the
> > local modification time of INSTALL.amd64, fetch the remote one and check
> > if the modification time has changed.
> 
> Hi Jesper,
> 
> This is unrelated to your diff but what I do instead is to check
> the BUILDINFO file - it's tiny and all the information you need,
> is already there.

Continuing OT: I also rely on the contents of BUILDINFO to determine if
a new snapshot is available:

  $ head `which snapshot`
  #!/bin/sh

  url="$(head -1 /etc/installurl)/snapshots/$(uname -m)"

  [ -e BUILDINFO ] && ftp -V -o- "${url}/BUILDINFO" | cmp -s BUILDINFO - && 
exit 1



Re: [patch] ftp(1): change mtime for http/https links

2017-09-22 Thread Philip Guenther
On Sat, 23 Sep 2017, Jesper Wallin wrote:
...
> The patch below will use the Last-Modified header in order to set the
> modification time for http or https links.  I also added, what to me
> looked like a missing "\n" on the error message in ftp.c.
...
> --- fetch.c   7 Mar 2017 08:00:23 -   1.163
> +++ fetch.c   22 Sep 2017 19:52:49 -
...
> @@ -1043,8 +1050,22 @@ cleanup_url_get:
>   fclose(fin);
>   else if (s != -1)
>   close(s);
> - if (out >= 0 && out != fileno(stdout))
> + if (out >= 0 && out != fileno(stdout)) {
> +
> + if (mtime != -1) {
> + struct timeval tv[2];
> + tv[0].tv_sec = time(NULL);
> + tv[0].tv_usec = tv[1].tv_usec = 0;
> + tv[1].tv_sec = mtime;
> +
> + if (futimes(out, tv) == -1)

I would avoid the time() call and use UTIME_NOW and futimens() instead:
if (mtime != -1) {
struct timespec ts[2];

ts[0].tv_nsec = UTIME_NOW;
ts[1].tv_sec = mtime;
tv[1].tv_nsec = 0;
if (futimens(out, ts) == -1)


> + fprintf(ttyout,
> + "Can't change modification time on %s to %s\n",
> + savefile, asctime(localtime(&mtime)));

asctime(localtime(x)) == ctime(x)


Philip Guenther



Re: [patch] ftp(1): change mtime for http/https links

2017-09-22 Thread Raf Czlonka
On Fri, Sep 22, 2017 at 11:01:57PM BST, Jesper Wallin wrote:
> Hi all,
> 
> My morning routine consists of downloading the latest snapshot files and
> running the upgrade.  To avoid wasting bandwidth and time, I check the
> local modification time of INSTALL.amd64, fetch the remote one and check
> if the modification time has changed.

Hi Jesper,

This is unrelated to your diff but what I do instead is to check
the BUILDINFO file - it's tiny and all the information you need,
is already there.

Cheers,

Raf
 
> A few days ago, the mirror I use had issues with the ftpd.  I quickly
> switched the ftp:// to http:// and continued with my routine.  By doing
> so, I noticed that when running ftp(1) on ftp links, it does preserve
> the modification time.  But for http or https links, the modification
> time is ignored.
> 
> I noticed the 'preserve' option is enabled by default, even when using
> the auto-fetch feature.  I find this behaviour a bit inconsistent and
> unintuitive, seeing it won't let the user specify whether or not the
> preserve option should be enabled.  Yet, it behaves differently
> depending on what protocol we use to fetch the file.
> 
> The patch below will use the Last-Modified header in order to set the
> modification time for http or https links.  I also added, what to me
> looked like a missing "\n" on the error message in ftp.c.
> 
> An alternative solution might be to have the preserve option disabled
> by default when auto-fetching files.
> 
> 
> Jesper Wallin
> 
> 
> Index: fetch.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
> retrieving revision 1.163
> diff -u -p -r1.163 fetch.c
> --- fetch.c   7 Mar 2017 08:00:23 -   1.163
> +++ fetch.c   22 Sep 2017 19:52:49 -
> @@ -210,6 +210,7 @@ url_get(const char *origline, const char
>   int status;
>   int save_errno;
>   const size_t buflen = 128 * 1024;
> + time_t mtime = -1;
>  
>   direction = "received";
>  
> @@ -860,6 +861,12 @@ noslash:
>   if (restart_point)
>   filesize += restart_point;
>  #endif /* !SMALL */
> +#define LASTMOD "Last-Modified: "
> + } else if (strncasecmp(cp, LASTMOD, sizeof(LASTMOD) - 1) == 0) {
> + struct tm tm;
> + cp += sizeof(LASTMOD) - 1;
> + if (strptime(cp, "%a, %d %b %Y %T %z", &tm) != NULL)
> + mtime = mktime(&tm);
>  #define LOCATION "Location: "
>   } else if (isredirect &&
>   strncasecmp(cp, LOCATION, sizeof(LOCATION) - 1) == 0) {
> @@ -1043,8 +1050,22 @@ cleanup_url_get:
>   fclose(fin);
>   else if (s != -1)
>   close(s);
> - if (out >= 0 && out != fileno(stdout))
> + if (out >= 0 && out != fileno(stdout)) {
> +
> + if (mtime != -1) {
> + struct timeval tv[2];
> + tv[0].tv_sec = time(NULL);
> + tv[0].tv_usec = tv[1].tv_usec = 0;
> + tv[1].tv_sec = mtime;
> +
> + if (futimes(out, tv) == -1)
> + fprintf(ttyout,
> + "Can't change modification time on %s to %s\n",
> + savefile, asctime(localtime(&mtime)));
> + }
> +
>   close(out);
> + }
>   free(buf);
>   free(proxyhost);
>   free(proxyurl);
> Index: ftp.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
> retrieving revision 1.100
> diff -u -p -r1.100 ftp.c
> --- ftp.c 22 Aug 2016 16:27:00 -  1.100
> +++ ftp.c 22 Sep 2017 19:52:50 -
> @@ -1217,7 +1217,7 @@ break2:
>   ut.modtime = mtime;
>   if (utime(local, &ut) == -1)
>   fprintf(ttyout,
> - "Can't change modification time on %s to %s",
> + "Can't change modification time on %s to %s\n",
>   local, asctime(localtime(&mtime)));
>   }
>   }
> 



Re: [patch] ftp(1): change mtime for http/https links

2017-09-22 Thread Stuart Henderson
I like this, but it's rather late in the release cycle to be adding
features to ftp(1), could you send a reminder when we're at 6.2-current?
(Also, if you haven't already done so, check that "make release" and the
installer still work).



On 2017/09/22 22:01, Jesper Wallin wrote:
> Hi all,
> 
> My morning routine consists of downloading the latest snapshot files and
> running the upgrade.  To avoid wasting bandwidth and time, I check the
> local modification time of INSTALL.amd64, fetch the remote one and check
> if the modification time has changed.
> 
> A few days ago, the mirror I use had issues with the ftpd.  I quickly
> switched the ftp:// to http:// and continued with my routine.  By doing
> so, I noticed that when running ftp(1) on ftp links, it does preserve
> the modification time.  But for http or https links, the modification
> time is ignored.
> 
> I noticed the 'preserve' option is enabled by default, even when using
> the auto-fetch feature.  I find this behaviour a bit inconsistent and
> unintuitive, seeing it won't let the user specify whether or not the
> preserve option should be enabled.  Yet, it behaves differently
> depending on what protocol we use to fetch the file.
> 
> The patch below will use the Last-Modified header in order to set the
> modification time for http or https links.  I also added, what to me
> looked like a missing "\n" on the error message in ftp.c.
> 
> An alternative solution might be to have the preserve option disabled
> by default when auto-fetching files.
> 
> 
> Jesper Wallin
> 
> 
> Index: fetch.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
> retrieving revision 1.163
> diff -u -p -r1.163 fetch.c
> --- fetch.c   7 Mar 2017 08:00:23 -   1.163
> +++ fetch.c   22 Sep 2017 19:52:49 -
> @@ -210,6 +210,7 @@ url_get(const char *origline, const char
>   int status;
>   int save_errno;
>   const size_t buflen = 128 * 1024;
> + time_t mtime = -1;
>  
>   direction = "received";
>  
> @@ -860,6 +861,12 @@ noslash:
>   if (restart_point)
>   filesize += restart_point;
>  #endif /* !SMALL */
> +#define LASTMOD "Last-Modified: "
> + } else if (strncasecmp(cp, LASTMOD, sizeof(LASTMOD) - 1) == 0) {
> + struct tm tm;
> + cp += sizeof(LASTMOD) - 1;
> + if (strptime(cp, "%a, %d %b %Y %T %z", &tm) != NULL)
> + mtime = mktime(&tm);
>  #define LOCATION "Location: "
>   } else if (isredirect &&
>   strncasecmp(cp, LOCATION, sizeof(LOCATION) - 1) == 0) {
> @@ -1043,8 +1050,22 @@ cleanup_url_get:
>   fclose(fin);
>   else if (s != -1)
>   close(s);
> - if (out >= 0 && out != fileno(stdout))
> + if (out >= 0 && out != fileno(stdout)) {
> +
> + if (mtime != -1) {
> + struct timeval tv[2];
> + tv[0].tv_sec = time(NULL);
> + tv[0].tv_usec = tv[1].tv_usec = 0;
> + tv[1].tv_sec = mtime;
> +
> + if (futimes(out, tv) == -1)
> + fprintf(ttyout,
> + "Can't change modification time on %s to %s\n",
> + savefile, asctime(localtime(&mtime)));
> + }
> +
>   close(out);
> + }
>   free(buf);
>   free(proxyhost);
>   free(proxyurl);
> Index: ftp.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
> retrieving revision 1.100
> diff -u -p -r1.100 ftp.c
> --- ftp.c 22 Aug 2016 16:27:00 -  1.100
> +++ ftp.c 22 Sep 2017 19:52:50 -
> @@ -1217,7 +1217,7 @@ break2:
>   ut.modtime = mtime;
>   if (utime(local, &ut) == -1)
>   fprintf(ttyout,
> - "Can't change modification time on %s to %s",
> + "Can't change modification time on %s to %s\n",
>   local, asctime(localtime(&mtime)));
>   }
>   }
> 



[patch] ftp(1): change mtime for http/https links

2017-09-22 Thread Jesper Wallin
Hi all,

My morning routine consists of downloading the latest snapshot files and
running the upgrade.  To avoid wasting bandwidth and time, I check the
local modification time of INSTALL.amd64, fetch the remote one and check
if the modification time has changed.

A few days ago, the mirror I use had issues with the ftpd.  I quickly
switched the ftp:// to http:// and continued with my routine.  By doing
so, I noticed that when running ftp(1) on ftp links, it does preserve
the modification time.  But for http or https links, the modification
time is ignored.

I noticed the 'preserve' option is enabled by default, even when using
the auto-fetch feature.  I find this behaviour a bit inconsistent and
unintuitive, seeing it won't let the user specify whether or not the
preserve option should be enabled.  Yet, it behaves differently
depending on what protocol we use to fetch the file.

The patch below will use the Last-Modified header in order to set the
modification time for http or https links.  I also added, what to me
looked like a missing "\n" on the error message in ftp.c.

An alternative solution might be to have the preserve option disabled
by default when auto-fetching files.


Jesper Wallin


Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.163
diff -u -p -r1.163 fetch.c
--- fetch.c 7 Mar 2017 08:00:23 -   1.163
+++ fetch.c 22 Sep 2017 19:52:49 -
@@ -210,6 +210,7 @@ url_get(const char *origline, const char
int status;
int save_errno;
const size_t buflen = 128 * 1024;
+   time_t mtime = -1;
 
direction = "received";
 
@@ -860,6 +861,12 @@ noslash:
if (restart_point)
filesize += restart_point;
 #endif /* !SMALL */
+#define LASTMOD "Last-Modified: "
+   } else if (strncasecmp(cp, LASTMOD, sizeof(LASTMOD) - 1) == 0) {
+   struct tm tm;
+   cp += sizeof(LASTMOD) - 1;
+   if (strptime(cp, "%a, %d %b %Y %T %z", &tm) != NULL)
+   mtime = mktime(&tm);
 #define LOCATION "Location: "
} else if (isredirect &&
strncasecmp(cp, LOCATION, sizeof(LOCATION) - 1) == 0) {
@@ -1043,8 +1050,22 @@ cleanup_url_get:
fclose(fin);
else if (s != -1)
close(s);
-   if (out >= 0 && out != fileno(stdout))
+   if (out >= 0 && out != fileno(stdout)) {
+
+   if (mtime != -1) {
+   struct timeval tv[2];
+   tv[0].tv_sec = time(NULL);
+   tv[0].tv_usec = tv[1].tv_usec = 0;
+   tv[1].tv_sec = mtime;
+
+   if (futimes(out, tv) == -1)
+   fprintf(ttyout,
+   "Can't change modification time on %s to %s\n",
+   savefile, asctime(localtime(&mtime)));
+   }
+
close(out);
+   }
free(buf);
free(proxyhost);
free(proxyurl);
Index: ftp.c
===
RCS file: /cvs/src/usr.bin/ftp/ftp.c,v
retrieving revision 1.100
diff -u -p -r1.100 ftp.c
--- ftp.c   22 Aug 2016 16:27:00 -  1.100
+++ ftp.c   22 Sep 2017 19:52:50 -
@@ -1217,7 +1217,7 @@ break2:
ut.modtime = mtime;
if (utime(local, &ut) == -1)
fprintf(ttyout,
-   "Can't change modification time on %s to %s",
+   "Can't change modification time on %s to %s\n",
local, asctime(localtime(&mtime)));
}
}