From c2c5abd1decf59363703bb4cd07165d24a0846f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Mon, 17 Apr 2017 12:52:27 -0700 Subject: [PATCH] Fix transport package Transport refactoring introduced a bug where HTTP(S) response body is closed before it's fully read (depending on whenever gzip is used or not), this change fixes it and makes the code easier to follow by removing duplicated code and enforcing all transport packages to implement ReaderCloser interface. --- transport/file.go | 18 +++++++++++++++--- transport/http.go | 7 ++----- transport/transport.go | 34 +++++++++++++--------------------- 3 files changed, 30 insertions(+), 29 deletions(-) diff --git a/transport/file.go b/transport/file.go index 8fd726703..3d3f9c9db 100644 --- a/transport/file.go +++ b/transport/file.go @@ -1,6 +1,7 @@ package transport import ( + "io" "os" log "github.com/Sirupsen/logrus" @@ -8,9 +9,20 @@ import ( type fileReader struct { filename string + fd *os.File } -func newFileReader(filname string) (*os.File, error) { - log.Infof("Reading file '%s'", filname) - return os.Open(filname) +func (fr *fileReader) Read(b []byte) (n int, err error) { + return fr.fd.Read(b) +} + +func (fr *fileReader) Close() error { + return fr.fd.Close() +} + +func newFileReader(filname string) (io.ReadCloser, error) { + log.Infof("Reading file '%s'", filname) + fd, err := os.Open(filname) + fr := fileReader{filename: filname, fd: fd} + return &fr, err } diff --git a/transport/http.go b/transport/http.go index 875746ec8..d8c13213f 100644 --- a/transport/http.go +++ b/transport/http.go @@ -15,7 +15,7 @@ type httpReader struct { Timeout time.Duration } -func newHTTPReader(url string, timeout time.Duration) (*io.ReadCloser, error) { +func newHTTPReader(url string, timeout time.Duration) (io.ReadCloser, error) { hr := httpReader{URL: url, Timeout: timeout} log.Infof("GET %s timeout=%s", hr.URL, hr.Timeout) @@ -38,8 +38,6 @@ func newHTTPReader(url string, timeout time.Duration) (*io.ReadCloser, error) { return nil, fmt.Errorf("Request to Alertmanager failed with %s", resp.Status) } - defer resp.Body.Close() - var reader io.ReadCloser switch resp.Header.Get("Content-Encoding") { case "gzip": @@ -47,9 +45,8 @@ func newHTTPReader(url string, timeout time.Duration) (*io.ReadCloser, error) { if err != nil { return nil, fmt.Errorf("Failed to decode gzipped content: %s", err.Error()) } - defer reader.Close() default: reader = resp.Body } - return &reader, nil + return reader, nil } diff --git a/transport/transport.go b/transport/transport.go index 1a0a4025f..c7f66afb8 100644 --- a/transport/transport.go +++ b/transport/transport.go @@ -3,37 +3,29 @@ package transport import ( "encoding/json" "fmt" + "io" "net/url" "time" ) -func readFile(filename string, target interface{}) error { - reader, err := newFileReader(filename) - if err != nil { - return err - } - return json.NewDecoder(reader).Decode(target) -} - -func readHTTP(url string, timeout time.Duration, target interface{}) error { - reader, err := newHTTPReader(url, timeout) - if err != nil { - return err - } - return json.NewDecoder(*reader).Decode(target) -} - // ReadJSON using one of supported transports (file:// http://) func ReadJSON(uri string, timeout time.Duration, target interface{}) error { u, err := url.Parse(uri) if err != nil { return err } - if u.Scheme == "file" { - return readFile(u.Path, target) + var reader io.ReadCloser + switch u.Scheme { + case "http", "https": + reader, err = newHTTPReader(u.String(), timeout) + case "file": + reader, err = newFileReader(u.Path) + default: + return fmt.Errorf("Unsupported URI scheme '%s' in '%s'", u.Scheme, u) } - if u.Scheme == "http" || u.Scheme == "https" { - return readHTTP(u.String(), timeout, target) + if err != nil { + return err } - return fmt.Errorf("Unsupported URI scheme '%s' in '%s'", u.Scheme, u) + defer reader.Close() + return json.NewDecoder(reader).Decode(target) }