all: audit tmp.NewFile use

The fact that this idiom was applied unevenly means the interface is in need of retooling, IMO. See-also: PROJQUAY-5165 Signed-off-by: Hank Donnay <hdonnay@redhat.com>
quay · Mar 7, 2023 · 35f60dd · 35f60dd
1 parent eea1e52
commit 35f60dd
Show file tree

Hide file tree

Showing 6 changed files with 55 additions and 13 deletions.
diff --git a/alpine/fetcher.go b/alpine/fetcher.go
@@ -55,18 +55,25 @@ func (u *updater) Fetch(ctx context.Context, hint driver.Fingerprint) (io.ReadCl
 	zlog.Debug(ctx).
 		Str("name", tf.Name()).
 		Msg("created tempfile")
+	var success bool
+	defer func() {
+		if !success {
+			if err := tf.Close(); err != nil {
+				zlog.Warn(ctx).Err(err).Msg("unable to close spool")
+			}
+		}
+	}()
 
 	var r io.Reader = res.Body
 	if _, err := io.Copy(tf, r); err != nil {
-		tf.Close()
 		return nil, hint, fmt.Errorf("alpine: unable to copy resp body to tempfile: %w", err)
 	}
 	if n, err := tf.Seek(0, io.SeekStart); err != nil || n != 0 {
-		tf.Close()
 		return nil, hint, fmt.Errorf("alpine: unable to seek database to start: at %d, %v", n, err)
 	}
 	zlog.Debug(ctx).Msg("decompressed and buffered database")
 
+	success = true
 	hint = driver.Fingerprint(res.Header.Get("etag"))
 	zlog.Debug(ctx).
 		Str("hint", string(hint)).

diff --git a/aws/client.go b/aws/client.go
@@ -117,6 +117,14 @@ func (c *Client) Updates(ctx context.Context) (io.ReadCloser, error) {
 			zlog.Error(ctx).Err(err).Msg("failed to open temp file")
 			continue
 		}
+		var success bool
+		defer func() {
+			if !success {
+				if err := tf.Close(); err != nil {
+					zlog.Warn(ctx).Err(err).Msg("unable to close spool")
+				}
+			}
+		}()
 
 		resp, err := c.c.Do(req)
 		if err != nil {
@@ -133,16 +141,13 @@ func (c *Client) Updates(ctx context.Context) (io.ReadCloser, error) {
 				Int("code", resp.StatusCode).
 				Str("status", resp.Status).
 				Msg("unexpected HTTP response")
-			tf.Close()
 			continue
 		}
 
 		if _, err := io.Copy(tf, resp.Body); err != nil {
-			tf.Close()
 			return nil, err
 		}
 		if o, err := tf.Seek(0, io.SeekStart); err != nil || o != 0 {
-			tf.Close()
 			return nil, err
 		}
 		gz, err := gzip.NewReader(tf)
@@ -151,6 +156,7 @@ func (c *Client) Updates(ctx context.Context) (io.ReadCloser, error) {
 		}
 
 		zlog.Debug(ctx).Msg("success")
+		success = true
 		return &gzippedFile{
 			Reader: gz,
 			Closer: tf,

diff --git a/debian/updater.go b/debian/updater.go
@@ -389,21 +389,27 @@ func (u *updater) Fetch(ctx context.Context, fingerprint driver.Fingerprint) (io
 	if err != nil {
 		return nil, "", err
 	}
+	var success bool
+	defer func() {
+		if !success {
+			if err := f.Close(); err != nil {
+				zlog.Warn(ctx).Err(err).Msg("unable to close spool")
+			}
+		}
+	}()
 	if _, err := io.Copy(f, resp.Body); err != nil {
-		f.Close()
 		return nil, "", fmt.Errorf("failed to read http body: %v", err)
 	}
 	if _, err := f.Seek(0, io.SeekStart); err != nil {
-		f.Close()
 		return nil, "", fmt.Errorf("failed to seek body: %v", err)
 	}
 	zlog.Info(ctx).Msg("fetched latest oval database successfully")
 
-	err = u.sm.Update(ctx)
-	if err != nil {
+	if err := u.sm.Update(ctx); err != nil {
 		return nil, "", fmt.Errorf("could not update source to binary map: %w", err)
 	}
 	zlog.Info(ctx).Msg("updated the debian source to binary map successfully")
+	success = true
 
 	return f, driver.Fingerprint(fp), err
 }

diff --git a/enricher/cvss/cvss.go b/enricher/cvss/cvss.go
@@ -172,6 +172,14 @@ func (e *Enricher) FetchEnrichment(ctx context.Context, hint driver.Fingerprint)
 	if err != nil {
 		return nil, hint, err
 	}
+	var success bool
+	defer func() {
+		if !success {
+			if err := out.Close(); err != nil {
+				zlog.Warn(ctx).Err(err).Msg("unable to close spool")
+			}
+		}
+	}()
 	// Doing this serially is slower, but much less complicated than using an
 	// ErrGroup or the like.
 	//
@@ -211,6 +219,7 @@ func (e *Enricher) FetchEnrichment(ctx context.Context, hint driver.Fingerprint)
 	if _, err := out.Seek(0, io.SeekStart); err != nil {
 		return nil, hint, fmt.Errorf("unable to reset item feed: %w", err)
 	}
+	success = true
 
 	nh, err := json.Marshal(cur)
 	if err != nil {

diff --git a/rhel/rhcc/updater.go b/rhel/rhcc/updater.go
@@ -141,22 +141,29 @@ func (u *updater) Fetch(ctx context.Context, hint driver.Fingerprint) (io.ReadCl
 	zlog.Debug(ctx).
 		Str("name", tf.Name()).
 		Msg("created tempfile")
+	var success bool
+	defer func() {
+		if !success {
+			if err := tf.Close(); err != nil {
+				zlog.Warn(ctx).Err(err).Msg("unable to close spool")
+			}
+		}
+	}()
 
 	var r io.Reader = res.Body
 	if u.bzipped {
 		// No cleanup/pooling.
 		r = bzip2.NewReader(res.Body)
 	}
 	if _, err := io.Copy(tf, r); err != nil {
-		tf.Close()
 		return nil, hint, fmt.Errorf("rhcc: unable to copy resp body to tempfile: %w", err)
 	}
 	if n, err := tf.Seek(0, io.SeekStart); err != nil || n != 0 {
-		tf.Close()
 		return nil, hint, fmt.Errorf("rhcc: unable to seek database to start: %w", err)
 	}
 	zlog.Debug(ctx).Msg("decompressed and buffered database")
 
+	success = true
 	hint = driver.Fingerprint(res.Header.Get("etag"))
 	zlog.Debug(ctx).
 		Str("hint", string(hint)).

diff --git a/ubuntu/updater.go b/ubuntu/updater.go
@@ -116,19 +116,26 @@ func (u *updater) Fetch(ctx context.Context, fingerprint driver.Fingerprint) (io
 	if err != nil {
 		return nil, "", err
 	}
+	var success bool
+	defer func() {
+		if !success {
+			if err := f.Close(); err != nil {
+				zlog.Warn(ctx).Err(err).Msg("unable to close spool")
+			}
+		}
+	}()
 	var r io.Reader = resp.Body
 	if u.useBzip2 {
 		r = bzip2.NewReader(r)
 	}
 	if _, err := io.Copy(f, r); err != nil {
-		f.Close()
 		return nil, "", fmt.Errorf("ubuntu: failed to read http body: %w", err)
 	}
 	if _, err := f.Seek(0, io.SeekStart); err != nil {
-		f.Close()
 		return nil, "", fmt.Errorf("ubuntu: failed to seek body: %w", err)
 	}
 
+	success = true
 	zlog.Info(ctx).Msg("fetched latest oval database successfully")
 	return f, driver.Fingerprint(fp), err
 }