repair artifacts commands use a 1min timeout and has a test case.

Artifact handler now does not panic with 404 errors.

handler/app/dirs.go

@@ -73,7 +73,7 @@ type Dirs struct {
 func (dir Dirs) Artifact(c echo.Context, db *sql.DB, logger *zap.SugaredLogger, readonly bool) error {
 	const name = "artifact"
 	art, err := dir.modelsFile(c, db)
-	if err != nil {
+	if art404 := art == nil || err != nil; art404 {
 		return err
 	}
 	data := empty(c)
@@ -113,6 +113,9 @@ func (dir Dirs) Artifact(c echo.Context, db *sql.DB, logger *zap.SugaredLogger,
 }
 
 func (dir Dirs) embed(art *models.File, data map[string]interface{}) (map[string]interface{}, error) {
+	if art == nil {
+		return data, nil
+	}
 	p, err := readme.Read(art, dir.Download, dir.Extra)
 	if err != nil {
 		if errors.Is(err, render.ErrDownload) {
@@ -247,6 +250,9 @@ func (dir Dirs) assets(nameDir, unid string) map[string][2]string {
 
 // missingAssets returns a string of missing assets for the file record of the artifact.
 func (dir Dirs) missingAssets(art *models.File) string {
+	if art == nil {
+		return ""
+	}
 	uid := art.UUID.String
 	missing := []string{}
 	dl := helper.File(filepath.Join(dir.Download, uid))
@@ -276,6 +282,9 @@ func (dir Dirs) missingAssets(art *models.File) string {
 
 // attributions returns the author attributions for the file record of the artifact.
 func (dir Dirs) attributions(art *models.File, data map[string]interface{}) map[string]interface{} {
+	if art == nil {
+		return data
+	}
 	data["writers"] = filerecord.AttrWriter(art)
 	data["artists"] = filerecord.AttrArtist(art)
 	data["programmers"] = filerecord.AttrProg(art)
@@ -285,6 +294,9 @@ func (dir Dirs) attributions(art *models.File, data map[string]interface{}) map[
 
 // filemetadata returns the file metadata for the file record of the artifact.
 func (dir Dirs) filemetadata(art *models.File, data map[string]interface{}) map[string]interface{} {
+	if art == nil {
+		return data
+	}
 	data["filename"] = filerecord.Basename(art)
 	data["filesize"] = simple.BytesHuman(art.Filesize.Int64)
 	data["filebyte"] = art.Filesize
@@ -303,6 +315,9 @@ func (dir Dirs) filemetadata(art *models.File, data map[string]interface{}) map[
 
 // otherRelations returns the other relations and external links for the file record of the artifact.
 func (dir Dirs) otherRelations(art *models.File, data map[string]interface{}) map[string]interface{} {
+	if art == nil {
+		return data
+	}
 	data["relations"] = filerecord.Relations(art)
 	data["websites"] = filerecord.Websites(art)
 	data["demozoo"] = filerecord.IdenficationDZ(art)
@@ -405,7 +420,7 @@ func errorWithID(err error, key string, id any) error {
 
 // embedText embeds the readme or file download text content for the file record of the artifact.
 func embedText(art *models.File, data map[string]interface{}, b ...byte) (map[string]interface{}, error) {
-	if len(b) == 0 {
+	if len(b) == 0 || art == nil {
 		return data, nil
 	}
 	const (
@@ -482,6 +497,9 @@ func decode(src io.Reader) (string, error) {
 
 // firstLead returns the lead for the file record which is the filename and releasers.
 func firstLead(art *models.File) string {
+	if art == nil {
+		return ""
+	}
 	fname := art.Filename.String
 	span := fmt.Sprintf("<span class=\"font-monospace fs-6 fw-light\">%s</span> ", fname)
 	return fmt.Sprintf("%s<br>%s", releasersHrefs(art), span)

internal/config/config_test.go

@@ -4,9 +4,13 @@ import (
 	"bytes"
 	"context"
 	"os"
+	"path/filepath"
 	"testing"
 
+	"github.com/Defacto2/helper"
+	"github.com/Defacto2/magicnumber"
 	"github.com/Defacto2/server/internal/config"
+	"github.com/Defacto2/server/internal/zaplog"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -123,3 +127,38 @@ func TestRepair(t *testing.T) {
 	err = config.RemoveImage("", "", "")
 	require.Error(t, err)
 }
+
+func TestReArchive(t *testing.T) {
+	t.Parallel()
+	r := config.Zip
+	logger := zaplog.Timestamp().Sugar()
+	ctx := context.WithValue(context.Background(), helper.LoggerKey, logger)
+	err := r.ReArchive(ctx, "", "", "")
+	require.Error(t, err)
+
+	// test the archive that uses the defunct implode method
+	src, err := filepath.Abs(filepath.Join("testdata", "IMPLODE.ZIP"))
+	require.NoError(t, err)
+	readr, err := os.Open(src)
+	require.NoError(t, err)
+	defer readr.Close()
+	sign := magicnumber.Find(readr)
+	assert.Equal(t, magicnumber.PKWAREZipImplode, sign)
+
+	err = r.ReArchive(ctx, src, "", "")
+	require.Error(t, err)
+	dst := filepath.Dir(src)
+	err = r.ReArchive(ctx, src, dst, "")
+	require.Error(t, err)
+	err = r.ReArchive(ctx, src, dst, "newfile")
+	require.NoError(t, err)
+
+	// test the new, re-created archive that uses the common deflate method
+	name := filepath.Join(dst, "newfile.zip")
+	readr, err = os.Open(name)
+	require.NoError(t, err)
+	defer readr.Close()
+	sign = magicnumber.Find(readr)
+	assert.Equal(t, magicnumber.PKWAREZip, sign)
+	defer os.Remove(name)
+}

internal/config/repair.go

@@ -64,7 +64,7 @@ func (c Config) Archives(ctx context.Context, exec boil.ContextExecutor) error {
 		if uid == "" || fixzip.Invalid(ctx, path) {
 			return nil
 		}
-		if err := Zip.rearchive(ctx, path, c.AbsExtra, uid); err != nil {
+		if err := Zip.ReArchive(ctx, path, c.AbsExtra, uid); err != nil {
 			return fmt.Errorf("zip repair and re-archive: %w", err)
 		}
 		return nil
@@ -77,7 +77,7 @@ func (c Config) Archives(ctx context.Context, exec boil.ContextExecutor) error {
 		if uid == "" || fixlha.Invalid(ctx, path) {
 			return nil
 		}
-		if err := LHA.rearchive(ctx, path, c.AbsExtra, uid); err != nil {
+		if err := LHA.ReArchive(ctx, path, c.AbsExtra, uid); err != nil {
 			return fmt.Errorf("lha/lzh repair and re-archive: %w", err)
 		}
 		return nil
@@ -90,7 +90,7 @@ func (c Config) Archives(ctx context.Context, exec boil.ContextExecutor) error {
 		if uid == "" || fixarc.Invalid(ctx, path) {
 			return nil
 		}
-		if err := Arc.rearchive(ctx, path, c.AbsExtra, uid); err != nil {
+		if err := Arc.ReArchive(ctx, path, c.AbsExtra, uid); err != nil {
 			return fmt.Errorf("arc repair and re-archive: %w", err)
 		}
 		return nil
@@ -103,7 +103,7 @@ func (c Config) Archives(ctx context.Context, exec boil.ContextExecutor) error {
 		if uid == "" || fixarj.Invalid(ctx, path) {
 			return nil
 		}
-		if err := Arj.rearchive(ctx, path, c.AbsExtra, uid); err != nil {
+		if err := Arj.ReArchive(ctx, path, c.AbsExtra, uid); err != nil {
 			return fmt.Errorf("arj repair and re-archive: %w", err)
 		}
 		return nil
@@ -209,11 +209,14 @@ func (r Repair) artifacts(ctx context.Context, exec boil.ContextExecutor, logger
 	return artifacts, nil
 }
 
-func (r Repair) rearchive(ctx context.Context, path, extra, uid string) error {
+func (r Repair) ReArchive(ctx context.Context, src, destDir, uid string) error {
+	if src == "" || destDir == "" || uid == "" {
+		return fmt.Errorf("rearchive %s %w: %q %q %q", r, ErrEmpty, src, destDir, uid)
+	}
 	logger := helper.Logger(ctx)
 	tmp, err := os.MkdirTemp(helper.TmpDir(), "rearchive-")
 	if err != nil {
-		return fmt.Errorf("rearchive mkdir temp %w: %s", err, path)
+		return fmt.Errorf("rearchive mkdir temp %w: %s", err, src)
 	}
 	defer os.RemoveAll(tmp)
 
@@ -228,12 +231,14 @@ func (r Repair) rearchive(ctx context.Context, path, extra, uid string) error {
 	case Arj:
 		extractCmd, extractArg = command.Zip7, "x"
 	}
-	cmd := exec.Command(extractCmd, extractArg, path)
+	ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
+	defer cancel()
+	cmd := exec.CommandContext(ctx, extractCmd, extractArg, src)
 	cmd.Dir = tmp
-	if err = cmd.Run(); err != nil {
-		return fmt.Errorf("rearchive run %w: %s", err, path)
+	if stdoutStderr, err := cmd.CombinedOutput(); err != nil {
+		return fmt.Errorf("rearchive run %w: %s: dump: %q",
+			err, src, stdoutStderr)
 	}
-
 	c, err := helper.Count(tmp)
 	if err != nil {
 		return fmt.Errorf("rearchive tmp count %w: %s", err, tmp)
@@ -252,7 +257,7 @@ func (r Repair) rearchive(ctx context.Context, path, extra, uid string) error {
 		return nil
 	}
 
-	finalArc := filepath.Join(extra, basename)
+	finalArc := filepath.Join(destDir, basename)
 	if err = helper.RenameCrossDevice(tmpArc, finalArc); err != nil {
 		defer os.RemoveAll(tmpArc)
 		return fmt.Errorf("rearchive rename %w: %s", err, tmpArc)