Bug 224426 - fetch -i runs in an endless loop
Summary: fetch -i runs in an endless loop
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Dag-Erling Smørgrav
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2017-12-18 15:29 UTC by Wolfram Schneider
Modified: 2018-10-09 10:50 UTC (History)
5 users (show)

See Also:
des: mfc-stable11+


Attachments
proposed patch to fetch to avoid endless loop (606 bytes, patch)
2017-12-21 20:21 UTC, Zak
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wolfram Schneider freebsd_committer freebsd_triage 2017-12-18 15:29:32 UTC
I wanted to try out the -i option of the fetch(1) command for the build of our website.

     -i file, --if-modified-since=file
                 If-Modified-Since mode: the remote file will only be
                 retrieved if it is newer than file on the local host.  (HTTP
                 only)

I also had the -a option in use. I run into an endless loop.

how to repeat:

# first run, file is not up-to-date
$ touch index.html
$ time fetch -a -o index.html -i index.html https://www.FreeBSD.org 

real	0m0.887s
user	0m0.019s
sys	0m0.016s

# second run, file is up-to-date
$ time fetch -a -o index.html -i index.html https://www.FreeBSD.org
time fetch -a -o index.html -i index.html https://www.FreeBSD.org         
fetch: https://www.FreeBSD.org: Not Modified
fetch: https://www.FreeBSD.org: Not Modified
fetch: https://www.FreeBSD.org: Not Modified
fetch: https://www.FreeBSD.org: Not Modified
fetch: https://www.FreeBSD.org: Not Modified
fetch: https://www.FreeBSD.org: Not Modified
fetch: https://www.FreeBSD.org: Not Modified
fetch: https://www.FreeBSD.org: Not Modified
fetch: https://www.FreeBSD.org: Not Modified
fetch: https://www.FreeBSD.org: Not Modified
^C
Comment 1 Zak 2017-12-21 20:21:15 UTC
Created attachment 189014 [details]
proposed patch to fetch to avoid endless loop

Not sure if this is the best solution, but it seems to work for the case you've encountered. It looks like the issue is due to HTTP and HTTPS being treated slightly differently. I don't know if this is indicative of a deeper problem though.
Comment 2 Wolfram Schneider freebsd_committer freebsd_triage 2017-12-22 11:57:12 UTC
(In reply to Zak from comment #1)

The manpage says: HTTP only

If this is still try, then I expect a fatal error: HTTPS not supported for -i
Comment 3 Wolfram Schneider freebsd_committer freebsd_triage 2017-12-22 12:03:29 UTC
(In reply to Wolfram Schneider from comment #2)

s/try/true/
Comment 4 Wolfram Schneider freebsd_committer freebsd_triage 2017-12-22 12:03:48 UTC
It is a little bit more complicated than I thought. Some websites run in an endless loop, others not:

endless loop:

fetch -a -o index.html -i index.html https://www.nytimes.com
fetch -a -o index.html -i index.html https://www.washingtonpost.com
fetch -a -o index.html -i index.html https://www.freebsd.org
fetch -a -o index.html -i index.html https://www.fsf.org/
fetch -a -o index.html -i index.html https://www.openbsd.org

no endless loop:

fetch -a -o index.html -i index.html https://www.apple.com
fetch -a -o index.html -i index.html https://duckduckgo.com
fetch -a -o index.html -i index.html https://www.google.com
fetch -a -o index.html -i index.html https://www.theguardian.com
Comment 5 Zak 2017-12-22 18:05:26 UTC
(In reply to Wolfram Schneider from comment #4)
Is the inconsistent looping behaviour present with my patch applied? I'm not at a proper computer so I can't check right now.
Comment 6 Heqing Yan 2017-12-22 20:54:16 UTC
(In reply to Wolfram Schneider from comment #4)
(In reply to Zak from comment #5)
I applied Zak's patch and cannot reproduce it. Did you test Zak's patch?
Comment 7 Wolfram Schneider freebsd_committer freebsd_triage 2017-12-23 08:51:37 UTC
(In reply to Heqing Yan from comment #6)

Zak's patch works fine for all web sites.
Comment 8 Dag-Erling Smørgrav freebsd_committer freebsd_triage 2018-05-12 17:00:16 UTC
The patch works, but uncovers a different issue: the “If-Modified-Since” element of the request is not preserved across redirects.  I will fix both.
Comment 9 commit-hook freebsd_committer freebsd_triage 2018-05-12 17:02:50 UTC
A commit references this bug:

Author: des
Date: Sat May 12 17:02:28 UTC 2018
New revision: 333571
URL: https://svnweb.freebsd.org/changeset/base/333571

Log:
  Preserve if-modified-since timestamps across redirects.

  PR:		224426
  MFC after:	1 week

Changes:
  head/lib/libfetch/http.c
Comment 10 commit-hook freebsd_committer freebsd_triage 2018-05-12 17:04:55 UTC
A commit references this bug:

Author: des
Date: Sat May 12 17:04:40 UTC 2018
New revision: 333572
URL: https://svnweb.freebsd.org/changeset/base/333572

Log:
  Support If-Modified-Since for https as well as http.

  PR:		224426
  Submitted by:	zsnafzig@edu.uwaterloo.ca
  MFC after:	1 week

Changes:
  head/usr.bin/fetch/fetch.c
Comment 11 commit-hook freebsd_committer freebsd_triage 2018-10-09 10:49:37 UTC
A commit references this bug:

Author: des
Date: Tue Oct  9 10:49:21 UTC 2018
New revision: 339250
URL: https://svnweb.freebsd.org/changeset/base/339250

Log:
  MFH (r314778): use reallocarray(3) for extra bounds checks
  MFH (r333306): fix typo in man page
  MFH (r333571, r333572): preserve if-modified-since across redirects
  MFH (r334317): simplify the DEBUG macro
  MFH (r334319): style bug roundup
  MFH (r334326): fix netrc file location logic, improve netrcfd handling
  MFH (r338572): fix end-of-transfer statistics, improve no-tty display

  PR:		202424, 224426, 228017

Changes:
_U  stable/11/
  stable/11/lib/libfetch/common.c
  stable/11/lib/libfetch/common.h
  stable/11/lib/libfetch/fetch.c
  stable/11/lib/libfetch/ftp.c
  stable/11/lib/libfetch/http.c
  stable/11/usr.bin/fetch/fetch.1
  stable/11/usr.bin/fetch/fetch.c