From f099250beb34b4041e47eb27a3d4aa5b88a47b4e Mon Sep 17 00:00:00 2001 From: Mario Loriedo Date: Sun, 25 Feb 2024 21:04:56 +0100 Subject: [PATCH] Better file close and err handling Signed-off-by: Mario Loriedo --- pkg/machine/compression/compression_test.go | 8 ++- pkg/machine/compression/generic.go | 19 +++---- pkg/machine/compression/gzip.go | 22 ++++---- .../compression/testdata/gen-testdata.sh | 51 ++++++++++++++++++ .../compression/testdata/sample-withzeros.bz2 | Bin 0 -> 49 bytes .../compression/testdata/sample-withzeros.gz | Bin 0 -> 47 bytes .../testdata/sample-withzeros.uncompressed | Bin 0 -> 20 bytes .../compression/testdata/sample-withzeros.xz | Bin 0 -> 68 bytes .../compression/testdata/sample-withzeros.zip | Bin 0 -> 194 bytes .../compression/testdata/sample-withzeros.zst | Bin 0 -> 23 bytes pkg/machine/compression/testdata/sample.gz | Bin 31 -> 35 bytes pkg/machine/compression/testdata/sample.zip | Bin 164 -> 170 bytes pkg/machine/compression/uncompressed.go | 8 +++ pkg/machine/compression/zstd.go | 17 +++--- 14 files changed, 95 insertions(+), 30 deletions(-) create mode 100755 pkg/machine/compression/testdata/gen-testdata.sh create mode 100644 pkg/machine/compression/testdata/sample-withzeros.bz2 create mode 100644 pkg/machine/compression/testdata/sample-withzeros.gz create mode 100644 pkg/machine/compression/testdata/sample-withzeros.uncompressed create mode 100644 pkg/machine/compression/testdata/sample-withzeros.xz create mode 100644 pkg/machine/compression/testdata/sample-withzeros.zip create mode 100644 pkg/machine/compression/testdata/sample-withzeros.zst diff --git a/pkg/machine/compression/compression_test.go b/pkg/machine/compression/compression_test.go index b41a9077c4..9c51cd8bca 100644 --- a/pkg/machine/compression/compression_test.go +++ b/pkg/machine/compression/compression_test.go @@ -111,11 +111,17 @@ func Test_Decompress(t *testing.T) { want want }{ {name: "zip", args: args{src: "./testdata/sample.zip", dst: "./testdata/hellozip"}, want: "zip\n"}, + {name: "zip with trailing zeros", args: args{src: "./testdata/sample-withzeros.zip", dst: "./testdata/hellozip-withzeros"}, want: "zip\n\x00\x00\x00\x00\x00\x00"}, {name: "xz", args: args{src: "./testdata/sample.xz", dst: "./testdata/helloxz"}, want: "xz\n"}, + {name: "xz with trailing zeros", args: args{src: "./testdata/sample-withzeros.xz", dst: "./testdata/helloxz-withzeros"}, want: "xz\n\x00\x00\x00\x00\x00\x00\x00"}, {name: "gzip", args: args{src: "./testdata/sample.gz", dst: "./testdata/hellogz"}, want: "gzip\n"}, + {name: "gzip with trailing zeros", args: args{src: "./testdata/sample-withzeros.gz", dst: "./testdata/hellogzip-withzeros"}, want: "gzip\n\x00\x00\x00\x00\x00"}, {name: "bzip2", args: args{src: "./testdata/sample.bz2", dst: "./testdata/hellobz2"}, want: "bzip2\n"}, + {name: "bzip2 with trailing zeros", args: args{src: "./testdata/sample-withzeros.bz2", dst: "./testdata/hellobz2-withzeros"}, want: "bzip2\n\x00\x00\x00\x00"}, {name: "zstd", args: args{src: "./testdata/sample.zst", dst: "./testdata/hellozstd"}, want: "zstd\n"}, - {name: "uncompressed", args: args{src: "./testdata/sample.uncompressed", dst: "./testdata/hellozuncompressed"}, want: "uncompressed\n"}, + {name: "zstd with trailing zeros", args: args{src: "./testdata/sample-withzeros.zst", dst: "./testdata/hellozstd-withzeros"}, want: "zstd\n\x00\x00\x00\x00\x00"}, + {name: "uncompressed", args: args{src: "./testdata/sample.uncompressed", dst: "./testdata/hellouncompressed"}, want: "uncompressed\n"}, + {name: "uncompressed with trailing zeros", args: args{src: "./testdata/sample-withzeros.uncompressed", dst: "./testdata/hellozuncompressed-withzeros"}, want: "uncompressed\n\x00\x00\x00\x00\x00\x00\x00"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/machine/compression/generic.go b/pkg/machine/compression/generic.go index d8fb9b25fb..7f808c2d67 100644 --- a/pkg/machine/compression/generic.go +++ b/pkg/machine/compression/generic.go @@ -10,10 +10,9 @@ import ( ) type genericDecompressor struct { - compressedFilePath string - compressedFile *os.File - decompressedFileReader io.ReadCloser - compressedFileInfo os.FileInfo + compressedFilePath string + compressedFile *os.File + compressedFileInfo os.FileInfo } func newGenericDecompressor(compressedFilePath string) (*genericDecompressor, error) { @@ -49,7 +48,11 @@ func (d *genericDecompressor) decompress(w WriteSeekCloser, r io.Reader) error { if err != nil { return err } - d.decompressedFileReader = decompressedFileReader + defer func() { + if err := decompressedFileReader.Close(); err != nil { + logrus.Errorf("Unable to close gz file: %q", err) + } + }() _, err = io.Copy(w, decompressedFileReader) return err @@ -59,10 +62,4 @@ func (d *genericDecompressor) close() { if err := d.compressedFile.Close(); err != nil { logrus.Errorf("Unable to close compressed file: %q", err) } - - if d.decompressedFileReader != nil { - if err := d.decompressedFileReader.Close(); err != nil { - logrus.Errorf("Unable to close uncompressed stream: %q", err) - } - } } diff --git a/pkg/machine/compression/gzip.go b/pkg/machine/compression/gzip.go index cd12ea0dea..f00830dc76 100644 --- a/pkg/machine/compression/gzip.go +++ b/pkg/machine/compression/gzip.go @@ -9,12 +9,11 @@ import ( type gzipDecompressor struct { genericDecompressor - gzReader io.ReadCloser } func newGzipDecompressor(compressedFilePath string) (*gzipDecompressor, error) { d, err := newGenericDecompressor(compressedFilePath) - return &gzipDecompressor{*d, nil}, err + return &gzipDecompressor{*d}, err } func (d *gzipDecompressor) decompress(w WriteSeekCloser, r io.Reader) error { @@ -22,16 +21,19 @@ func (d *gzipDecompressor) decompress(w WriteSeekCloser, r io.Reader) error { if err != nil { return err } - d.gzReader = gzReader + defer func() { + if err := gzReader.Close(); err != nil { + logrus.Errorf("Unable to close gz file: %q", err) + } + }() sparseWriter := NewSparseWriter(w) + defer func() { + if err := sparseWriter.Close(); err != nil { + logrus.Errorf("Unable to close uncompressed file: %q", err) + } + }() + _, err = io.Copy(sparseWriter, gzReader) return err } - -func (d *gzipDecompressor) close() { - if err := d.gzReader.Close(); err != nil { - logrus.Errorf("Unable to close gz file: %q", err) - } - d.genericDecompressor.close() -} diff --git a/pkg/machine/compression/testdata/gen-testdata.sh b/pkg/machine/compression/testdata/gen-testdata.sh new file mode 100755 index 0000000000..375b039e19 --- /dev/null +++ b/pkg/machine/compression/testdata/gen-testdata.sh @@ -0,0 +1,51 @@ +#!/bin/bash + +echo "zstd" > hellozstd-withzeros && \ +truncate -c -s 10 hellozstd-withzeros && \ +zstd -f --sparse hellozstd-withzeros -o sample-withzeros.zst && \ +rm hellozstd-withzeros + +echo "zstd" > hellozstd && \ +zstd -f --sparse hellozstd -o sample.zst && \ +rm hellozstd + +echo "gzip" > hellogzip-withzeros && \ +truncate -c -s 10 hellogzip-withzeros && \ +gzip -f -c hellogzip-withzeros > sample-withzeros.gz && \ +rm hellogzip-withzeros + +echo "gzip" > hellogzip && \ +gzip -f -c hellogzip > sample.gz && \ +rm hellogzip + +echo "bzip2" > hellobzip2-withzeros && \ +truncate -c -s 10 hellobzip2-withzeros && \ +bzip2 -f -c hellobzip2-withzeros > sample-withzeros.bz2 && \ +rm hellobzip2-withzeros + +echo "bzip2" > hellobzip2 && \ +bzip2 -f -c hellobzip2 > sample.bz2 && \ +rm hellobzip2 + +echo "uncompressed" > sample-withzeros.uncompressed && \ +truncate -c -s 20 sample-withzeros.uncompressed + +echo "uncompressed" > sample.uncompressed + +echo "xz" > helloxz-withzeros && \ +truncate -c -s 10 helloxz-withzeros && \ +xz -f -z -c helloxz-withzeros > sample-withzeros.xz && \ +rm helloxz-withzeros + +echo "xz" > helloxz && \ +xz -f -z -c helloxz > sample.xz && \ +rm helloxz + +echo "zip" > hellozip-withzeros && \ +truncate -c -s 10 hellozip-withzeros && \ +zip sample-withzeros.zip hellozip-withzeros && \ +rm hellozip-withzeros + +echo "zip" > hellozip && \ +zip sample.zip hellozip && \ +rm hellozip diff --git a/pkg/machine/compression/testdata/sample-withzeros.bz2 b/pkg/machine/compression/testdata/sample-withzeros.bz2 new file mode 100644 index 0000000000000000000000000000000000000000..782a681c6ca6ef41916345e58e82945091feaac6 GIT binary patch literal 49 zcmZ>Y%CIzaj8qGbbWD1FhJk_kWP^i%0E2*ngMb2qArHf4FdS)f~q^21Q0O1_p)_{ill=88|DdxPTle;8keiZF#$WF$1F#m#Eak Pbop(cYZ*auERj(FT3QgU literal 0 HcmV?d00001 diff --git a/pkg/machine/compression/testdata/sample-withzeros.zip b/pkg/machine/compression/testdata/sample-withzeros.zip new file mode 100644 index 0000000000000000000000000000000000000000..778730a2f603b5832635f7997f7bedc2659fce84 GIT binary patch literal 194 zcmWIWW@Zs#U|`^2D4h@y!Pdg~iv!5x0%9Qs8HSA1oSgis%mUr=%#w_%)S~?2&=5`r z<~+q)sUTch!Og(P@|BT+0c^(VQ#ucl7?>FXycwC~m~ojZ0W=!|mNbHBY<93h>_9U= Tz?+o~q=OL%BY<=Wh{FH?jaetv literal 0 HcmV?d00001 diff --git a/pkg/machine/compression/testdata/sample-withzeros.zst b/pkg/machine/compression/testdata/sample-withzeros.zst new file mode 100644 index 0000000000000000000000000000000000000000..6fac8c6631a34230cf50f40142f9d0e3ced77e8d GIT binary patch literal 23 ccmdPcs{dDoE0BSqs<FdS-{}E{*=xGCWiktafz%93=9C#LknvF literal 31 mcmb2|=HQT%I+V)5oRON7lh5G2{*=xGCWiktafz%93=9C1SqVY_ diff --git a/pkg/machine/compression/testdata/sample.zip b/pkg/machine/compression/testdata/sample.zip index 481aeadc5260f128ca9c1c3bb9cbef07f4be5841..a03bcd4bb0322a260884af73cb3daef645844a85 100644 GIT binary patch delta 105 zcmZ3&xQfvzz?+$civa{mCqzUn{;#!`1;_?r4h9*9jMSW*{Hn}?&=5`r<~+q)sX#nY eUqKKF5u)2wLGF*N@Gu(iffdK$$F&R?; literal 164 zcmWIWW@h1H0D&jTZV`+BYprDgvO$=YL53kCH76%OG=!6Zxmw~-Y7G#VR&X;gvV3I( zsu2Mys>&?j3h-uRl4HhYhy+j-0|QV!!;(f23u+`Q#7H#b0=!w-K#CZF&>KiQgE$NT Ds}vm! diff --git a/pkg/machine/compression/uncompressed.go b/pkg/machine/compression/uncompressed.go index 03ebfb4a41..8bb5ab43a5 100644 --- a/pkg/machine/compression/uncompressed.go +++ b/pkg/machine/compression/uncompressed.go @@ -2,6 +2,8 @@ package compression import ( "io" + + "github.com/sirupsen/logrus" ) type uncompressedDecompressor struct { @@ -15,6 +17,12 @@ func newUncompressedDecompressor(compressedFilePath string) (*uncompressedDecomp func (d *uncompressedDecompressor) decompress(w WriteSeekCloser, r io.Reader) error { sparseWriter := NewSparseWriter(w) + defer func() { + if err := sparseWriter.Close(); err != nil { + logrus.Errorf("Unable to close uncompressed file: %q", err) + } + }() + _, err := io.Copy(sparseWriter, r) return err } diff --git a/pkg/machine/compression/zstd.go b/pkg/machine/compression/zstd.go index bf234961ea..be3b1fd825 100644 --- a/pkg/machine/compression/zstd.go +++ b/pkg/machine/compression/zstd.go @@ -4,16 +4,16 @@ import ( "io" "github.com/klauspost/compress/zstd" + "github.com/sirupsen/logrus" ) type zstdDecompressor struct { genericDecompressor - zstdReader *zstd.Decoder } func newZstdDecompressor(compressedFilePath string) (*zstdDecompressor, error) { d, err := newGenericDecompressor(compressedFilePath) - return &zstdDecompressor{*d, nil}, err + return &zstdDecompressor{*d}, err } func (d *zstdDecompressor) decompress(w WriteSeekCloser, r io.Reader) error { @@ -21,14 +21,15 @@ func (d *zstdDecompressor) decompress(w WriteSeekCloser, r io.Reader) error { if err != nil { return err } - d.zstdReader = zstdReader + defer zstdReader.Close() sparseWriter := NewSparseWriter(w) + defer func() { + if err := sparseWriter.Close(); err != nil { + logrus.Errorf("Unable to close uncompressed file: %q", err) + } + }() + _, err = io.Copy(sparseWriter, zstdReader) return err } - -func (d *zstdDecompressor) close() { - d.zstdReader.Close() - d.genericDecompressor.close() -}