From 698633cd9596b34f1cbdb1b6114619a19cde0180 Mon Sep 17 00:00:00 2001 From: siddharth0a <99310385+siddharth0a@users.noreply.github.com> Date: Mon, 19 Aug 2024 10:38:12 +0900 Subject: [PATCH] feat: ensure proper closing of gzip and file writers/readers to prevent resource leaks (#11475) * add gzipCloser struct * add Close method * fix OpenDecompressed func * fix CompressByFileType func * knit * Apply suggestions from code review close both even if one fails Co-authored-by: Adrian Sutton * fix WriteCloser Close method * fix name for more general * fix writercloser name for more geneeral * add construction function for WrappedCloser * using construction func * seperate wrapped closer struct to wrapped_closer.go --------- Co-authored-by: Adrian Sutton --- op-service/ioutil/gzip.go | 8 +++--- op-service/ioutil/wrapped_closer.go | 44 +++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) create mode 100644 op-service/ioutil/wrapped_closer.go diff --git a/op-service/ioutil/gzip.go b/op-service/ioutil/gzip.go index 52fbb10cc4c1..65d848ad201f 100644 --- a/op-service/ioutil/gzip.go +++ b/op-service/ioutil/gzip.go @@ -12,23 +12,23 @@ import ( // OpenDecompressed opens a reader for the specified file and automatically gzip decompresses the content // if the filename ends with .gz func OpenDecompressed(path string) (io.ReadCloser, error) { - var r io.ReadCloser r, err := os.Open(path) if err != nil { return nil, err } if IsGzip(path) { - r, err = gzip.NewReader(r) + gr, err := gzip.NewReader(r) if err != nil { + r.Close() return nil, fmt.Errorf("failed to create gzip reader: %w", err) } + return NewWrappedReadCloser(gr, r), nil } return r, nil } // OpenCompressed opens a file for writing and automatically compresses the content if the filename ends with .gz func OpenCompressed(file string, flag int, perm os.FileMode) (io.WriteCloser, error) { - var out io.WriteCloser out, err := os.OpenFile(file, flag, perm) if err != nil { return nil, err @@ -70,7 +70,7 @@ func IsGzip(path string) bool { func CompressByFileType(file string, out io.WriteCloser) io.WriteCloser { if IsGzip(file) { - return gzip.NewWriter(out) + return NewWrappedWriteCloser(gzip.NewWriter(out), out) } return out } diff --git a/op-service/ioutil/wrapped_closer.go b/op-service/ioutil/wrapped_closer.go new file mode 100644 index 000000000000..3440ca677cb2 --- /dev/null +++ b/op-service/ioutil/wrapped_closer.go @@ -0,0 +1,44 @@ +package ioutil + +import ( + "errors" + "io" +) + +// WrappedReadCloser is a struct that closes both the gzip.Reader and the underlying io.Closer. +type WrappedReadCloser struct { + io.ReadCloser + closer io.Closer +} + +// WrappedWriteCloser is a struct that closes both the gzip.Writer and the underlying io.Closer. +type WrappedWriteCloser struct { + io.WriteCloser + closer io.Closer +} + +// Close closes both the gzip.Reader and the underlying reader. +func (g *WrappedReadCloser) Close() error { + return errors.Join(g.ReadCloser.Close(), g.closer.Close()) +} + +// Close closes both the gzip.Writer and the underlying writer. +func (g *WrappedWriteCloser) Close() error { + return errors.Join(g.WriteCloser.Close(), g.closer.Close()) +} + +// NewWrappedReadCloser is a constructor function that initializes a WrappedReadCloser structure. +func NewWrappedReadCloser(r io.ReadCloser, c io.Closer) *WrappedReadCloser { + return &WrappedReadCloser{ + ReadCloser: r, + closer: c, + } +} + +// NewWrappedWriteCloser is a constructor function that initializes a WrappedWriteCloser structure. +func NewWrappedWriteCloser(r io.WriteCloser, c io.Closer) *WrappedWriteCloser { + return &WrappedWriteCloser{ + WriteCloser: r, + closer: c, + } +}