From a147a4c90a006a08efb708b3f8e20ea26c9d342e Mon Sep 17 00:00:00 2001 From: Julian Thome Date: Fri, 15 Mar 2024 15:54:26 +0100 Subject: [PATCH 1/4] fix: restrict zip decompression --- commands/ci/artifact/artifact.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/commands/ci/artifact/artifact.go b/commands/ci/artifact/artifact.go index 964a3d0f9..0b38c58b4 100644 --- a/commands/ci/artifact/artifact.go +++ b/commands/ci/artifact/artifact.go @@ -4,6 +4,7 @@ import ( "archive/zip" "fmt" "io" + "log" "os" "path/filepath" "strings" @@ -103,7 +104,12 @@ func NewCmdRun(f *cmdutils.Factory) *cobra.Command { if err != nil { return err } - defer srcFile.Close() + + defer func() { + if err := srcFile.Close(); err != nil { + log.Fatal(err.Error()) + } + }() err = ensurePathIsCreated(destPath) if err != nil { -- GitLab From 9834810874ef64066be7191923f7e587021b660b Mon Sep 17 00:00:00 2001 From: Julian Thome Date: Fri, 15 Mar 2024 15:54:56 +0100 Subject: [PATCH 2/4] Revert "fix: restrict zip decompression" This reverts commit a147a4c90a006a08efb708b3f8e20ea26c9d342e. --- commands/ci/artifact/artifact.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/commands/ci/artifact/artifact.go b/commands/ci/artifact/artifact.go index 0b38c58b4..964a3d0f9 100644 --- a/commands/ci/artifact/artifact.go +++ b/commands/ci/artifact/artifact.go @@ -4,7 +4,6 @@ import ( "archive/zip" "fmt" "io" - "log" "os" "path/filepath" "strings" @@ -104,12 +103,7 @@ func NewCmdRun(f *cmdutils.Factory) *cobra.Command { if err != nil { return err } - - defer func() { - if err := srcFile.Close(); err != nil { - log.Fatal(err.Error()) - } - }() + defer srcFile.Close() err = ensurePathIsCreated(destPath) if err != nil { -- GitLab From fdb473d5ed75f6cc62bb64b535f44b44125e0feb Mon Sep 17 00:00:00 2001 From: Julian Thome Date: Fri, 15 Mar 2024 16:07:55 +0100 Subject: [PATCH 3/4] fix: unhandled errors of unsafe calls in defer --- api/request/request.go | 6 +++++- cmd/gen-docs/docs.go | 6 +++++- commands/api/api.go | 16 ++++++++++++++-- commands/auth/login/login.go | 8 +++++++- commands/cmdutils/cmdutils.go | 8 +++++++- commands/release/create/create.go | 1 - commands/release/download/download.go | 14 ++++++++++++-- commands/release/releaseutils/upload/upload.go | 1 - commands/ssh-key/add/add.go | 13 +++++++++++-- commands/variable/variableutils/value.go | 9 +++++++-- internal/recovery/recovery.go | 7 ++++++- pkg/git/ssh_config.go | 7 ++++++- pkg/iostreams/iostreams.go | 7 ++++++- 13 files changed, 86 insertions(+), 17 deletions(-) diff --git a/api/request/request.go b/api/request/request.go index e1f6eacaa..72876b733 100644 --- a/api/request/request.go +++ b/api/request/request.go @@ -23,7 +23,11 @@ func MakeRequest(payload, url, method string) (string, error) { if err != nil { return "", err } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + log.Fatal(err) + } + }() bodyBytes, _ := io.ReadAll(resp.Body) bodyString := string(bodyBytes) diff --git a/cmd/gen-docs/docs.go b/cmd/gen-docs/docs.go index 04ab2d015..a15e32722 100644 --- a/cmd/gen-docs/docs.go +++ b/cmd/gen-docs/docs.go @@ -174,7 +174,11 @@ func GenMarkdownTreeCustom(cmd *cobra.Command, dir string) error { if err != nil { return err } - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + fatal(err) + } + }() if err := GenMarkdownCustom(cmd, f); err != nil { return err diff --git a/commands/api/api.go b/commands/api/api.go index e41a9437f..d3ce9e984 100644 --- a/commands/api/api.go +++ b/commands/api/api.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "log" "net/http" "net/url" "os" @@ -219,7 +220,12 @@ func apiRun(opts *ApiOptions) error { if err != nil { return err } - defer file.Close() + defer func() { + if err := file.Close(); err != nil { + log.Fatal(err.Error()) + } + }() + requestPath, err = parseQuery(requestPath, params) if err != nil { return err @@ -487,7 +493,13 @@ func readUserFile(fn string, stdin io.ReadCloser) ([]byte, error) { return nil, err } } - defer r.Close() + + defer func() { + if err := r.Close(); err != nil { + log.Fatal(err.Error()) + } + }() + return io.ReadAll(r) } diff --git a/commands/auth/login/login.go b/commands/auth/login/login.go index a79fd577c..f42039d4a 100644 --- a/commands/auth/login/login.go +++ b/commands/auth/login/login.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "io" + "log" "os" "regexp" "strings" @@ -72,7 +73,12 @@ func NewCmdLogin(f *cmdutils.Factory) *cobra.Command { } if tokenStdin { - defer opts.IO.In.Close() + defer func() { + if err := opts.IO.In.Close(); err != nil { + log.Fatal(err.Error()) + } + }() + token, err := io.ReadAll(opts.IO.In) if err != nil { return fmt.Errorf("failed to read token from STDIN: %w", err) diff --git a/commands/cmdutils/cmdutils.go b/commands/cmdutils/cmdutils.go index 7d402276e..680fac4ae 100644 --- a/commands/cmdutils/cmdutils.go +++ b/commands/cmdutils/cmdutils.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "io" + "log" "os" "path/filepath" "sort" @@ -78,7 +79,12 @@ func ListGitLabTemplates(tmplType string) ([]string, error) { return files, nil } fileNames, err := f.Readdirnames(-1) - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + log.Println(err) + } + }() + if err != nil { // return empty slice if error return files, nil diff --git a/commands/release/create/create.go b/commands/release/create/create.go index 594d0b0ce..f57aedbf5 100644 --- a/commands/release/create/create.go +++ b/commands/release/create/create.go @@ -338,7 +338,6 @@ func createRun(opts *CreateOpts) error { } release, _, err = client.Releases.CreateRelease(repo.FullName(), createOpts) - if err != nil { return releaseFailedErr(err, start) } diff --git a/commands/release/download/download.go b/commands/release/download/download.go index 57854b4ae..82c6b4ded 100644 --- a/commands/release/download/download.go +++ b/commands/release/download/download.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "io" + "log" "net/http" "net/url" "os" @@ -233,7 +234,11 @@ func downloadAsset(client *api.Client, assetURL, destinationPath string) error { return err } - defer resp.Body.Close() + defer func() { + if err := resp.Body.Close(); err != nil { + log.Fatal(err) + } + }() if resp.StatusCode > 299 { return errors.New(resp.Status) @@ -243,7 +248,12 @@ func downloadAsset(client *api.Client, assetURL, destinationPath string) error { if err != nil { return err } - defer f.Close() + + defer func() { + if err := f.Close(); err != nil { + log.Fatal(err) + } + }() _, err = io.Copy(f, resp.Body) return err diff --git a/commands/release/releaseutils/upload/upload.go b/commands/release/releaseutils/upload/upload.go index 5707bdfaa..654b558de 100644 --- a/commands/release/releaseutils/upload/upload.go +++ b/commands/release/releaseutils/upload/upload.go @@ -81,7 +81,6 @@ func (c *Context) UploadFiles(projectID, tagName string) error { FilePath: &filename, LinkType: file.Type, }) - if err != nil { return err } diff --git a/commands/ssh-key/add/add.go b/commands/ssh-key/add/add.go index b20c6bfb7..c8c64d61e 100644 --- a/commands/ssh-key/add/add.go +++ b/commands/ssh-key/add/add.go @@ -3,6 +3,7 @@ package add import ( "errors" "io" + "log" "os" "github.com/MakeNowJust/heredoc" @@ -83,13 +84,21 @@ func addRun(opts *AddOpts) error { var keyFileReader io.Reader if opts.KeyFile == "-" { keyFileReader = opts.IO.In - defer opts.IO.In.Close() + defer func() { + if err := opts.IO.In.Close(); err != nil { + log.Fatal(err) + } + }() } else { f, err := os.Open(opts.KeyFile) if err != nil { return err } - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + log.Fatal(err) + } + }() keyFileReader = f } diff --git a/commands/variable/variableutils/value.go b/commands/variable/variableutils/value.go index 38032fd50..ab63cb03e 100644 --- a/commands/variable/variableutils/value.go +++ b/commands/variable/variableutils/value.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "io" + "log" "strings" "gitlab.com/gitlab-org/cli/commands/cmdutils" @@ -21,8 +22,12 @@ func GetValue(value string, ios *iostreams.IOStreams, args []string) (string, er return "", &cmdutils.FlagError{Err: errors.New("no value specified but nothing on STDIN")} } - // read value from STDIN if not provided - defer ios.In.Close() + defer func() { + if err := ios.In.Close(); err != nil { + log.Fatal(err) + } + }() + stdinValue, err := io.ReadAll(ios.In) if err != nil { return "", fmt.Errorf("failed to read value from STDIN: %w", err) diff --git a/internal/recovery/recovery.go b/internal/recovery/recovery.go index fcc49e4f7..64a0938bc 100644 --- a/internal/recovery/recovery.go +++ b/internal/recovery/recovery.go @@ -3,6 +3,7 @@ package recovery import ( "encoding/json" "fmt" + "log" "os" "path/filepath" @@ -41,7 +42,11 @@ func CreateFile(repoName, filename string, i any) (string, error) { return "", fmt.Errorf("creating recovery file: %w", err) } - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + log.Fatal(err) + } + }() if err := json.NewEncoder(f).Encode(i); err != nil { return "", fmt.Errorf("writing file: %w", err) diff --git a/pkg/git/ssh_config.go b/pkg/git/ssh_config.go index 135f07204..bc3ed7037 100644 --- a/pkg/git/ssh_config.go +++ b/pkg/git/ssh_config.go @@ -3,6 +3,7 @@ package git import ( "bufio" "io" + "log" "net/url" "os" "path/filepath" @@ -56,7 +57,11 @@ func (p *sshParser) read(fileName string) error { if err != nil { return err } - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + log.Fatal(err) + } + }() file = f } else { var err error diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index f3448d1f5..4ba0e589e 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -5,6 +5,7 @@ import ( "bytes" "fmt" "io" + "log" "os" "os/exec" "regexp" @@ -148,7 +149,11 @@ func (s *IOStreams) StartPager() error { // We should eventually add some error reporting for the go function go func() { - defer pagedOut.Close() + defer func() { + if err := pagedOut.Close(); err != nil { + log.Fatal(err) + } + }() scanner := bufio.NewScanner(pipeReader) -- GitLab From 873dc2207f3c0a15ee44e106086750fe49a7c988 Mon Sep 17 00:00:00 2001 From: Julian Thome Date: Fri, 15 Mar 2024 16:12:20 +0100 Subject: [PATCH 4/4] fix: unhandled errors of unsafe calls in defer --- commands/cmdutils/cmdutils.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands/cmdutils/cmdutils.go b/commands/cmdutils/cmdutils.go index 680fac4ae..1ef514c01 100644 --- a/commands/cmdutils/cmdutils.go +++ b/commands/cmdutils/cmdutils.go @@ -81,7 +81,7 @@ func ListGitLabTemplates(tmplType string) ([]string, error) { fileNames, err := f.Readdirnames(-1) defer func() { if err := f.Close(); err != nil { - log.Println(err) + log.Fatal(err) } }() -- GitLab