From d267a7ccee6e5ceb5f9098503704779d28c3c16c Mon Sep 17 00:00:00 2001 From: Carlo Alberto Ferraris Date: Wed, 19 Jan 2022 08:53:00 +0000 Subject: [PATCH] Check compressor options strictly during compressor initialization --- contrib/andybalholm/brotli/brotli.go | 8 +++++++- contrib/google/cbrotli/brotli.go | 10 +++------- contrib/internal/utils/check.go | 24 ++++++++++++++++++++++++ contrib/klauspost/gzip/gzip.go | 14 +++++++++++++- contrib/klauspost/pgzip/pgzip.go | 16 ++++++++++++++-- contrib/klauspost/zstd/zstd.go | 15 ++++++++++++++- contrib/valyala/gozstd/zstd.go | 13 +++++++++++++ contrib/valyala/gozstd/zstd_race_test.go | 1 + 8 files changed, 89 insertions(+), 12 deletions(-) create mode 100644 contrib/internal/utils/check.go diff --git a/contrib/andybalholm/brotli/brotli.go b/contrib/andybalholm/brotli/brotli.go index 334061e..8a8d205 100644 --- a/contrib/andybalholm/brotli/brotli.go +++ b/contrib/andybalholm/brotli/brotli.go @@ -5,6 +5,7 @@ import ( "io" "sync" + "github.com/CAFxX/httpcompression/contrib/internal/utils" "github.com/andybalholm/brotli" ) @@ -26,7 +27,12 @@ func New(opts Options) (c *compressor, err error) { c, err = nil, fmt.Errorf("panic: %v", r) } }() - _ = brotli.NewWriterOptions(nil, opts) + + tw := brotli.NewWriterOptions(io.Discard, opts) + if err := utils.CheckWriter(tw); err != nil { + return nil, fmt.Errorf("brotli: writer initialization: %w", err) + } + c = &compressor{opts: opts} return c, nil } diff --git a/contrib/google/cbrotli/brotli.go b/contrib/google/cbrotli/brotli.go index a09e327..1321227 100644 --- a/contrib/google/cbrotli/brotli.go +++ b/contrib/google/cbrotli/brotli.go @@ -5,6 +5,7 @@ import ( "io" "runtime" + "github.com/CAFxX/httpcompression/contrib/internal/utils" "github.com/google/brotli/go/cbrotli" ) @@ -18,13 +19,8 @@ type compressor struct { func New(opts cbrotli.WriterOptions) (c *compressor, err error) { tw := cbrotli.NewWriter(io.Discard, opts) - n, err := tw.Write([]byte("test")) - if n != 4 || err != nil { - return nil, fmt.Errorf("cbrotli: writer initialization (write): %d, %w", n, err) - } - err = tw.Close() - if err != nil { - return nil, fmt.Errorf("cbrotli: writer initialization (close): %w", err) + if err := utils.CheckWriter(tw); err != nil { + return nil, fmt.Errorf("cbrotli: writer initialization: %w", err) } c = &compressor{opts: opts} diff --git a/contrib/internal/utils/check.go b/contrib/internal/utils/check.go new file mode 100644 index 0000000..0c66b1e --- /dev/null +++ b/contrib/internal/utils/check.go @@ -0,0 +1,24 @@ +package utils + +import ( + "fmt" + "io" +) + +func CheckWriter(w io.Writer) error { + const data = "test" + n, err := w.Write([]byte(data)) + if err != nil { + return fmt.Errorf("write: %w", err) + } + if n != len(data) { + return fmt.Errorf("write: short write: %d", n) + } + if w, ok := w.(io.Closer); ok { + err := w.Close() + if err != nil { + return fmt.Errorf("close: %w", err) + } + } + return nil +} diff --git a/contrib/klauspost/gzip/gzip.go b/contrib/klauspost/gzip/gzip.go index 490f139..c6b5012 100644 --- a/contrib/klauspost/gzip/gzip.go +++ b/contrib/klauspost/gzip/gzip.go @@ -1,9 +1,11 @@ package gzip import ( + "fmt" "io" "sync" + "github.com/CAFxX/httpcompression/contrib/internal/utils" "github.com/klauspost/compress/gzip" ) @@ -22,10 +24,20 @@ type Options struct { } func New(opts Options) (c *compressor, err error) { - _, err = gzip.NewWriterLevel(nil, opts.Level) + defer func() { + if r := recover(); r != nil { + c, err = nil, fmt.Errorf("panic: %v", r) + } + }() + + tw, err := gzip.NewWriterLevel(io.Discard, opts.Level) if err != nil { return nil, err } + if err := utils.CheckWriter(tw); err != nil { + return nil, fmt.Errorf("gzip: writer initialization: %w", err) + } + c = &compressor{opts: opts} return c, nil } diff --git a/contrib/klauspost/pgzip/pgzip.go b/contrib/klauspost/pgzip/pgzip.go index da895a2..6f93ff8 100644 --- a/contrib/klauspost/pgzip/pgzip.go +++ b/contrib/klauspost/pgzip/pgzip.go @@ -1,9 +1,11 @@ package pgzip import ( + "fmt" "io" "sync" + "github.com/CAFxX/httpcompression/contrib/internal/utils" "github.com/klauspost/pgzip" ) @@ -24,14 +26,24 @@ type Options struct { } func New(opts Options) (c *compressor, err error) { - gw, err := pgzip.NewWriterLevel(nil, opts.Level) + defer func() { + if r := recover(); r != nil { + c, err = nil, fmt.Errorf("panic: %v", r) + } + }() + + tw, err := pgzip.NewWriterLevel(io.Discard, opts.Level) if err != nil { return nil, err } - err = gw.SetConcurrency(opts.BlockSize, opts.Blocks) + err = tw.SetConcurrency(opts.BlockSize, opts.Blocks) if err != nil { return nil, err } + if err := utils.CheckWriter(tw); err != nil { + return nil, fmt.Errorf("pgzip: writer initialization: %w", err) + } + c = &compressor{opts: opts} return c, nil } diff --git a/contrib/klauspost/zstd/zstd.go b/contrib/klauspost/zstd/zstd.go index d291c1f..09b15ce 100644 --- a/contrib/klauspost/zstd/zstd.go +++ b/contrib/klauspost/zstd/zstd.go @@ -1,9 +1,11 @@ package zstd import ( + "fmt" "io" "sync" + "github.com/CAFxX/httpcompression/contrib/internal/utils" "github.com/klauspost/compress/zstd" ) @@ -18,11 +20,22 @@ type compressor struct { } func New(opts ...zstd.EOption) (c *compressor, err error) { + defer func() { + if r := recover(); r != nil { + c, err = nil, fmt.Errorf("panic: %v", r) + } + }() + opts = append([]zstd.EOption(nil), opts...) - _, err = zstd.NewWriter(nil, opts...) + + tw, err := zstd.NewWriter(io.Discard, opts...) if err != nil { return nil, err } + if err := utils.CheckWriter(tw); err != nil { + return nil, fmt.Errorf("zstd: writer initialization: %w", err) + } + c = &compressor{opts: opts} return c, nil } diff --git a/contrib/valyala/gozstd/zstd.go b/contrib/valyala/gozstd/zstd.go index 427d36c..6fa8425 100644 --- a/contrib/valyala/gozstd/zstd.go +++ b/contrib/valyala/gozstd/zstd.go @@ -1,9 +1,11 @@ package gozstd import ( + "fmt" "io" "sync" + "github.com/CAFxX/httpcompression/contrib/internal/utils" "github.com/valyala/gozstd" ) @@ -17,6 +19,17 @@ type compressor struct { } func New(opts gozstd.WriterParams) (c *compressor, err error) { + defer func() { + if r := recover(); r != nil { + c, err = nil, fmt.Errorf("panic: %v", r) + } + }() + + tw := gozstd.NewWriterParams(io.Discard, &opts) + if err := utils.CheckWriter(tw); err != nil { + return nil, fmt.Errorf("gozstd: writer initialization: %w", err) + } + c = &compressor{opts: opts} return c, nil } diff --git a/contrib/valyala/gozstd/zstd_race_test.go b/contrib/valyala/gozstd/zstd_race_test.go index 78cdf09..99004e6 100644 --- a/contrib/valyala/gozstd/zstd_race_test.go +++ b/contrib/valyala/gozstd/zstd_race_test.go @@ -1,3 +1,4 @@ +//go:build race // +build race package gozstd_test