🐛 Downloading re-uses request

Fixes #140

CHANGELOG.md

@@ -11,10 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `show_link` option added in config to optionally see the URL (#133)
 
 ### Changed
-- Updated [go-gemini](https://github.com/makeworld-the-better-one/go-gemini) to v0.9.3
+- Updated [go-gemini](https://github.com/makeworld-the-better-one/go-gemini) to v0.10.0
   - Supports CN-only wildcard certs
   - Time out when header takes too long
 - Preformatted text is now light yellow by default
+- Downloading a file no longer uses a second request
 
 ### Fixed
 - Single quotes are used in the default config for commands and paths so that Windows paths with backslashes will be parsed correctly

client/client.go

@@ -18,7 +18,6 @@ var (
 	certCacheMu = &sync.RWMutex{}
 
 	fetchClient *gemini.Client
-	dlClient    *gemini.Client // For downloading
 )
 
 func Init() {
@@ -26,10 +25,6 @@ func Init() {
 		ConnectTimeout: 10 * time.Second, // Default is 15
 		ReadTimeout:    time.Duration(viper.GetInt("a-general.page_max_time")) * time.Second,
 	}
-	dlClient = &gemini.Client{
-		ConnectTimeout: 10 * time.Second, // Default is 15
-		// No read timeout, download can take as long as it needs
-	}
 }
 
 func clientCert(host string) ([]byte, []byte) {
@@ -112,11 +107,6 @@ func Fetch(u string) (*gemini.Response, error) {
 	return fetch(u, fetchClient)
 }
 
-// Download is the same as Fetch but with no read timeout.
-func Download(u string) (*gemini.Response, error) {
-	return fetch(u, dlClient)
-}
-
 func fetchWithProxy(proxyHostname, proxyPort, u string, c *gemini.Client) (*gemini.Response, error) {
 	parsed, _ := url.Parse(u)
 	cert, key := clientCert(parsed.Host)
@@ -145,8 +135,3 @@ func fetchWithProxy(proxyHostname, proxyPort, u string, c *gemini.Client) (*gemi
 func FetchWithProxy(proxyHostname, proxyPort, u string) (*gemini.Response, error) {
 	return fetchWithProxy(proxyHostname, proxyPort, u, fetchClient)
 }
-
-// DownloadWithProxy is the same as FetchWithProxy but with no read timeout.
-func DownloadWithProxy(proxyHostname, proxyPort, u string) (*gemini.Response, error) {
-	return fetchWithProxy(proxyHostname, proxyPort, u, dlClient)
-}

display/handlers.go

@@ -15,6 +15,7 @@ import (
 	"github.com/makeworld-the-better-one/amfora/client"
 	"github.com/makeworld-the-better-one/amfora/config"
 	"github.com/makeworld-the-better-one/amfora/renderer"
+	"github.com/makeworld-the-better-one/amfora/rr"
 	"github.com/makeworld-the-better-one/amfora/structs"
 	"github.com/makeworld-the-better-one/amfora/subscriptions"
 	"github.com/makeworld-the-better-one/amfora/webbrowser"
@@ -362,6 +363,10 @@ func handleURL(t *tab, u string, numRedirects int) (string, bool) {
 		Error("URL Fetch Error", err.Error())
 		return ret("", false)
 	}
+
+	// Fetch happened successfully, use RestartReader to buffer read data
+	res.Body = rr.NewRestartReader(res.Body)
+
 	if renderer.CanDisplay(res) {
 		page, err := renderer.MakePage(u, res, textWidth(), leftMargin(), usingProxy)
 		// Rendering may have taken a while, make sure tab is still valid
@@ -369,35 +374,20 @@ func handleURL(t *tab, u string, numRedirects int) (string, bool) {
 			return ret("", false)
 		}
 
-		var res2 *gemini.Response
-		var dlErr error
-
 		if errors.Is(err, renderer.ErrTooLarge) {
-			// Make new request for downloading purposes
-			if usingProxy {
-				res2, dlErr = client.DownloadWithProxy(proxyHostname, proxyPort, u)
-			} else {
-				res2, dlErr = client.Download(u)
-			}
-			if dlErr != nil && !errors.Is(dlErr, client.ErrTofu) {
-				Error("URL Fetch Error", err.Error())
-				return ret("", false)
-			}
-			go dlChoice("That page is too large. What would you like to do?", u, res2)
+			// Downloading now
+			// Disable read timeout and go back to start
+			res.SetReadTimeout(0)
+			res.Body.(*rr.RestartReader).Restart()
+			go dlChoice("That page is too large. What would you like to do?", u, res)
 			return ret("", false)
 		}
 		if errors.Is(err, renderer.ErrTimedOut) {
-			// Make new request for downloading purposes
-			if usingProxy {
-				res2, dlErr = client.DownloadWithProxy(proxyHostname, proxyPort, u)
-			} else {
-				res2, dlErr = client.Download(u)
-			}
-			if dlErr != nil && !errors.Is(dlErr, client.ErrTofu) {
-				Error("URL Fetch Error", err.Error())
-				return ret("", false)
-			}
-			go dlChoice("Loading that page timed out. What would you like to do?", u, res2)
+			// Downloading now
+			// Disable read timeout and go back to start
+			res.SetReadTimeout(0)
+			res.Body.(*rr.RestartReader).Restart()
+			go dlChoice("Loading that page timed out. What would you like to do?", u, res)
 			return ret("", false)
 		}
 		if err != nil {
@@ -510,6 +500,9 @@ func handleURL(t *tab, u string, numRedirects int) (string, bool) {
 			added := addFeedDirect(u, feed, subscriptions.IsSubscribed(u))
 			if !added {
 				// Otherwise offer download choices
+				// Disable read timeout and go back to start
+				res.SetReadTimeout(0)
+				res.Body.(*rr.RestartReader).Restart()
 				go dlChoice("That file could not be displayed. What would you like to do?", u, res)
 			}
 		}()
@@ -517,6 +510,9 @@ func handleURL(t *tab, u string, numRedirects int) (string, bool) {
 	}
 
 	// Otherwise offer download choices
+	// Disable read timeout and go back to start
+	res.SetReadTimeout(0)
+	res.Body.(*rr.RestartReader).Restart()
 	go dlChoice("That file could not be displayed. What would you like to do?", u, res)
 	return ret("", false)
 }

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/fsnotify/fsnotify v1.4.9 // indirect
 	github.com/gdamore/tcell v1.3.1-0.20200608133353-cb1e5d6fa606
 	github.com/google/go-cmp v0.5.0 // indirect
-	github.com/makeworld-the-better-one/go-gemini v0.9.3
+	github.com/makeworld-the-better-one/go-gemini v0.10.0
 	github.com/makeworld-the-better-one/go-isemoji v1.1.0
 	github.com/makeworld-the-better-one/progressbar/v3 v3.3.5-0.20200710151429-125743e22b4f
 	github.com/mitchellh/go-homedir v1.1.0

go.sum

@@ -133,8 +133,8 @@ github.com/lucasb-eyer/go-colorful v1.0.3 h1:QIbQXiugsb+q10B+MI+7DI1oQLdmnep86tW
 github.com/lucasb-eyer/go-colorful v1.0.3/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0=
 github.com/magiconair/properties v1.8.1 h1:ZC2Vc7/ZFkGmsVC9KvOjumD+G5lXy2RtTKyzRKO2BQ4=
 github.com/magiconair/properties v1.8.1/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ=
-github.com/makeworld-the-better-one/go-gemini v0.9.3 h1:vpJc1u4LYpEI5h7GcOE2zSfOmpE9gQzt0vEayp/ilWc=
-github.com/makeworld-the-better-one/go-gemini v0.9.3/go.mod h1:P7/FbZ+IEIbA/d+A0Y3w2GNgD8SA2AcNv7aDGJbaWG4=
+github.com/makeworld-the-better-one/go-gemini v0.10.0 h1:MZYuGD5RcjXD5k+gZLKOs1djKaPDaQrdY0OhdIh637c=
+github.com/makeworld-the-better-one/go-gemini v0.10.0/go.mod h1:P7/FbZ+IEIbA/d+A0Y3w2GNgD8SA2AcNv7aDGJbaWG4=
 github.com/makeworld-the-better-one/go-isemoji v1.1.0 h1:wZBHOKB5zAIgaU2vaWnXFDDhatebB8TySrNVxjVV84g=
 github.com/makeworld-the-better-one/go-isemoji v1.1.0/go.mod h1:FBjkPl9rr0G4vlZCc+Mr+QcnOfGCTbGWYW8/1sp06I0=
 github.com/makeworld-the-better-one/gofeed v1.1.1-0.20201123002655-c0c6354134fe h1:i3b9Qy5z23DcXRnrsMYcM5s9Ng5VIidM1xZd+szuTsY=

rr/README.md

@@ -0,0 +1,37 @@
+# package `rr`, aka `RestartReader`
+
+This package exists just to hold the `RestartReader` type. It wraps `io.ReadCloser` and implements it. It holds the data from every `Read` in a `[]byte` buffer, and allows you to call `.Restart()`, causing subsequent `Read` calls to start from the beginning again.
+
+See [#140](https://github.com/makeworld-the-better-one/amfora/issues/140) for why this was needed.
+
+Other projects are encouraged to copy this code if it's useful to them, and this package may move out of Amfora if I end up using it in multiple projects.
+
+## License
+
+If you prefer, you can consider the code in this package, and this package only, to be licensed under the MIT license instead.
+
+<details>
+<summary>Click to see MIT license terms</summary>
+
+```
+Copyright (c) 2020 makeworld
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.
+```
+</details>

rr/rr.go

@@ -0,0 +1,82 @@
+package rr
+
+import (
+	"errors"
+	"io"
+)
+
+// ErrTooLarge is passed to panic if memory cannot be allocated to store data in a buffer.
+var ErrClosed = errors.New("RestartReader: closed")
+
+type RestartReader struct {
+	r   io.ReadCloser
+	buf []byte
+
+	// Where in the buffer we are. If it's equal to len(buf) then the reader
+	// should be used.
+	i int64
+}
+
+func (rr *RestartReader) Read(p []byte) (n int, err error) {
+	if rr.buf == nil {
+		return 0, ErrClosed
+	}
+
+	if rr.i >= int64(len(rr.buf)) {
+		// Read new data
+		tmp := make([]byte, len(p))
+		n, err = rr.r.Read(tmp)
+		if n > 0 {
+			rr.buf = append(rr.buf, tmp[:n]...)
+			copy(p, tmp[:n])
+		}
+		rr.i = int64(len(rr.buf))
+		return
+	}
+
+	// Reading from buffer
+
+	bufSize := len(rr.buf[rr.i:])
+
+	if len(p) > bufSize {
+		// It wants more data then what's in the buffer
+		tmp := make([]byte, len(p)-bufSize)
+		n, err = rr.r.Read(tmp)
+		if n > 0 {
+			rr.buf = append(rr.buf, tmp[:n]...)
+		}
+		copy(p, rr.buf[rr.i:])
+		n += bufSize
+		rr.i = int64(len(rr.buf))
+		return
+	}
+	// All the required data is in the buffer
+	end := rr.i + int64(len(p))
+	copy(p, rr.buf[rr.i:end])
+	rr.i = end
+	n = len(p)
+	err = nil
+	return
+}
+
+// Restart causes subsequent Read calls to read from the beginning, instead
+// of where they left off.
+func (rr *RestartReader) Restart() {
+	rr.i = 0
+}
+
+// Close clears the buffer and closes the underlying io.ReadCloser, returning
+// its error.
+func (rr *RestartReader) Close() error {
+	rr.buf = nil
+	return rr.r.Close()
+}
+
+// NewRestartReader creates and initializes a new RestartReader that reads from
+// the provided io.ReadCloser.
+func NewRestartReader(r io.ReadCloser) *RestartReader {
+	return &RestartReader{
+		r:   r,
+		buf: make([]byte, 0),
+	}
+}

rr/rr_test.go

@@ -0,0 +1,44 @@
+package rr
+
+import (
+	"io/ioutil"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+var r1 *RestartReader
+
+func reset() {
+	r1 = NewRestartReader(ioutil.NopCloser(strings.NewReader("1234567890")))
+}
+
+func TestRead(t *testing.T) {
+	reset()
+	p := make([]byte, 1)
+	n, err := r1.Read(p)
+	assert.Equal(t, 1, n, "should read one byte")
+	assert.Equal(t, nil, err, "should be no error")
+	assert.Equal(t, []byte{'1'}, p, "should have read one byte, '1'")
+}
+
+func TestRestart(t *testing.T) {
+	reset()
+	p := make([]byte, 4)
+	r1.Read(p)
+
+	r1.Restart()
+	p = make([]byte, 5)
+	n, err := r1.Read(p)
+	assert.Equal(t, []byte("12345"), p, "should read the first 5 bytes again")
+	assert.Equal(t, 5, n, "should have read 4 bytes")
+	assert.Equal(t, nil, err, "err should be nil")
+
+	r1.Restart()
+	p = make([]byte, 4)
+	n, err = r1.Read(p)
+	assert.Equal(t, []byte("1234"), p, "should read the first 4 bytes again")
+	assert.Equal(t, 4, n, "should have read 4 bytes")
+	assert.Equal(t, nil, err, "err should be nil")
+}