From 90089cba0a216c042d24650e8cfb826b4b6d656f Mon Sep 17 00:00:00 2001 From: makeworld Date: Thu, 17 Dec 2020 11:29:03 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Downloading=20re-uses=20request?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #140 --- CHANGELOG.md | 3 +- client/client.go | 15 --------- display/handlers.go | 46 ++++++++++++------------- go.mod | 2 +- go.sum | 4 +-- rr/README.md | 37 ++++++++++++++++++++ rr/rr.go | 82 +++++++++++++++++++++++++++++++++++++++++++++ rr/rr_test.go | 44 ++++++++++++++++++++++++ 8 files changed, 189 insertions(+), 44 deletions(-) create mode 100644 rr/README.md create mode 100644 rr/rr.go create mode 100644 rr/rr_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index bc886f8..74c1c34 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/client/client.go b/client/client.go index 42e9777..f216fcc 100644 --- a/client/client.go +++ b/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) -} diff --git a/display/handlers.go b/display/handlers.go index c211d1e..856c7e8 100644 --- a/display/handlers.go +++ b/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) } diff --git a/go.mod b/go.mod index 70b1468..5dab22f 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 8c7bee8..8368eef 100644 --- a/go.sum +++ b/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= diff --git a/rr/README.md b/rr/README.md new file mode 100644 index 0000000..a6ce3cb --- /dev/null +++ b/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. + +
+Click to see MIT license terms + +``` +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. +``` +
diff --git a/rr/rr.go b/rr/rr.go new file mode 100644 index 0000000..a9b84c5 --- /dev/null +++ b/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), + } +} diff --git a/rr/rr_test.go b/rr/rr_test.go new file mode 100644 index 0000000..1ee52cc --- /dev/null +++ b/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") +}