From 242037e16efa75ffbea08c0c424607706c6f8863 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82?= Date: Wed, 9 Sep 2026 14:56:58 +0200 Subject: [PATCH 1/2] Handle an aws.ServeFile error gracefully instead of panicking ServeFile previously called helper.Check(err) on the result of aws.ServeFile, which panics on any error (e.g. the object having gone missing from the bucket between metadata lookup and download). Log the error and write a plain error response instead. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_0168YUwGEzEHTqd9CwzXE53d --- internal/storage/FileServing.go | 6 ++++-- internal/storage/FileServing_test.go | 31 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/internal/storage/FileServing.go b/internal/storage/FileServing.go index 64ce9f70..8d5516ac 100644 --- a/internal/storage/FileServing.go +++ b/internal/storage/FileServing.go @@ -647,8 +647,10 @@ func ServeFile(file models.File, w http.ResponseWriter, r *http.Request, forceDo // confirm that the file has been completely downloaded. It expires automatically after 24 hours. statusId := downloadstatus.SetDownload(file) isBlocking, err := aws.ServeFile(w, r, file, forceDownload, forceDecryption) - // TODO chances are high that an error is returned here, we should consider proper output - helper.Check(err) + if err != nil { + fmt.Println(err) + _, _ = w.Write([]byte("Error serving file")) + } if isBlocking { downloadstatus.SetComplete(statusId) } diff --git a/internal/storage/FileServing_test.go b/internal/storage/FileServing_test.go index d8369d62..1c38b8a8 100644 --- a/internal/storage/FileServing_test.go +++ b/internal/storage/FileServing_test.go @@ -625,6 +625,37 @@ func TestServeFile(t *testing.T) { test.ResponseBodyContains(t, w, "Error decrypting file") } +func TestServeFileAwsErrorHandling(t *testing.T) { + if !aws.IsIncludedInBuild { + t.Skip("AWS support not included in build") + } + testconfiguration.EnableS3() + config, ok := cloudconfig.Load() + test.IsEqualBool(t, ok, true) + ok = aws.Init(config.Aws) + test.IsEqualBool(t, ok, true) + + // A file with an AWS bucket set, but never actually uploaded, causes aws.ServeFile to + // return an error. ServeFile must handle that gracefully instead of panicking. + file := models.File{ + Id: "awsErrorHandlingTest1", + Name: "aws error handling test", + AwsBucket: "gokapi-test", + SHA1: "nonexistentAwsObjectSha1", + ExpireAt: time.Now().Add(time.Hour).Unix(), + SizeBytes: 10, + } + database.SaveMetaData(file) + + r := httptest.NewRequest("GET", "/", nil) + w := httptest.NewRecorder() + ServeFile(file, w, r, false, true, false, false) + test.ResponseBodyContains(t, w, "Error serving file") + + database.DeleteMetaData(file.Id) + testconfiguration.DisableS3() +} + func TestCleanUp(t *testing.T) { files := database.GetAllMetadata() downloadstatus.DeleteAll() From d2db204676d1580c5d7423044c8892dc88d3c2b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82?= Date: Sun, 13 Sep 2026 11:01:01 +0200 Subject: [PATCH 2/2] Fix test to actually exercise the AWS error path forceDecryption=false made aws.ServeFile take the redirectToDownload path, which only presigns a URL without contacting S3, so it never returns an error for a missing object. Setting forceDecryption=true routes through serveDecryptedFile, which calls s3.GetObject directly and surfaces the missing-object error the test is meant to cover. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01JWZJU5aSCEBB9DHBWvL4t4 --- internal/storage/FileServing_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/storage/FileServing_test.go b/internal/storage/FileServing_test.go index 1c38b8a8..e5bc8129 100644 --- a/internal/storage/FileServing_test.go +++ b/internal/storage/FileServing_test.go @@ -637,6 +637,9 @@ func TestServeFileAwsErrorHandling(t *testing.T) { // A file with an AWS bucket set, but never actually uploaded, causes aws.ServeFile to // return an error. ServeFile must handle that gracefully instead of panicking. + // forceDecryption is set to true so aws.ServeFile takes the serveDecryptedFile path, + // which calls s3.GetObject directly instead of just presigning a redirect URL - only + // that path actually contacts S3 and surfaces the missing-object error. file := models.File{ Id: "awsErrorHandlingTest1", Name: "aws error handling test", @@ -649,7 +652,7 @@ func TestServeFileAwsErrorHandling(t *testing.T) { r := httptest.NewRequest("GET", "/", nil) w := httptest.NewRecorder() - ServeFile(file, w, r, false, true, false, false) + ServeFile(file, w, r, false, true, true, false) test.ResponseBodyContains(t, w, "Error serving file") database.DeleteMetaData(file.Id)