From cb3065da1d2a8d07b2f9136d6a04ee50a5ece058 Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Tue, 29 Sep 2026 01:26:24 +0800 Subject: [PATCH] =?UTF-8?q?fix(files):=20=E6=A8=A1=E7=BB=84=E5=8C=85?= =?UTF-8?q?=E4=B8=8A=E4=BC=A0=E4=B9=9F=E9=80=90=E6=AE=B5=E6=A0=B8=E5=AF=B9?= =?UTF-8?q?=20SHA-256=E3=80=81=E9=95=BF=E6=96=87=E4=BB=B6=E5=90=8D?= =?UTF-8?q?=E8=83=BD=E5=86=99=E3=80=81=E5=8D=A1=E4=BD=8F=E7=9A=84=E5=AF=BC?= =?UTF-8?q?=E5=87=BA=E6=8C=89=E6=97=B6=E5=81=9C=E6=8E=89=E3=80=81=E4=B8=96?= =?UTF-8?q?=E7=95=8C=E8=A2=AB=E5=8D=A0=E7=94=A8=E6=97=B6=E6=96=87=E4=BB=B6?= =?UTF-8?q?=E9=A1=B5=E8=AF=B4=E6=98=8E=E5=8E=9F=E5=9B=A0=E5=B9=B6=E7=AD=89?= =?UTF-8?q?=E5=AE=83=E7=BB=93=E6=9D=9F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- cmd/felis/api.go | 24 +++ cmd/felis/api_test.go | 25 +++ cmd/felis/export.go | 38 +++- cmd/felis/export_test.go | 27 ++- cmd/felis/files.go | 36 +++- cmd/felis/files_test.go | 94 ++++++++- cmd/felis/memlimit.go | 48 +++++ cmd/felis/memlimit_test.go | 58 ++++++ docs/openapi.yaml | 158 ++++++++++++--- internal/api/api.go | 4 +- internal/api/exports.go | 115 +++++++++-- internal/api/exports_test.go | 139 ++++++++++++-- internal/api/handlers_fileops.go | 79 ++++++-- internal/api/handlers_fileops_test.go | 94 ++++++++- internal/api/handlers_files.go | 115 ++++++++++- internal/api/handlers_files_test.go | 121 +++++++++++- internal/api/jobstatus_test.go | 7 + internal/api/k8sjobstatus.go | 12 +- internal/api/maintenance.go | 2 +- internal/api/submissions.go | 21 +- internal/api/submissions_chunked_test.go | 47 ++++- internal/api/submissions_test.go | 35 ++-- internal/fileedit/editor.go | 7 +- internal/fileedit/exec.go | 32 +++- internal/fileedit/exec_test.go | 23 ++- internal/fileedit/k8sjobs.go | 36 +++- internal/fileedit/k8sjobs_test.go | 43 ++++- internal/fileedit/manage_test.go | 39 ++-- internal/fileedit/session.go | 84 ++++++-- internal/fileedit/session_test.go | 127 +++++++++--- internal/fileedit/stage.go | 29 ++- internal/fileedit/stage_test.go | 63 ++++-- internal/fileedit/unzip.go | 46 +++-- internal/fileedit/unzip_test.go | 23 +++ internal/submit/digest.go | 41 ++++ internal/submit/digest_test.go | 126 ++++++++++++ internal/worldexport/jobspec.go | 11 ++ internal/worldexport/jobspec_test.go | 34 ++++ internal/worldexport/worldexport.go | 13 ++ panel/src/components/files/names.ts | 8 +- panel/src/components/files/opText.test.ts | 15 ++ panel/src/components/files/opText.ts | 11 +- .../components/files/sessionUpload.test.ts | 86 ++++++++- panel/src/components/files/sessionUpload.ts | 53 +++++- panel/src/components/files/useUploads.ts | 6 +- panel/src/components/files/useWorldJobs.ts | 76 ++++++++ panel/src/i18n/resources/en-US/errors.json | 8 +- panel/src/i18n/resources/en-US/files.json | 12 +- panel/src/i18n/resources/zh-CN/errors.json | 8 +- panel/src/i18n/resources/zh-CN/files.json | 12 +- panel/src/lib/api.test.ts | 50 +++-- panel/src/lib/api.ts | 73 ++++--- panel/src/lib/contextUpload.test.ts | 15 ++ panel/src/lib/contextUpload.ts | 6 +- panel/src/lib/digest.ts | 24 +++ panel/src/lib/openapi.gen.ts | 103 ++++++++-- panel/src/lib/types.ts | 4 +- panel/src/pages/ServerFiles.test.tsx | 180 +++++++++++++++++- panel/src/pages/ServerFiles.tsx | 50 ++++- 59 files changed, 2538 insertions(+), 338 deletions(-) create mode 100644 cmd/felis/memlimit.go create mode 100644 cmd/felis/memlimit_test.go create mode 100644 internal/submit/digest.go create mode 100644 internal/submit/digest_test.go create mode 100644 panel/src/components/files/useWorldJobs.ts create mode 100644 panel/src/lib/digest.ts diff --git a/cmd/felis/api.go b/cmd/felis/api.go index 1ce4be8..a010d4a 100644 --- a/cmd/felis/api.go +++ b/cmd/felis/api.go @@ -494,6 +494,9 @@ func cmdAPI(args []string, stdout, stderr io.Writer) int { if fileStage != nil { go expireFileSessions(ctx, fileStage, fileSessionSweep, stderr) } + if exporter != nil { + go expireExports(ctx, a, exportSweep) + } go retention.Loop(ctx, drv.DB(), retention.Policy{Audit: auditRetention}, retentionInterval, slog.Default()) servers := []*http.Server{internalSrv, externalSrv} @@ -824,6 +827,27 @@ func expireFileSessions(ctx context.Context, s *fileedit.Stage, every time.Durat } } +// exportSweep is how often expireExports runs: an export whose Job never +// connected is stopped within a minute of going stale. +const exportSweep = time.Minute + +// expireExports runs the export sweep (api.API.ExpireExports) on a ticker. The +// export routes sweep as they are called, and an owner who closed the tab calls +// none; a Job whose Pod never got going would then keep the server from +// starting until the Job's deadline. +func expireExports(ctx context.Context, a interface{ ExpireExports() }, every time.Duration) { + t := time.NewTicker(every) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return + case <-t.C: + a.ExpireExports() + } + } +} + // reapRejectedContexts deletes, once an hour, the uploaded contexts of // submissions rejected more than submit.RejectedContextRetention ago, and the // chunked uploads left untouched for submit.StalePartRetention. Without it a diff --git a/cmd/felis/api_test.go b/cmd/felis/api_test.go index 59895d7..991e821 100644 --- a/cmd/felis/api_test.go +++ b/cmd/felis/api_test.go @@ -286,3 +286,28 @@ func TestExpireFileSessions(t *testing.T) { t.Fatalf("said %q", got) } } + +type sweepCount struct{ n atomic.Int32 } + +func (s *sweepCount) ExpireExports() { s.n.Add(1) } + +// TestExpireExports: the loop sweeps on each tick, and returns once felis-api +// shuts down. +func TestExpireExports(t *testing.T) { + var s sweepCount + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { expireExports(ctx, &s, time.Millisecond); close(done) }() + for deadline := time.Now().Add(5 * time.Second); s.n.Load() < 3; time.Sleep(time.Millisecond) { + if time.Now().After(deadline) { + cancel() + t.Fatalf("swept %d times in 5s at a 1ms tick", s.n.Load()) + } + } + cancel() + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("the loop outlived its context") + } +} diff --git a/cmd/felis/export.go b/cmd/felis/export.go index 398ced4..ab6fb3f 100644 --- a/cmd/felis/export.go +++ b/cmd/felis/export.go @@ -3,6 +3,7 @@ package main import ( "context" "crypto/sha256" + "encoding/base64" "encoding/hex" "encoding/json" "errors" @@ -14,6 +15,7 @@ import ( "net/http" "os" "os/signal" + "strconv" "strings" "syscall" "time" @@ -67,6 +69,7 @@ func cmdExport(args []string, stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "felis export: --target-url and %s are required\n", worldexport.TokenEnv) return 2 } + limitHeapToCgroup() ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() @@ -222,19 +225,30 @@ func (d *digestReader) Read(p []byte) (int, error) { return n, err } -// streamExport runs write straight into the body of the PUT. An error from -// write aborts the chunked body, and felis-api then cuts the browser's download -// off rather than end it; that error is the one reported, since the PUT's own -// error only wraps it. When the PUT ends first, write is stopped. +// streamExport runs write straight into the body of the PUT, hashing it as it +// goes. Once write has finished, the SHA-256 of all it wrote rides the +// request's trailer (worldexport.DigestTrailer), and felis-api holds back the +// last bytes from the browser until what it received hashes the same. An error +// from write aborts the chunked body before the trailer, and felis-api then +// cuts the browser's download off rather than end it; that error is the one +// reported, since the PUT's own error only wraps it. When the PUT ends first, +// write is stopped. func streamExport(ctx context.Context, target, token, contentType string, size int64, write func(io.Writer) error) error { pr, pw := io.Pipe() + trailer := http.Header{worldexport.DigestTrailer: nil} werr := make(chan error, 1) go func() { - err := write(pw) + sum := sha256.New() + err := write(io.MultiWriter(pw, sum)) + if err == nil { + // Set before the body ends: the transport reads the trailer once it + // has read the body to its end. + trailer.Set(worldexport.DigestTrailer, "sha-256=:"+base64.StdEncoding.EncodeToString(sum.Sum(nil))+":") + } pw.CloseWithError(err) werr <- err }() - err := putExport(ctx, target, token, contentType, pr, size) + err := putExport(ctx, target, token, contentType, pr, size, trailer) pr.CloseWithError(io.ErrClosedPipe) if w := <-werr; w != nil && !errors.Is(w, io.ErrClosedPipe) { return w @@ -248,12 +262,20 @@ func streamExport(ctx context.Context, target, token, contentType string, size i // redirects. felis-api answers only after the whole download, which the Job's // activeDeadlineSeconds bounds, so the header timeout is a backstop for a // wedged endpoint and not the real limit. -func putExport(ctx context.Context, target, token, contentType string, body io.Reader, size int64) error { +// +// The body always goes chunked, which is what lets it end with a trailer; a +// size the Job knows (-1 when it does not) goes as worldexport.LengthHeader in +// place of Content-Length. +func putExport(ctx context.Context, target, token, contentType string, body io.Reader, size int64, trailer http.Header) error { req, err := http.NewRequestWithContext(ctx, http.MethodPut, target, body) if err != nil { return err } - req.ContentLength = size + req.ContentLength = -1 + req.Trailer = trailer + if size >= 0 { + req.Header.Set(worldexport.LengthHeader, strconv.FormatInt(size, 10)) + } req.Header.Set("Authorization", "Bearer "+token) req.Header.Set("Content-Type", contentType) client := &http.Client{ diff --git a/cmd/felis/export_test.go b/cmd/felis/export_test.go index b05cd5e..e25cb55 100644 --- a/cmd/felis/export_test.go +++ b/cmd/felis/export_test.go @@ -8,6 +8,7 @@ import ( "context" "crypto/rand" "crypto/sha256" + "encoding/base64" "encoding/hex" "errors" "io" @@ -54,6 +55,25 @@ func receiveExport(t *testing.T, reply func(w http.ResponseWriter)) *exportRecei func noContent(w http.ResponseWriter) { w.WriteHeader(http.StatusNoContent) } +// sentWhole fails unless the upload rcv got ended with the Content-Digest +// trailer of its own bytes, and declared length as its size (-1: none). +func sentWhole(t *testing.T, rcv *exportReceiver, length int64) { + t.Helper() + sum := sha256.Sum256(rcv.body) + want := "sha-256=:" + base64.StdEncoding.EncodeToString(sum[:]) + ":" + wantLength := "" + if length >= 0 { + wantLength = strconv.FormatInt(length, 10) + } + r := rcv.req + if rcv.readErr != nil || r.Trailer.Get(worldexport.DigestTrailer) != want || r.Header.Get(worldexport.LengthHeader) != wantLength || + r.ContentLength != -1 || strings.Join(r.TransferEncoding, ",") != "chunked" { + t.Fatalf("upload read %v, trailer %v, %s %q, length %d, encoding %v; want trailer %q and %s %q, chunked", + rcv.readErr, r.Trailer, worldexport.LengthHeader, r.Header.Get(worldexport.LengthHeader), r.ContentLength, r.TransferEncoding, + want, worldexport.LengthHeader, wantLength) + } +} + func tarEntries(t *testing.T, archive []byte) map[string]string { t.Helper() gz, err := gzip.NewReader(bytes.NewReader(archive)) @@ -141,6 +161,7 @@ func TestCmdExportWorld(t *testing.T) { if got := tarEntries(t, rcv.body); !reflect.DeepEqual(got, want) { t.Fatalf("archive holds %v\nwant %v", got, want) } + sentWhole(t, rcv, -1) want2 := "felis export: left out 1 entries a tar cannot hold (symbolic links, devices, sockets)\n" + "felis export: left out 2 files that hold platform secrets\n" + "felis export: server=survival mode=world downloaded\n" @@ -297,6 +318,7 @@ func TestCmdExportBackup(t *testing.T) { if got := tarEntries(t, rcv.body); !reflect.DeepEqual(got, want) { t.Fatalf("archive holds %d entries, want exactly the redacted properties, level.dat and the region file", len(got)) } + sentWhole(t, rcv, -1) if want := "felis export: left out 1 files that hold platform secrets\nfelis export: server=survival mode=backup downloaded\n"; stdout.String() != want { t.Errorf("stdout = %q, want %q", stdout.String(), want) } @@ -417,9 +439,10 @@ func TestCmdExportFiles(t *testing.T) { if code != 0 || stdout != "felis export: server=survival mode=files downloaded\n" { t.Fatalf("exit %d, stdout %q, stderr %q", code, stdout, stderr) } - if string(rcv.body) != want || rcv.req.ContentLength != int64(len(want)) || rcv.req.Header.Get("Content-Type") != "application/octet-stream" { - t.Fatalf("body %q, length %d, type %q; want %q", rcv.body, rcv.req.ContentLength, rcv.req.Header.Get("Content-Type"), want) + if string(rcv.body) != want || rcv.req.Header.Get("Content-Type") != "application/octet-stream" { + t.Fatalf("body %q, type %q; want %q", rcv.body, rcv.req.Header.Get("Content-Type"), want) } + sentWhole(t, rcv, int64(len(want))) }) } diff --git a/cmd/felis/files.go b/cmd/felis/files.go index 1e7b11b..6ae49e6 100644 --- a/cmd/felis/files.go +++ b/cmd/felis/files.go @@ -57,6 +57,7 @@ func cmdFiles(args []string, stdout, stderr io.Writer) int { fmt.Fprintln(stderr, "felis files: --op is required") return 2 } + limitHeapToCgroup() req := fileedit.Request{ Op: *op, Path: *path, To: *to, Expect: *expect, CreateOnly: *createOnly, Overwrite: *overwrite, } @@ -86,6 +87,13 @@ func cmdFiles(args []string, stdout, stderr io.Writer) int { req.Upload = &fileedit.Upload{ Size: *size, SHA256: *sum, Open: func() (io.ReadCloser, error) { return fetchUpload(ctx, *sourceURL, token) }, + Landed: func() { + if err := reportLanded(ctx, *sourceURL, token); err != nil { + // The file is in place; felis-api drops its copy when it + // has sat idle long enough, and the panel cancels it too. + fmt.Fprintf(stderr, "felis files: tell felis-api the upload landed: %v\n", err) + } + }, } } // An upload or an unzip (the only ops that report progress) can run long @@ -112,7 +120,7 @@ func cmdFiles(args []string, stdout, stderr io.Writer) int { // fetchUpload opens the staged upload on felis-api's internal face. There is no // retry: the token opens the upload once (fileedit.Stage), so a second attempt // could only be refused, and the caller retries the failed Job whole (a file -// sent in parts stays staged until it has been served whole once, so that retry +// sent in parts stays staged until its Job reports it landed, so that retry // does not send it again). Redirects are refused because the request carries the // token and the internal face never redirects; the header timeout catches a // wedged endpoint, and the Job's activeDeadlineSeconds bounds the body. @@ -136,3 +144,29 @@ func fetchUpload(ctx context.Context, url, token string) (io.ReadCloser, error) } return resp.Body, nil } + +// reportLanded tells felis-api the upload's file is in place (DELETE on the URL +// it was fetched from, with the same token), so it deletes the copy it staged. +// One try: the file has landed whatever the answer, and a copy nobody deletes +// is dropped once it has sat idle for fileedit.SessionIdle. +func reportLanded(ctx context.Context, url, token string) error { + ctx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, url, nil) + if err != nil { + return err + } + req.Header.Set("Authorization", "Bearer "+token) + client := &http.Client{ + CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }, + } + resp, err := client.Do(req) + if err != nil { + return err + } + resp.Body.Close() + if resp.StatusCode != http.StatusNoContent { + return fmt.Errorf("DELETE returned %s", resp.Status) + } + return nil +} diff --git a/cmd/felis/files_test.go b/cmd/felis/files_test.go index 8c76054..b27c0e9 100644 --- a/cmd/felis/files_test.go +++ b/cmd/felis/files_test.go @@ -3,6 +3,7 @@ package main import ( "archive/zip" "bytes" + "cmp" "crypto/sha256" "encoding/base64" "encoding/hex" @@ -35,19 +36,43 @@ func filesResult(t *testing.T, stdout string) fileedit.Result { return res } -// stagedUpload serves body to a request carrying Bearer token, and 404 to any -// other, the way felis-api's internal face does. -func stagedUpload(t *testing.T, token string, body []byte) *httptest.Server { +// stagedSource is felis-api's internal face for one staged upload. It serves +// body to a GET carrying Bearer token and 404 to any other, and answers the +// DELETE that reports the file landed with landedCode (204 when unset), +// redirecting to landedTo when that is a redirect. reports counts those +// DELETEs, each with the token and at the path the bytes came from. +type stagedSource struct { + *httptest.Server + reports, strays atomic.Int32 + landedCode int + landedTo string +} + +func stagedUpload(t *testing.T, token string, body []byte) *stagedSource { t.Helper() - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Header.Get("Authorization") != "Bearer "+token { + s := &stagedSource{} + s.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer "+token || r.URL.Path != "/u" { + s.strays.Add(1) http.Error(w, "no such upload", http.StatusNotFound) return } - w.Write(body) + switch r.Method { + case http.MethodGet: + w.Write(body) + case http.MethodDelete: + s.reports.Add(1) + if s.landedTo != "" { + w.Header().Set("Location", s.landedTo) + } + w.WriteHeader(cmp.Or(s.landedCode, http.StatusNoContent)) + default: + s.strays.Add(1) + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + } })) - t.Cleanup(srv.Close) - return srv + t.Cleanup(s.Close) + return s } func uploadArgs(root, sourceURL string, body []byte) []string { @@ -89,8 +114,58 @@ func TestCmdFilesUpload(t *testing.T) { if err != nil || !bytes.Equal(got, body) { t.Fatalf("landed %q, %v", got, err) } + if n, strays := srv.reports.Load(), srv.strays.Load(); n != 1 || strays != 0 || stderr.Len() != 0 { + t.Fatalf("reported landed %d times, %d stray requests, stderr %q; want once", n, strays, stderr.String()) + } }) + t.Run("a file already there is a result, and nothing is reported landed", func(t *testing.T) { + root := uploadRoot(t) + if err := os.WriteFile(filepath.Join(root, "plugins", "a.jar"), []byte("old!"), 0o644); err != nil { + t.Fatal(err) + } + srv := stagedUpload(t, "tok", body) + t.Setenv(fileedit.UploadTokenEnv, "tok") + var stdout, stderr bytes.Buffer + if code := cmdFiles(uploadArgs(root, srv.URL+"/u", body), &stdout, &stderr); code != 0 { + t.Fatalf("exit %d, stderr %q", code, stderr.String()) + } + if res := filesResult(t, stdout.String()); res.Code != fileedit.CodeExists || srv.reports.Load() != 0 { + t.Fatalf("result = %+v, reported landed %d times", res, srv.reports.Load()) + } + }) + + // The file is in place whatever felis-api answers, so the Job still succeeds + // and says why the staged copy may linger. A redirect is not followed, since + // the request carries the token. + for name, tc := range map[string]struct { + code int + stderr string + }{ + "refused": {http.StatusNotFound, "felis files: tell felis-api the upload landed: DELETE returned 404 Not Found\n"}, + "redirected": {http.StatusFound, "felis files: tell felis-api the upload landed: DELETE returned 302 Found\n"}, + } { + t.Run("a landed report "+name+" still lands the file", func(t *testing.T) { + var elsewhere atomic.Int32 + away := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { elsewhere.Add(1) })) + defer away.Close() + root := uploadRoot(t) + srv := stagedUpload(t, "tok", body) + srv.landedCode, srv.landedTo = tc.code, away.URL+"/u" + t.Setenv(fileedit.UploadTokenEnv, "tok") + var stdout, stderr bytes.Buffer + if code := cmdFiles(uploadArgs(root, srv.URL+"/u", body), &stdout, &stderr); code != 0 { + t.Fatalf("exit %d, stderr %q", code, stderr.String()) + } + if res := filesResult(t, stdout.String()); res.Code != "" || stderr.String() != tc.stderr || elsewhere.Load() != 0 { + t.Fatalf("result = %+v, stderr %q, redirect followed %d times", res, stderr.String(), elsewhere.Load()) + } + if got, err := os.ReadFile(filepath.Join(root, "plugins", "a.jar")); err != nil || !bytes.Equal(got, body) { + t.Fatalf("landed %q, %v", got, err) + } + }) + } + // A refused fetch is the Job failing, never a Result: the API answers it with a // 500 the caller retries whole. t.Run("a refused fetch exits 1 and lands nothing", func(t *testing.T) { @@ -104,6 +179,9 @@ func TestCmdFilesUpload(t *testing.T) { if !strings.Contains(stderr.String(), "404") { t.Fatalf("stderr %q does not name the status", stderr.String()) } + if srv.reports.Load() != 0 { + t.Fatal("a refused fetch was reported landed") + } if _, err := os.Lstat(filepath.Join(root, "plugins", "a.jar")); !os.IsNotExist(err) { t.Fatalf("a refused fetch left a file: %v", err) } diff --git a/cmd/felis/memlimit.go b/cmd/felis/memlimit.go new file mode 100644 index 0000000..1a4b333 --- /dev/null +++ b/cmd/felis/memlimit.go @@ -0,0 +1,48 @@ +package main + +import ( + "os" + "runtime/debug" + "strconv" + "strings" +) + +// cgroupMemoryFiles are where a container reads the memory it is allowed: +// cgroup v2 first, then v1. +var cgroupMemoryFiles = []string{"/sys/fs/cgroup/memory.max", "/sys/fs/cgroup/memory/memory.limit_in_bytes"} + +// limitHeapToCgroup sets the Go heap's soft limit from the container's memory +// limit, so the collector works harder as a Job nears it and the kernel does not +// kill the Job first. An extraction or a folder zipped for download keeps a few +// hundred bytes per entry for as long as it runs; without the limit the heap +// grows to twice that before a collection, and a 256 MiB Job was killed at +// 400,000 entries whose live heap was 115 MB. GOMEMLIMIT set by hand wins. +func limitHeapToCgroup() { + if os.Getenv("GOMEMLIMIT") != "" { + return + } + for _, f := range cgroupMemoryFiles { + b, err := os.ReadFile(f) + if err != nil { + continue + } + if n, ok := softMemoryLimit(string(b)); ok { + debug.SetMemoryLimit(n) + } + return + } +} + +// softMemoryLimit answers three fifths of the limit a cgroup memory file holds, +// or false for "max" (no limit) and anything unreadable. The rest is left for +// what the kernel charges the container beyond the Go heap: the page cache of +// the files it reads and writes, and the inodes it creates. Under a 256 MiB +// limit, 400,000 extracted entries peaked at 184 MB resident with the heap held +// to 150 MiB. +func softMemoryLimit(content string) (int64, bool) { + n, err := strconv.ParseInt(strings.TrimSpace(content), 10, 64) + if err != nil || n <= 0 { + return 0, false + } + return n / 5 * 3, true +} diff --git a/cmd/felis/memlimit_test.go b/cmd/felis/memlimit_test.go new file mode 100644 index 0000000..7798cb8 --- /dev/null +++ b/cmd/felis/memlimit_test.go @@ -0,0 +1,58 @@ +package main + +import ( + "math" + "os" + "path/filepath" + "runtime/debug" + "testing" +) + +func TestSoftMemoryLimit(t *testing.T) { + for _, c := range []struct { + in string + want int64 + ok bool + }{ + {"268435456\n", 161061273, true}, // 256 MiB, as memory.max holds it + {"max\n", 0, false}, + {"0\n", 0, false}, + {"-1", 0, false}, + {"", 0, false}, + } { + got, ok := softMemoryLimit(c.in) + if got != c.want || ok != c.ok { + t.Errorf("softMemoryLimit(%q) = %d %v, want %d %v", c.in, got, ok, c.want, c.ok) + } + } +} + +// TestLimitHeapToCgroup checks the limit comes from the first cgroup file there +// is, and that GOMEMLIMIT set by hand leaves the heap alone. +func TestLimitHeapToCgroup(t *testing.T) { + prevFiles, prevLimit := cgroupMemoryFiles, debug.SetMemoryLimit(-1) + t.Cleanup(func() { cgroupMemoryFiles = prevFiles; debug.SetMemoryLimit(prevLimit) }) + dir := t.TempDir() + v1, v1b := filepath.Join(dir, "v1"), filepath.Join(dir, "v1b") + if err := os.WriteFile(v1, []byte("268435456\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(v1b, []byte("536870912\n"), 0o600); err != nil { + t.Fatal(err) + } + cgroupMemoryFiles = []string{filepath.Join(dir, "missing"), v1, v1b} + + t.Setenv("GOMEMLIMIT", "") + debug.SetMemoryLimit(math.MaxInt64) + limitHeapToCgroup() + if got := debug.SetMemoryLimit(-1); got != 161061273 { + t.Fatalf("limit = %d, want three fifths of 256 MiB", got) + } + + t.Setenv("GOMEMLIMIT", "1GiB") + debug.SetMemoryLimit(math.MaxInt64) + limitHeapToCgroup() + if got := debug.SetMemoryLimit(-1); got != math.MaxInt64 { + t.Fatalf("limit = %d with GOMEMLIMIT set, want it left alone", got) + } +} diff --git a/docs/openapi.yaml b/docs/openapi.yaml index b6fd3e7..e11cf06 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -746,13 +746,25 @@ components: FileUploadSession: type: object description: Where an upload session stands (internal/api/handlers_fileops.go fileSessionView). - required: [id, path, size, received, part_max_bytes] + required: [id, path, size, received, part_max_bytes, parts] properties: id: { type: string, description: 32 hex characters. } path: { type: string, description: Where the file lands, relative to the world root. } size: { type: integer, format: int64, description: The file's length. } received: { type: integer, format: int64, description: Bytes here so far; the next part starts here. } part_max_bytes: { type: integer, format: int64, description: The most one part may carry. } + parts: + type: array + description: >- + The parts taken so far, in order, each with the SHA-256 it arrived + with. A client resuming from a file it still holds hashes the same + ranges and starts over when one differs. + items: + type: object + required: [size, sha256] + properties: + size: { type: integer, format: int64 } + sha256: { type: string, pattern: '^[0-9a-f]{64}$' } StartFileOp: type: object @@ -795,7 +807,7 @@ components: description: On file_exists from an extraction, the first 200 files it would replace, sorted. conflict_count: { type: integer, description: How many files it would replace in all. } need: { type: integer, format: int64, description: On volume_full, the bytes needed. } - avail: { type: integer, format: int64, description: On volume_full, the bytes free. } + avail: { type: integer, format: int64, description: On volume_full, the bytes free; left out when none are. } ExportStatus: type: object @@ -1362,8 +1374,8 @@ paths: spent token are all the same 404, so the route says nothing about which uploads exist. An upload session committed through POST …/files/uploads/{id}/commit is fetched here the same way, under the - session id; it stays staged until it has been sent whole once, so a Job - that failed before then can be committed again. + session id; it stays staged until its Job reports the file landed + (DELETE), so a Job that failed at any point can be committed again. x-felis-face: [internal] x-felis-tier: public security: [] @@ -1382,6 +1394,30 @@ paths: schema: { type: string, format: binary } '404': $ref: '#/components/responses/NotFound' + delete: + tags: [files] + operationId: internalFileUploadLanded + summary: The Job reports a staged upload landed (the same one-time bearer token). + description: >- + Sent once the file is in place. An upload session is then dropped from + felis-api's disk; a single-request upload goes when its request ends in + any case. Only the token of the session's latest commit is taken. As for + the fetch, every refusal is the same 404. + x-felis-face: [internal] + x-felis-tier: public + security: [] + parameters: + - { name: id, in: path, required: true, schema: { type: string } } + - name: Authorization + in: header + required: true + description: Bearer followed by the token the Job fetched the upload with. + schema: { type: string } + responses: + '204': + $ref: '#/components/responses/NoContent' + '404': + $ref: '#/components/responses/NotFound' /api/v1/internal/exports/{id}: put: @@ -1389,8 +1425,13 @@ paths: operationId: internalExportUpload summary: Hand one export's archive over for download (one-time bearer token). description: >- - The export Job PUTs the tar.gz here, chunked for a world and with its - Content-Length for a backup. The Job holds no service token, so the route + The export Job PUTs the archive or file here, always chunked, with its + size as X-Felis-Export-Length when it knows it and, once the body has + ended, the SHA-256 of all it sent as the Content-Digest trailer + (sha-256=::). felis-api holds the last bytes back from the + browser until the bytes it received number and hash as the Job said, so + a body changed on the way, or one without the trailer, ends the + download short and the browser reports it failed. The Job holds no service token, so the route is public on the internal face and the bearer token minted with the export is the whole check; an unknown id, a wrong or missing token and a token already used are all the same 404. The request then waits, body @@ -1408,6 +1449,11 @@ paths: required: true description: Bearer followed by the token minted with the export. schema: { type: string } + - name: X-Felis-Export-Length + in: header + required: false + description: The body's length in bytes, when the Job knows it; the download then carries it as Content-Length. + schema: { type: integer, format: int64, minimum: 0 } requestBody: required: true content: @@ -1416,6 +1462,11 @@ paths: responses: '204': $ref: '#/components/responses/NoContent' + '400': + description: X-Felis-Export-Length is not a byte count (bad_request); the token is not spent. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } '404': $ref: '#/components/responses/NotFound' '409': @@ -1424,7 +1475,7 @@ paths: application/json: schema: { $ref: '#/components/schemas/Error' } '410': - description: Nobody opened the download within 90 seconds, or the browser left before the archive ended (export_expired). + description: Nobody opened the download within 90 seconds, the browser left before the archive ended, or what arrived did not number or hash as the Job declared, so the download was cut off (export_expired). content: application/json: schema: { $ref: '#/components/schemas/Error' } @@ -4347,8 +4398,10 @@ paths: description: >- The first request spends the ticket, whatever becomes of it. The archive streams as the Job sends it, with Content-Length when it is known; a - download that cannot finish (the Job died, or a backup did not match its - recorded sha256) is cut off, so the browser reports it failed. HEAD is + download that cannot finish (the Job died, a backup did not match its + recorded sha256, or the bytes did not hash to the SHA-256 the Job sent + with them) is cut off before its last bytes, so the browser reports it + failed. HEAD is refused, since it would spend the ticket on no body. x-felis-face: [external] x-felis-tier: app @@ -4496,7 +4549,7 @@ paths: properties: path: { type: string } truncated: { type: boolean, description: The listing hit the entry cap and is incomplete. } - free_bytes: { type: integer, format: int64, description: Bytes free on the world volume, for a client to check an upload fits before sending it. } + free_bytes: { type: integer, format: int64, nullable: true, description: Bytes free on the world volume, for a client to check an upload fits before sending it; 0 is a full volume, and null a volume whose free space the Job could not read. } entries: type: array items: @@ -4971,8 +5024,11 @@ paths: token, so the world lock is taken only after the body has arrived and a slow upload holds off no backup. The file lands atomically: a synced temporary sibling is checked against the staged size and SHA-256, then renamed into - place, so a failed upload leaves the old file whole. Same stopped-gate and - os.Root containment as a write. Audited as file.upload. + place, so a failed upload leaves the old file whole. The body carries its + SHA-256 as Content-Digest; felis-api checks it as the body arrives, and + the Job checks the same digest again as it fetches the staged copy, so + every hop between the browser and the world volume is verified. Same + stopped-gate and os.Root containment as a write. Audited as file.upload. x-felis-face: [external] x-felis-tier: app security: [{ sessionCookie: [] }] @@ -4988,6 +5044,14 @@ paths: required: false description: true replaces an existing file, keeping its mode. Anything else refuses to. schema: { type: string, enum: ["true", "false"] } + - name: Content-Digest + in: header + required: true + description: >- + The SHA-256 of the body as RFC 9530 sends it, sha-256=::. + Other algorithms listed beside it are ignored. Bytes that do not hash + to it were changed on the way and are refused whole. + schema: { type: string, example: 'sha-256=:47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=:' } requestBody: required: true content: @@ -5009,8 +5073,10 @@ paths: '400': description: >- Missing path, invalid server name, a folder or the world root at the path, - a path that escapes the world root, or a body that ended before - Content-Length bytes arrived (upload_incomplete). + a path that escapes the world root, a body that ended before + Content-Length bytes arrived (upload_incomplete), no Content-Digest + (digest_required), a malformed one (bad_digest), or bytes that do not + hash to it (digest_mismatch; nothing is staged, so send it again). content: application/json: schema: { $ref: '#/components/schemas/Error' } @@ -5170,8 +5236,9 @@ paths: summary: Send one part of an upload session (owner-or-admin, the account that began it). description: >- The raw body is appended at offset, which must be where the session - ends. Content-Length is required, and the part is taken whole or not at - all: one cut short leaves the session where it was. Parts go one at a + ends. Content-Length and the part's own Content-Digest are required, and + the part is taken whole or not at all: one cut short, or one whose bytes + do not hash to its digest, leaves the session where it was. Parts go one at a time (409 upload_busy while one arrives). Needs no stopped server, so starting the server midway costs only the commit's refusal until it is stopped again. @@ -5186,6 +5253,14 @@ paths: required: true description: The byte position the part starts at, the session's received. schema: { type: integer, format: int64, minimum: 0 } + - name: Content-Digest + in: header + required: true + description: >- + The SHA-256 of the body as RFC 9530 sends it, sha-256=::. + Other algorithms listed beside it are ignored. Bytes that do not hash + to it were changed on the way and are refused whole. + schema: { type: string, example: 'sha-256=:47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=:' } requestBody: required: true content: @@ -5198,7 +5273,12 @@ paths: application/json: schema: { $ref: '#/components/schemas/FileUploadSession' } '400': - description: A missing or malformed offset (bad_request), a body that ended before its Content-Length (upload_incomplete), or a malformed server name (bad_name). + description: >- + A missing or malformed offset (bad_request), a body that ended before + its Content-Length (upload_incomplete), no Content-Digest + (digest_required), a malformed one (bad_digest), bytes that do not hash + to it (digest_mismatch; the part was not taken, so send it again), or a + malformed server name (bad_name). content: application/json: schema: { $ref: '#/components/schemas/Error' } @@ -7287,13 +7367,22 @@ paths: the API, a larger context goes through the chunked upload at /api/v1/me/submissions/{id}/context/upload instead. An upload that would push the caller past their per-user stored-context budget is refused with 403 - before the excess is persisted. Returns 503 when the deployment's context - store has no implemented upload transport. + before the excess is persisted. The body's SHA-256 is required as + Content-Digest; bytes that do not hash to it were changed on the way, + and none of them replace the context stored before. Returns 503 when + the deployment's context store has no implemented upload transport. x-felis-face: [external] x-felis-tier: app security: [{ sessionCookie: [] }] parameters: - { name: id, in: path, required: true, schema: { type: string } } + - name: Content-Digest + in: header + required: true + description: >- + The SHA-256 of the body as RFC 9530 sends it, sha-256=::. + Other algorithms listed beside it are ignored. + schema: { type: string, example: 'sha-256=:47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=:' } requestBody: required: true content: @@ -7306,7 +7395,14 @@ paths: application/json: schema: { $ref: '#/components/schemas/Submission' } '400': - $ref: '#/components/responses/BadRequest' + description: >- + A body that is not a gzip tarball or is over the context cap + (bad_request), no Content-Digest (digest_required), a malformed one + (bad_digest), or bytes that do not hash to it (digest_mismatch; + nothing was stored, so send it again). + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } '401': $ref: '#/components/responses/Unauthorized' '403': @@ -7367,8 +7463,9 @@ paths: upload_offset_mismatch and the client reads GET for where to resume. The first part must open with the gzip magic (400). The staged total meets the same context cap (400) and storage budget (403) as a single upload. - A part that breaks off is cut back off, so the staged bytes are always a - prefix of the file. One request per upload at a time (409 upload_busy). + A part that breaks off is cut back off, and so is one whose bytes do not + hash to its Content-Digest, so the staged bytes are always a prefix of + the file. One request per upload at a time (409 upload_busy). Staged bytes untouched for 24 hours are deleted. The budget check reads blob sizes remembered for up to a minute; when a size has to be read and the uploads store does not answer, the answer is 503 @@ -7380,6 +7477,13 @@ paths: parameters: - { name: id, in: path, required: true, schema: { type: string } } - { name: offset, in: query, required: true, schema: { type: integer, format: int64, minimum: 0 } } + - name: Content-Digest + in: header + required: true + description: >- + The SHA-256 of the part as RFC 9530 sends it, sha-256=::. + Other algorithms listed beside it are ignored. + schema: { type: string, example: 'sha-256=:47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=:' } requestBody: required: true content: @@ -7392,7 +7496,15 @@ paths: application/json: schema: { $ref: '#/components/schemas/ContextUploadProgress' } '400': - $ref: '#/components/responses/BadRequest' + description: >- + A missing or malformed offset, a first part without the gzip magic + or a total over the context cap (bad_request), no Content-Digest + (digest_required), a malformed one (bad_digest), or bytes that do + not hash to it (digest_mismatch; the part was cut back off, so read + where the upload stands and send it again). + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } '401': $ref: '#/components/responses/Unauthorized' '403': diff --git a/internal/api/api.go b/internal/api/api.go index a44efb9..ccc79f6 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -464,10 +464,12 @@ func (a *API) internalAPIRoutes() []apiRoute { // Principal); the shared enqueueBackup tail enforces the RWO stopped-gate. {Method: "POST", Pattern: "/api/v1/internal/servers/{name}/backup", Callers: ops, h: a.handleInternalBackup}, - // A file upload's staged bytes, fetched once by the Job landing them. Public + // A file upload's staged bytes, fetched once by the Job landing them, which + // then reports them landed so a file sent in parts is deleted. Public // because that Job holds no service token; the one-time bearer token minted // with the upload is the check (handlers_files.go). {Method: "GET", Pattern: "/api/v1/internal/file-uploads/{id}", Public: true, h: a.handleInternalFileUpload}, + {Method: "DELETE", Pattern: "/api/v1/internal/file-uploads/{id}", Public: true, h: a.handleInternalFileUploadLanded}, // An export Job's archive, held open until the owner's browser downloads // it. Public for the same reason as file uploads: the Job holds no service diff --git a/internal/api/exports.go b/internal/api/exports.go index e3f38a6..f8e74db 100644 --- a/internal/api/exports.go +++ b/internal/api/exports.go @@ -44,6 +44,12 @@ import ( // archive passes through felis-api's memory once and never touches a disk // it owns, and the Job moves at the browser's pace. // +// Nothing on the way is trusted to deliver the bytes intact. The Job hashes +// what it sends and ends its chunked PUT with the SHA-256 as a trailer; +// felis-api hashes what it receives and holds the last buffer back from the +// browser until the two agree (relayExport), so a download whose bytes changed +// between the Job and here fails in the browser rather than lands complete. +// // A backup is checked by the Job against the sha256 recorded when it was // written (cmd/felis export): on a mismatch the Job aborts its upload short of // the archive's end, the download aborts with it, and the browser reports a @@ -54,9 +60,10 @@ import ( // Exporter starts the Job that archives a world or a backup and hands it to the // internal upload route (internal/worldexport). Optional: when nil the export -// routes answer 503. +// routes answer 503. Stop deletes a Job felis-api has given up on. type Exporter interface { Start(ctx context.Context, r worldexport.Request) (job string, err error) + Stop(ctx context.Context, job string) error } // Limits on exports. Each one keeps a Job, a connection and a 64 KiB copy @@ -176,8 +183,10 @@ type exportEntry struct { // exportUpload is the Job's PUT, parked until a browser claims it. type exportUpload struct { - body io.Reader - size int64 // -1 when the Job streams it chunked + body io.Reader + size int64 // what the Job declared (worldexport.LengthHeader); -1 when it did not + // digest reads the Job's trailer, which is there once body has ended. + digest func() []string claimed chan struct{} done chan error // how the download ended; buffered } @@ -191,6 +200,9 @@ type exportRegistry struct { byTicket map[string]*exportEntry byID map[string]*exportEntry starts map[exportStarts][]time.Time // oldest first + // stop deletes the Job of an export the sweep expired while it was + // pending, and is run apart from the lock. + stop func(job string) } func (a *API) exportTickets() *exportRegistry { @@ -199,17 +211,41 @@ func (a *API) exportTickets() *exportRegistry { byTicket: map[string]*exportEntry{}, byID: map[string]*exportEntry{}, starts: map[exportStarts][]time.Time{}, + stop: a.stopExportJob, } }) return a.exports } +// ExpireExports runs the export sweep on its own, for felis-api's loop: the +// routes sweep as they are called, and an owner who closed the tab calls none. +func (a *API) ExpireExports() { + g := a.exportTickets() + g.mu.Lock() + defer g.mu.Unlock() + g.sweepLocked(a.now()) +} + +// stopExportJob deletes one export Job. A delete that fails leaves the Job to +// its own deadline. +func (a *API) stopExportJob(job string) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + if err := a.Exporter.Stop(ctx, job); err != nil { + log.Printf("api: stop export job %s, which never connected: %v", job, err) + } +} + // sweepLocked expires what has waited too long and forgets what ended long ago. +// A Job still pending at its TTL never connected: its Pod is stuck unscheduled +// or pulling, and until its deadline it would keep the server from starting, +// so it is deleted. func (g *exportRegistry) sweepLocked(now time.Time) { for t, e := range g.byTicket { switch { case e.state == exportPending && now.Sub(e.at) >= exportPendingTTL: e.state, e.at = exportSpent, now + go g.stop(e.job) case !e.active() && now.Sub(e.at) >= exportKeepSpent: delete(g.byTicket, t) delete(g.byID, e.id) @@ -661,7 +697,7 @@ func (a *API) handleExportDownload(w http.ResponseWriter, r *http.Request) { } w.WriteHeader(http.StatusOK) - err = copyExport(w, e.upload.body) + n, sum, err := relayExport(w, e.upload) e.upload.done <- err if err != nil { log.Printf("api: export %s of %s ended early: %v", e.id, e.server, err) @@ -669,17 +705,64 @@ func (a *API) handleExportDownload(w http.ResponseWriter, r *http.Request) { // read as failed in the browser, never as a complete file. panic(http.ErrAbortHandler) } + log.Printf("api: export %s of %s sent %d bytes, sha256 %s", e.id, e.server, n, sum) } -// copyExport copies body into w through a fixed 32 KiB buffer, restarting w's -// write deadline on every write. -func copyExport(w http.ResponseWriter, body io.Reader) error { +var ( + errExportDigest = errors.New("the bytes the export Job sent do not match the SHA-256 it sent with them") + errExportLength = errors.New("the export Job sent a different number of bytes than it declared") +) + +// relayExport copies the Job's upload into the download through fixed 32 KiB +// buffers, restarting w's write deadline on every write, and checks it on the +// way: the bytes must hash to the SHA-256 the Job's trailer carries once it has +// sent them all (worldexport.DigestTrailer), and number what it declared, when +// it did. The latest buffer read is held back until both hold, so bytes changed +// between the Job and here, or a Job that sent no digest, end the download +// short of its end, and the browser reports it failed rather than keep a +// complete-looking corrupt file. It returns the bytes sent and their SHA-256. +func relayExport(w http.ResponseWriter, up *exportUpload) (int64, string, error) { out := &stallWriter{w: w, rc: http.NewResponseController(w)} - if _, err := io.CopyBuffer(out, body, make([]byte, exportCopyBuffer)); err != nil { - return err + h := sha256.New() + var n int64 + next, spare := make([]byte, exportCopyBuffer), make([]byte, exportCopyBuffer) + var held []byte + for { + k, err := up.body.Read(next) + if k > 0 { + if len(held) > 0 { + if _, werr := out.Write(held); werr != nil { + return n, "", werr + } + } + h.Write(next[:k]) + n += int64(k) + held = next[:k] + next, spare = spare, next + } + if err == io.EOF { + break + } + if err != nil { + return n, "", err + } + } + want, err := parseContentDigest(up.digest()) + switch { + case up.size >= 0 && n != up.size: + return n, "", fmt.Errorf("%w: %d of %d", errExportLength, n, up.size) + case err != nil: + return n, "", fmt.Errorf("%w: %v", errExportDigest, err) + case subtle.ConstantTimeCompare(h.Sum(nil), want) != 1: + return n, "", errExportDigest + } + if len(held) > 0 { + if _, err := out.Write(held); err != nil { + return n, "", err + } } _ = out.rc.SetWriteDeadline(time.Time{}) // the connection may serve another request - return nil + return n, hex.EncodeToString(h.Sum(nil)), nil } // stallWriter restarts the connection's write deadline before every write, so @@ -708,9 +791,19 @@ func (a *API) handleInternalExportUpload(w http.ResponseWriter, r *http.Request) writeError(w, r, errNoExport()) return } + size := int64(-1) + if v := r.Header.Get(worldexport.LengthHeader); v != "" { + n, err := strconv.ParseInt(v, 10, 64) + if err != nil || n < 0 { + writeError(w, r, newError(http.StatusBadRequest, "bad_request", "%s must be a byte count", worldexport.LengthHeader)) + return + } + size = n + } rc := takeBodyDeadline(w, r) up := &exportUpload{ - body: &stallBody{r: r.Body, rc: rc}, size: r.ContentLength, + body: &stallBody{r: r.Body, rc: rc}, size: size, + digest: func() []string { return r.Trailer.Values(worldexport.DigestTrailer) }, claimed: make(chan struct{}), done: make(chan error, 1), } reg := a.exportTickets() diff --git a/internal/api/exports_test.go b/internal/api/exports_test.go index b0d0a35..47c3809 100644 --- a/internal/api/exports_test.go +++ b/internal/api/exports_test.go @@ -31,6 +31,14 @@ import ( type fakeExporter struct { reqs []worldexport.Request err error + // stopped receives each Job Stop is asked to delete; the sweep asks from + // a goroutine of its own. + stopped chan string +} + +func (f *fakeExporter) Stop(_ context.Context, job string) error { + f.stopped <- job + return nil } func (f *fakeExporter) Start(_ context.Context, r worldexport.Request) (string, error) { @@ -92,7 +100,7 @@ func exportFixture() (*API, *fakeRepo, *fakeCluster, *fakeExporter) { for _, name := range []string{"survival", "gamma"} { cl.byName[name] = &ServerInfo{Name: name, Phase: "Stopped", DesiredState: string(v1alpha1.DesiredStopped)} } - ex := &fakeExporter{} + ex := &fakeExporter{stopped: make(chan string, 64)} a := newTestAPI(repo, cl) a.External = exportUsers a.Exporter, a.InternalBaseURL = ex, exportBase @@ -140,11 +148,15 @@ func waitExportReady(t *testing.T, h http.Handler, ticket, user string) { } // uploadExport serves the Job's PUT on the internal face in the background. -func uploadExport(h http.Handler, id, token string, body io.Reader) <-chan *httptest.ResponseRecorder { +// digest, when set, is the trailer the Job sends once its body has ended. +func uploadExport(h http.Handler, id, token string, body io.Reader, digest string) <-chan *httptest.ResponseRecorder { out := make(chan *httptest.ResponseRecorder, 1) go func() { r := httptest.NewRequest("PUT", "/api/v1/internal/exports/"+id, body) r.Header.Set("Authorization", "Bearer "+token) + if digest != "" { + r.Trailer = http.Header{worldexport.DigestTrailer: {digest}} + } w := httptest.NewRecorder() h.ServeHTTP(w, r) out <- w @@ -167,6 +179,18 @@ func awaitUpload(t *testing.T, up <-chan *httptest.ResponseRecorder) *httptest.R // claimed the export by mistake would block on the parked body, and an upload // let in by mistake would wait for a browser, so either fails the test after a // bound instead of hanging it. +// awaitStop returns the next Job the sweep asked to delete. +func awaitStop(t *testing.T, ex *fakeExporter) string { + t.Helper() + select { + case job := <-ex.stopped: + return job + case <-time.After(5 * time.Second): + t.Fatal("no Job was stopped") + return "" + } +} + func doSoon(t *testing.T, h http.Handler, method, path, body string, headers map[string]string) *httptest.ResponseRecorder { t.Helper() out := make(chan *httptest.ResponseRecorder, 1) @@ -341,7 +365,7 @@ func TestExportWorldGate(t *testing.T) { w := do(a.ExternalHandler(), "POST", worldPath, "", as("admin1")) var raw map[string]map[string]string _ = json.Unmarshal(w.Body.Bytes(), &raw) - if want := "a world export is running on this server's world; retry once it finishes"; w.Code != http.StatusConflict || + if want := "a world export or file download is running on this server's world; retry once it finishes"; w.Code != http.StatusConflict || raw["error"]["code"] != "maintenance_in_progress" || raw["error"]["message"] != want { t.Fatalf("busy = %d %s, want 409 %q", w.Code, w.Body.String(), want) } @@ -459,9 +483,17 @@ func TestExportRendezvous(t *testing.T) { t.Fatalf("PUT to another id = %d", w.Code) } + // A length that is no byte count is refused before the token is spent. + for _, bad := range []string{"-1", "12abc", "0x10"} { + if w := doSoon(t, in, "PUT", "/api/v1/internal/exports/"+job.ID, "x", map[string]string{ + "Authorization": "Bearer " + job.Token, worldexport.LengthHeader: bad}); w.Code != http.StatusBadRequest || decodeErr(t, w) != "bad_request" { + t.Fatalf("PUT declaring %q bytes = %d %s", bad, w.Code, w.Body.String()) + } + } + archive := randomBytes(3*exportCopyBuffer + 4321) pr, pw := io.Pipe() - up := uploadExport(in, job.ID, job.Token, pr) + up := uploadExport(in, job.ID, job.Token, pr, contentDigestOf(string(archive))) waitExportReady(t, ext, v.Ticket, "owner1") if w := doSoon(t, in, "PUT", "/api/v1/internal/exports/"+job.ID, "x", map[string]string{"Authorization": "Bearer " + job.Token}); w.Code != http.StatusNotFound { t.Fatalf("a second PUT with the spent token = %d, want 404", w.Code) @@ -541,11 +573,47 @@ func TestExportExpiry(t *testing.T) { t.Fatalf("past the pending TTL: %d %+v", code, s) } job := ex.reqs[0] + if got := awaitStop(t, ex); got != worldexport.JobName(job.Server, job.ID) { + t.Fatalf("stopped %q, want the export's own Job", got) + } if w := doSoon(t, a.InternalHandler(), "PUT", "/api/v1/internal/exports/"+job.ID, "x", map[string]string{"Authorization": "Bearer " + job.Token}); w.Code != http.StatusNotFound { t.Fatalf("a late Job's PUT = %d, want 404", w.Code) } }) + // The owner closed the tab, so no route sweeps; felis-api's loop does. + t.Run("the loop stops a Job left pending, and only that one", func(t *testing.T) { + a, _, _, ex := exportFixture() + var clock atomic.Int64 + clock.Store(1_700_000_000) + a.Now = func() time.Time { return time.Unix(clock.Load(), 0) } + ext := a.ExternalHandler() + beginExport(t, ext, worldPath, "owner1") + stuck := ex.reqs[0] + v := beginExport(t, ext, "/api/v1/servers/creative/backups/bk2/export", "admin1") + moving := ex.reqs[1] + uploadExport(a.InternalHandler(), moving.ID, moving.Token, strings.NewReader("archive"), contentDigestOf("archive")) + waitExportReady(t, ext, v.Ticket, "admin1") + + clock.Add(int64(exportPendingTTL/time.Second) - 1) + a.ExpireExports() + select { + case job := <-ex.stopped: + t.Fatalf("stopped %s inside the pending TTL", job) + case <-time.After(50 * time.Millisecond): + } + clock.Add(1) + a.ExpireExports() + if got := awaitStop(t, ex); got != worldexport.JobName(stuck.Server, stuck.ID) { + t.Fatalf("stopped %q, want the Job that never connected", got) + } + select { + case job := <-ex.stopped: + t.Fatalf("also stopped %s, whose upload was waiting for its browser", job) + case <-time.After(50 * time.Millisecond): + } + }) + t.Run("a browser that never comes", func(t *testing.T) { defer func(old time.Duration) { exportClaimTTL = old }(exportClaimTTL) exportClaimTTL = 30 * time.Millisecond @@ -553,7 +621,7 @@ func TestExportExpiry(t *testing.T) { ext := a.ExternalHandler() v := beginExport(t, ext, backupPath, "owner1") job := ex.reqs[0] - w := awaitUpload(t, uploadExport(a.InternalHandler(), job.ID, job.Token, strings.NewReader("archive"))) + w := awaitUpload(t, uploadExport(a.InternalHandler(), job.ID, job.Token, strings.NewReader("archive"), contentDigestOf("archive"))) if w.Code != http.StatusGone || decodeErr(t, w) != "export_expired" { t.Fatalf("unclaimed upload = %d %s, want 410 export_expired", w.Code, w.Body.String()) } @@ -639,12 +707,20 @@ func exportServers(t *testing.T, a *API) (ext, in *httptest.Server) { return ext, in } -func realUpload(t *testing.T, in *httptest.Server, job worldexport.Request, body io.Reader, size int64) <-chan *http.Response { +// realUpload sends the Job's PUT as cmd/felis export does: chunked, with the +// size when it is known (not -1) and, when set, digest as the trailer. +func realUpload(t *testing.T, in *httptest.Server, job worldexport.Request, body io.Reader, size int64, digest string) <-chan *http.Response { req, err := http.NewRequest("PUT", in.URL+"/api/v1/internal/exports/"+job.ID, body) if err != nil { t.Fatal(err) } - req.ContentLength = size + req.ContentLength = -1 + if size >= 0 { + req.Header.Set(worldexport.LengthHeader, strconv.FormatInt(size, 10)) + } + if digest != "" { + req.Trailer = http.Header{worldexport.DigestTrailer: {digest}} + } req.Header.Set("Authorization", "Bearer "+job.Token) out := make(chan *http.Response, 1) go func() { @@ -696,7 +772,7 @@ func TestExportStreamsWhatTheJobSends(t *testing.T) { a, _, _, ex := exportFixture() ext, in := exportServers(t, a) v := beginExport(t, a.ExternalHandler(), backupPath, "owner1") - up := realUpload(t, in, ex.reqs[0], bytes.NewReader(archive), int64(len(archive))) + up := realUpload(t, in, ex.reqs[0], bytes.NewReader(archive), int64(len(archive)), contentDigestOf(string(archive))) waitExportReady(t, a.ExternalHandler(), v.Ticket, "owner1") resp, got, err := realDownload(ext, v.Ticket) if resp == nil { @@ -723,7 +799,7 @@ func TestExportStreamsWhatTheJobSends(t *testing.T) { _, _ = pw.Write(archive[:len(archive)-100]) pw.CloseWithError(errors.New("the backup archive does not match the sha256 recorded when it was written")) }() - up := realUpload(t, in, ex.reqs[0], pr, -1) + up := realUpload(t, in, ex.reqs[0], pr, -1, contentDigestOf(string(archive))) waitExportReady(t, a.ExternalHandler(), v.Ticket, "owner1") resp, got, err := realDownload(ext, v.Ticket) if resp == nil { @@ -737,11 +813,46 @@ func TestExportStreamsWhatTheJobSends(t *testing.T) { } }) + // The Job's trailer is the SHA-256 of what it sent. Bytes that arrive + // otherwise, or without it, or not as many as it declared, never reach the + // browser to their end, and the Job hears the download failed. + t.Run("bytes changed on the way never reach the browser whole", func(t *testing.T) { + flipped := bytes.Clone(archive) + flipped[len(flipped)/2] ^= 1 + for _, c := range []struct { + name, digest string + size int64 + }{ + {"another archive's digest", contentDigestOf(string(flipped)), int64(len(archive))}, + {"no digest", "", -1}, + {"a digest that is no SHA-256", "sha-256=:AAAA:", -1}, + {"one byte more declared than sent", contentDigestOf(string(archive)), int64(len(archive)) + 1}, + } { + t.Run(c.name, func(t *testing.T) { + a, _, _, ex := exportFixture() + ext, in := exportServers(t, a) + v := beginExport(t, a.ExternalHandler(), backupPath, "owner1") + up := realUpload(t, in, ex.reqs[0], bytes.NewReader(archive), c.size, c.digest) + waitExportReady(t, a.ExternalHandler(), v.Ticket, "owner1") + resp, got, err := realDownload(ext, v.Ticket) + if resp == nil { + t.Fatalf("download: %v", err) + } + if err == nil || len(got) >= len(archive) || !bytes.Equal(got, archive[:len(got)]) { + t.Fatalf("read %d of %d bytes, err %v; want an error short of the end", len(got), len(archive), err) + } + if upResp := awaitResponse(t, up); upResp.StatusCode != http.StatusGone { + t.Fatalf("the upload answered %d, want 410", upResp.StatusCode) + } + }) + } + }) + t.Run("a browser that leaves early is what the Job hears", func(t *testing.T) { a, _, _, ex := exportFixture() ext, _ := exportServers(t, a) v := beginExport(t, a.ExternalHandler(), worldPath, "owner1") - up := uploadExport(a.InternalHandler(), ex.reqs[0].ID, ex.reqs[0].Token, io.LimitReader(zeros{}, 1<<30)) + up := uploadExport(a.InternalHandler(), ex.reqs[0].ID, ex.reqs[0].Token, io.LimitReader(zeros{}, 1<<30), "") waitExportReady(t, a.ExternalHandler(), v.Ticket, "owner1") req, _ := http.NewRequest("GET", ext.URL+"/api/v1/exports/"+v.Ticket+"/download", nil) req.Header.Set("X-Test-User", "owner1") @@ -957,7 +1068,7 @@ func TestFileDownloadServed(t *testing.T) { // Past what the server would buffer and measure itself when the // handler sets no length. body := randomBytes(64 << 10) - up := realUpload(t, in, ex.reqs[0], bytes.NewReader(body), tc.size) + up := realUpload(t, in, ex.reqs[0], bytes.NewReader(body), tc.size, contentDigestOf(string(body))) waitExportReady(t, a.ExternalHandler(), v.Ticket, "owner1") resp, got, err := realDownload(ext, v.Ticket) if resp == nil || err != nil || !bytes.Equal(got, body) { @@ -1015,7 +1126,7 @@ func TestExportUploadPace(t *testing.T) { _, _ = pw.Write(archive[1000:]) pw.Close() }() - up := realUpload(t, in, ex.reqs[0], pr, -1) + up := realUpload(t, in, ex.reqs[0], pr, -1, contentDigestOf(string(archive))) waitExportReady(t, a.ExternalHandler(), v.Ticket, "owner1") time.Sleep(3 * bodyGrace) _, got, err := realDownload(ext, v.Ticket) @@ -1034,7 +1145,7 @@ func TestExportUploadPace(t *testing.T) { pr, pw := io.Pipe() defer pw.Close() go func() { _, _ = pw.Write(make([]byte, 1000)) }() // then nothing, ever - up := realUpload(t, in, ex.reqs[0], pr, -1) + up := realUpload(t, in, ex.reqs[0], pr, -1, "") waitExportReady(t, a.ExternalHandler(), v.Ticket, "owner1") done := make(chan error, 1) go func() { _, _, err := realDownload(ext, v.Ticket); done <- err }() @@ -1054,7 +1165,7 @@ func TestExportUploadPace(t *testing.T) { a, _, _, ex := exportFixture() ext, in := exportServers(t, a) v := beginExport(t, a.ExternalHandler(), worldPath, "owner1") - realUpload(t, in, ex.reqs[0], io.LimitReader(zeros{}, 1<<30), -1) + realUpload(t, in, ex.reqs[0], io.LimitReader(zeros{}, 1<<30), -1, "") waitExportReady(t, a.ExternalHandler(), v.Ticket, "owner1") req, _ := http.NewRequest("GET", ext.URL+"/api/v1/exports/"+v.Ticket+"/download", nil) req.Header.Set("X-Test-User", "owner1") diff --git a/internal/api/handlers_fileops.go b/internal/api/handlers_fileops.go index 38a0e46..7ccced5 100644 --- a/internal/api/handlers_fileops.go +++ b/internal/api/handlers_fileops.go @@ -29,17 +29,50 @@ import ( // the world volume while it runs, as any file write does, and the server cannot // start until it ends. -// fileSessionView is where an upload session stands. +// fileSessionView is where an upload session stands. Parts are the parts it +// took, in order, each with the SHA-256 it arrived with: a client resuming from +// a file on disk checks the file still holds those bytes before it sends the +// rest. type fileSessionView struct { - ID string `json:"id"` - Path string `json:"path"` - Size int64 `json:"size"` - Received int64 `json:"received"` - PartMaxBytes int64 `json:"part_max_bytes"` + ID string `json:"id"` + Path string `json:"path"` + Size int64 `json:"size"` + Received int64 `json:"received"` + PartMaxBytes int64 `json:"part_max_bytes"` + Parts []filePartView `json:"parts"` +} + +// filePartView is one part a session took. +type filePartView struct { + Size int64 `json:"size"` + SHA256 string `json:"sha256"` } func sessionView(s fileedit.Session) fileSessionView { - return fileSessionView{ID: s.ID, Path: s.Path, Size: s.Size, Received: s.Received, PartMaxBytes: fileedit.PartBytes} + parts := make([]filePartView, 0, len(s.Parts)) + for _, p := range s.Parts { + parts = append(parts, filePartView{Size: p.Size, SHA256: p.SHA256}) + } + return fileSessionView{ + ID: s.ID, Path: s.Path, Size: s.Size, Received: s.Received, + PartMaxBytes: fileedit.PartBytes, Parts: parts, + } +} + +// namesFit reports whether every name in path fits one folder entry. +func namesFit(path string) bool { + for _, name := range strings.Split(path, "/") { + if len(name) > fileedit.NameMax { + return false + } + } + return true +} + +// errNameTooLong refuses a path namesFit rejects, before any byte is taken: +// the Job would only find out once it tried to create the file. +func errNameTooLong() *apiError { + return newError(http.StatusBadRequest, "bad_path", "a name in the path is longer than %d bytes", fileedit.NameMax) } // beginFileUploadRequest is the POST …/files/uploads body. @@ -73,6 +106,10 @@ func (a *API) handleBeginFileUpload(w http.ResponseWriter, r *http.Request) { "the path must name a file inside the world folder")) return } + if !namesFit(path) { + writeError(w, r, errNameTooLong()) + return + } var body beginFileUploadRequest if err := decodeJSON(w, r, &body); err != nil { writeError(w, r, err) @@ -111,9 +148,11 @@ func (a *API) handleFileUploadStatus(w http.ResponseWriter, r *http.Request) { // /api/v1/servers/{name}/files/uploads/{id}?offset=… — append the raw body to // the caller's session. offset must be where the session ends (409 // upload_offset_mismatch otherwise; the status says where), and Content-Length -// is required, as for the one-request upload: the part is taken whole or not at -// all, and a part that breaks midway leaves the session where it was. A part -// over fileedit.PartBytes is refused before a byte of it is read (Append). +// and Content-Digest are required, as for the one-request upload: the part is +// taken whole or not at all, and a part that breaks midway, or whose bytes do +// not hash to its digest (400 digest_mismatch), leaves the session where it +// was. A part over fileedit.PartBytes is refused before a byte of it is read +// (Append). func (a *API) handleFileUploadPart(w http.ResponseWriter, r *http.Request) { name, user, ok := a.authorizeFileSession(w, r) if !ok { @@ -130,7 +169,11 @@ func (a *API) handleFileUploadPart(w http.ResponseWriter, r *http.Request) { "a part needs a Content-Length")) return } - s, err := a.FileStage.Append(user, name, r.PathValue("id"), offset, r.Body, r.ContentLength) + want, ok := contentDigest(w, r) + if !ok { + return + } + s, err := a.FileStage.Append(user, name, r.PathValue("id"), offset, r.Body, r.ContentLength, want) if err != nil { writeFileSessionError(w, r, err) return @@ -166,8 +209,9 @@ type startFileOpRequest struct { // The world lock is taken BEFORE the session is sealed: a commit made while an // earlier commit's Job is still fetching the file is refused by that Job's hold // on the volume, so it never mints the fresh token that would lock the running -// Job out. The session outlives a Job that fails before fetching every byte, so -// such a commit is simply made again; one that fetched them all is gone. +// Job out. The session outlives a Job that fails for any reason, so such a +// commit is simply made again; the Job whose file landed deletes it +// (handleInternalFileUploadLanded). func (a *API) handleCommitFileUpload(w http.ResponseWriter, r *http.Request) { name, ok := a.authorizeFileOp(w, r) if !ok { @@ -335,15 +379,18 @@ func opView(op fileedit.OpState) fileOpView { } // opError maps a failed op onto the API's codes. A Job that printed no result -// carries only its condition's reason (DeadlineExceeded, BackoffLimitExceeded): +// carries only its reason (DeadlineExceeded, BackoffLimitExceeded, OOMKilled): // the log it left is the world's content and the runtime's, and none of it is // the caller's to read. func opError(op fileedit.OpState) *fileOpError { res := op.Result if res == nil { msg := "the file operation stopped before it could report how it went (%s); run it again" - if op.Reason == "DeadlineExceeded" { + switch op.Reason { + case "DeadlineExceeded": msg = "the file operation ran out of time (%s); run it again" + case fileedit.ReasonOOMKilled: + msg = "the file operation ran out of memory (%s); an archive of this many files has to be split into smaller ones" } return &fileOpError{Code: "job_failed", Message: fmt.Sprintf(msg, op.Reason)} } @@ -434,6 +481,8 @@ func writeFileSessionError(w http.ResponseWriter, r *http.Request, err error) { case errors.Is(err, fileedit.ErrPartTooLarge): writeError(w, r, newError(http.StatusRequestEntityTooLarge, "part_too_large", "the part is larger than part_max_bytes, or runs past the size the upload began with")) + case errors.Is(err, fileedit.ErrDigestMismatch): + writeError(w, r, errDigestMismatch()) case errors.Is(err, fileedit.ErrShortUpload): writeError(w, r, newError(http.StatusBadRequest, "upload_incomplete", "the part ended before its Content-Length; read where the upload stands and send it again")) diff --git a/internal/api/handlers_fileops_test.go b/internal/api/handlers_fileops_test.go index 5027c6b..d1f959b 100644 --- a/internal/api/handlers_fileops_test.go +++ b/internal/api/handlers_fileops_test.go @@ -56,10 +56,19 @@ func sessionAnswer(t *testing.T, w *httptest.ResponseRecorder) fileSessionView { } // doPart sends one part with the Content-Length given, whatever the body's own -// length: -1 sends none, and one past the body is a part cut short. +// length: -1 sends none, and one past the body is a part cut short. Its +// Content-Digest is that of the body. func doPart(h http.Handler, target, body string, length int64) *httptest.ResponseRecorder { + return doPartDigest(h, target, body, length, contentDigestOf(body)) +} + +// doPartDigest is doPart with the Content-Digest given; empty sends none. +func doPartDigest(h http.Handler, target, body string, length int64, digest string) *httptest.ResponseRecorder { r := httptest.NewRequest("PUT", target, strings.NewReader(body)) r.Header.Set("Content-Type", "application/octet-stream") + if digest != "" { + r.Header.Set("Content-Digest", digest) + } r.ContentLength = length w := httptest.NewRecorder() h.ServeHTTP(w, r) @@ -108,7 +117,10 @@ func TestFileUploadSession(t *testing.T) { h := api.ExternalHandler() s := beginSession(t, api, 10) - if len(s.ID) != 32 || s.Path != sessionPath || s.Size != 10 || s.Received != 0 || s.PartMaxBytes != fileedit.PartBytes { + // A new session lists its parts as [], which decodes non-nil; null would + // leave a client resuming with nothing to walk. + if len(s.ID) != 32 || s.Path != sessionPath || s.Size != 10 || s.Received != 0 || s.PartMaxBytes != fileedit.PartBytes || + s.Parts == nil || len(s.Parts) != 0 { t.Fatalf("begin = %+v", s) } @@ -129,8 +141,10 @@ func TestFileUploadSession(t *testing.T) { t.Fatalf("early commit: code = %d calls = %d acquired %v (%s)", w.Code, files.calls, cl.acquired, w.Body.String()) } - if w := status(api, s.ID); w.Code != http.StatusOK || sessionAnswer(t, w) != (fileSessionView{ - ID: s.ID, Path: sessionPath, Size: 10, Received: 5, PartMaxBytes: fileedit.PartBytes}) { + hello := sha256.Sum256([]byte("hello")) + if w := status(api, s.ID); w.Code != http.StatusOK || !reflect.DeepEqual(sessionAnswer(t, w), fileSessionView{ + ID: s.ID, Path: sessionPath, Size: 10, Received: 5, PartMaxBytes: fileedit.PartBytes, + Parts: []filePartView{{Size: 5, SHA256: hex.EncodeToString(hello[:])}}}) { t.Fatalf("status: code = %d (%s)", w.Code, w.Body.String()) } if w := doPart(h, partAt(s.ID, 5), "world", 5); w.Code != http.StatusOK || sessionAnswer(t, w).Received != 10 { @@ -172,11 +186,31 @@ func TestFileUploadSession(t *testing.T) { if again := fetchStaged(api, src); again.Code != http.StatusNotFound { t.Fatalf("second fetch: code = %d", again.Code) } - // Served whole, so the session is gone and its file with it. - if w := status(api, s.ID); w.Code != http.StatusNotFound || decodeErr(t, w) != "upload_not_found" { + // Served whole, the session stays until the Job says the file landed: a + // Job that fails after its fetch leaves the upload to be committed again. + if w := status(api, s.ID); w.Code != http.StatusOK || sessionAnswer(t, w).Received != 10 { t.Fatalf("status after the fetch: code = %d (%s)", w.Code, w.Body.String()) } + landed := func(token string) *httptest.ResponseRecorder { + return do(api.InternalHandler(), "DELETE", strings.TrimPrefix(src.URL, api.InternalBaseURL), "", + map[string]string{"Authorization": "Bearer " + token}) + } + if w := landed(strings.Repeat("0", len(src.Token))); w.Code != http.StatusNotFound || decodeErr(t, w) != "not_found" { + t.Fatalf("landed with the wrong token: code = %d (%s)", w.Code, w.Body.String()) + } + if w := status(api, s.ID); w.Code != http.StatusOK { + t.Fatalf("a wrong token let go of the session: code = %d (%s)", w.Code, w.Body.String()) + } + if w := landed(src.Token); w.Code != http.StatusNoContent { + t.Fatalf("landed: code = %d (%s)", w.Code, w.Body.String()) + } + if w := status(api, s.ID); w.Code != http.StatusNotFound || decodeErr(t, w) != "upload_not_found" { + t.Fatalf("status after it landed: code = %d (%s)", w.Code, w.Body.String()) + } stageEmpty(t, api) + if w := landed(src.Token); w.Code != http.StatusNotFound { + t.Fatalf("landed twice: code = %d (%s)", w.Code, w.Body.String()) + } }) t.Run("a Job that never fetched is committed again with a fresh token", func(t *testing.T) { @@ -260,6 +294,32 @@ func TestFileUploadSession(t *testing.T) { } }) + t.Run("a part changed on the way is refused and taken back", func(t *testing.T) { + api, _, _, _ := mkFiles(t) + api.External = staticExternal{p: fileOwner} + h := api.ExternalHandler() + s := beginSession(t, api, 10) + doPart(h, partAt(s.ID, 0), "hello", 5) + for _, c := range []struct { + name, digest, errCode string + }{ + {"another part's digest", contentDigestOf("wor1d"), "digest_mismatch"}, + {"no digest", "", "digest_required"}, + {"a malformed digest", "sha-256=:bm90IGEgc3VtCg==:", "bad_digest"}, + } { + w := doPartDigest(h, partAt(s.ID, 5), "world", 5, c.digest) + if w.Code != http.StatusBadRequest || decodeErr(t, w) != c.errCode { + t.Fatalf("%s: code = %d (%s), want 400 %s", c.name, w.Code, w.Body.String(), c.errCode) + } + if got := sessionAnswer(t, status(api, s.ID)); got.Received != 5 || len(got.Parts) != 1 { + t.Fatalf("%s: session after the refused part = %+v", c.name, got) + } + } + if w := doPart(h, partAt(s.ID, 5), "world", 5); w.Code != http.StatusOK || sessionAnswer(t, w).Received != 10 { + t.Fatalf("the part sent again: code = %d (%s)", w.Code, w.Body.String()) + } + }) + t.Run("a part cut short leaves the session where it was", func(t *testing.T) { api, _, _, _ := mkFiles(t) api.External = staticExternal{p: fileOwner} @@ -283,6 +343,7 @@ func TestFileUploadSession(t *testing.T) { go func() { r := httptest.NewRequest("PUT", partAt(s.ID, 0), pr) r.Header.Set("Content-Type", "application/octet-stream") + r.Header.Set("Content-Digest", contentDigestOf("abc")) r.ContentLength = 3 w := httptest.NewRecorder() h.ServeHTTP(w, r) @@ -432,6 +493,8 @@ func TestFileUploadSession(t *testing.T) { {"a path leaving the world", sessionsRoute + "?path=../a.jar", `{"size":3}`, nil, http.StatusBadRequest, "bad_path"}, {"an absolute path", sessionsRoute + "?path=/etc/a.jar", `{"size":3}`, nil, http.StatusBadRequest, "bad_path"}, {"the world folder itself", sessionsRoute + "?path=plugins/..", `{"size":3}`, nil, http.StatusBadRequest, "bad_path"}, + {"a name longer than a folder entry holds", sessionsRoute + "?path=plugins/" + strings.Repeat("n", fileedit.NameMax-3) + ".jar", `{"size":3}`, + nil, http.StatusBadRequest, "bad_path"}, {"a staging disk at its floor", sessionsRoute + "?path=a.jar", `{"size":3}`, func(a *API) { a.FileStage.MinFree = 1 }, http.StatusInsufficientStorage, "upload_staging_full"}, {"no stage", sessionsRoute + "?path=a.jar", `{"size":3}`, @@ -459,6 +522,19 @@ func TestFileUploadSession(t *testing.T) { } }) + t.Run("a name as long as a folder entry holds is taken", func(t *testing.T) { + api, _, _, _ := mkFiles(t) + api.External = staticExternal{p: fileOwner} + long := strings.Repeat("n", fileedit.NameMax-4) + ".jar" + w := do(api.ExternalHandler(), "POST", sessionsRoute+"?path=plugins/"+long, `{"size":3}`, jsonHeader) + if w.Code != http.StatusCreated { + t.Fatalf("code = %d (%s), want 201", w.Code, w.Body.String()) + } + if s := sessionAnswer(t, w); s.Path != "plugins/"+long { + t.Fatalf("path = %q", s.Path) + } + }) + t.Run("a fifth upload at once -> 429", func(t *testing.T) { api, _, _, _ := mkFiles(t) api.External = staticExternal{p: fileOwner} @@ -686,7 +762,9 @@ func TestFileOps(t *testing.T) { deadline.Reason = "DeadlineExceeded" killed := base("i", fileedit.OpUpload, fileedit.OpFailed) killed.Reason = "BackoffLimitExceeded" - files.ops = []fileedit.OpState{running, unzipped, uploaded, exists, full, changed, unsafe, deadline, killed} + oom := base("j", fileedit.OpUnzip, fileedit.OpFailed) + oom.Reason = "OOMKilled" + files.ops = []fileedit.OpState{running, unzipped, uploaded, exists, full, changed, unsafe, deadline, killed, oom} w := list(api) if w.Code != http.StatusOK || files.gotServer != "survival" { @@ -725,6 +803,8 @@ func TestFileOps(t *testing.T) { "the file operation ran out of time (DeadlineExceeded); run it again", nil)), op("i", "upload", "failed", failure("job_failed", "the file operation stopped before it could report how it went (BackoffLimitExceeded); run it again", nil)), + op("j", "unzip", "failed", failure("job_failed", + "the file operation ran out of memory (OOMKilled); an archive of this many files has to be split into smaller ones", nil)), }} if got := fileAnswer(t, w); !reflect.DeepEqual(got, want) { gotJSON, _ := json.MarshalIndent(got, "", " ") diff --git a/internal/api/handlers_files.go b/internal/api/handlers_files.go index e8e41a9..d5c1611 100644 --- a/internal/api/handlers_files.go +++ b/internal/api/handlers_files.go @@ -2,6 +2,8 @@ package api import ( "context" + "crypto/sha256" + "encoding/base64" "errors" "io" "net/http" @@ -109,9 +111,14 @@ func (a *API) handleListFiles(w http.ResponseWriter, r *http.Request) { ls.Entries = []fileedit.Entry{} // an empty directory is [], never null } // free_bytes lets the panel refuse an upload the volume cannot take before - // sending any of it; the Job that lands it checks again. + // sending any of it; the Job that lands it checks again. It is null when the + // Job could not read it, so a full volume (0) is never mistaken for that. + var free any = ls.Free + if ls.Free < 0 { + free = nil + } writeJSON(w, http.StatusOK, map[string]any{ - "path": path, "entries": ls.Entries, "truncated": ls.Truncated, "free_bytes": ls.Free, + "path": path, "entries": ls.Entries, "truncated": ls.Truncated, "free_bytes": free, }) } @@ -196,8 +203,10 @@ func (a *API) handleWriteFile(w http.ResponseWriter, r *http.Request) { } // A write holds the world volume for its Job's lifetime (internal/maintenance); - // reads and listings do not, since a read-only mount cannot hurt a server - // starting beside it. + // reads and listings do not. A read-only mount cannot hurt a server starting + // beside it, and a read that overlaps a restore or another change can at worst + // show a file mid-change: the sha256 it returned then no longer matches, so a + // save built on it is refused with file_changed. release, ok := a.acquireWorld(w, r, name, maintenance.KindFileWrite, "stop the server before editing its files") if !ok { return @@ -335,7 +344,10 @@ func (a *API) handleRenameFile(w http.ResponseWriter, r *http.Request) { // // Content-Length is required (411 length_required): the stage reserves room for // the declared size before a byte is written, and a size promised up front is -// what lets a short body be told from a whole one. +// what lets a short body be told from a whole one. So is Content-Digest, the +// SHA-256 the client computed over the body (contentDigest): bytes that arrive +// hashing to anything else are refused with 400 digest_mismatch, and the client +// sends them again. func (a *API) handleUploadFile(w http.ResponseWriter, r *http.Request) { name, ok := a.authorizeFileOp(w, r) if !ok { @@ -345,6 +357,10 @@ func (a *API) handleUploadFile(w http.ResponseWriter, r *http.Request) { if !ok { return } + if !namesFit(path) { + writeError(w, r, errNameTooLong()) + return + } if a.FileStage == nil || a.InternalBaseURL == "" { writeError(w, r, newError(http.StatusServiceUnavailable, "files_unavailable", "uploads are not configured")) @@ -361,10 +377,17 @@ func (a *API) handleUploadFile(w http.ResponseWriter, r *http.Request) { r.ContentLength, fileedit.MaxUploadBytes)) return } + want, ok := contentDigest(w, r) + if !ok { + return + } overwrite := r.URL.Query().Get("overwrite") == "true" - staged, drop, err := a.FileStage.Put(r.Body, r.ContentLength) + staged, drop, err := a.FileStage.Put(r.Body, r.ContentLength, want) switch { + case errors.Is(err, fileedit.ErrDigestMismatch): + writeError(w, r, errDigestMismatch()) + return case errors.Is(err, fileedit.ErrStageFull): writeError(w, r, newError(http.StatusInsufficientStorage, "upload_staging_full", "felis has no room to take this upload right now; try again later or ask an admin")) @@ -429,11 +452,83 @@ func (a *API) handleInternalFileUpload(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/octet-stream") w.Header().Set("Content-Length", strconv.FormatInt(size, 10)) w.WriteHeader(http.StatusOK) - // A session sent whole is on the Job's side now; one cut short stays, so - // committing it again does not mean sending it again. - if n, err := io.Copy(w, f); err == nil && n == size { - a.FileStage.Served(r.PathValue("id")) + _, _ = io.Copy(w, f) +} + +// handleInternalFileUploadLanded serves DELETE +// /api/v1/internal/file-uploads/{id} — the Job that fetched a file sent in +// parts reports it landed, with the token it fetched it by, and the staged copy +// is deleted. Until then the copy stays: a Job that failed after its fetch (the +// digest did not match, the volume filled, a file was in the way) is started +// again by committing again, without the file being sent again. Public on the +// internal face like the fetch, with the same token as the check; a token that +// does not open the upload, or an unknown id, is 404. +func (a *API) handleInternalFileUploadLanded(w http.ResponseWriter, r *http.Request) { + token, ok := strings.CutPrefix(r.Header.Get("Authorization"), "Bearer ") + if a.FileStage == nil || !ok || a.FileStage.Landed(r.PathValue("id"), token) != nil { + writeError(w, r, newError(http.StatusNotFound, "not_found", "no such upload")) + return } + w.WriteHeader(http.StatusNoContent) +} + +// contentDigest reads the SHA-256 a client computed over the body it sends, from +// its Content-Digest header (parseContentDigest). An upload without one is +// refused (400 digest_required): felis-api could not otherwise tell bytes +// changed on the way from the bytes that were sent. It writes the refusal itself +// and reports false. +func contentDigest(w http.ResponseWriter, r *http.Request) ([]byte, bool) { + sum, err := parseContentDigest(r.Header.Values("Content-Digest")) + switch { + case errors.Is(err, errNoContentDigest): + writeError(w, r, newError(http.StatusBadRequest, "digest_required", + "send the SHA-256 of the body as Content-Digest: sha-256=::")) + return nil, false + case err != nil: + writeError(w, r, newError(http.StatusBadRequest, "bad_digest", + "Content-Digest must carry sha-256=::")) + return nil, false + } + return sum, true +} + +var ( + errNoContentDigest = errors.New("no sha-256 Content-Digest") + errBadContentDigest = errors.New("a malformed sha-256 Content-Digest") +) + +// parseContentDigest finds the SHA-256 in the values of a Content-Digest field +// (RFC 9530): sha-256=::, among any other algorithms it lists, which +// are ignored. errNoContentDigest when there is none, errBadContentDigest when +// it is not 32 bytes of base64 between colons. +func parseContentDigest(values []string) ([]byte, error) { + var found []byte + for _, v := range values { + for _, member := range strings.Split(v, ",") { + key, value, _ := strings.Cut(member, "=") + if !strings.EqualFold(strings.TrimSpace(key), "sha-256") { + continue + } + value, _, _ = strings.Cut(value, ";") // parameters + value = strings.TrimSpace(value) + inner, pre := strings.CutPrefix(value, ":") + inner, post := strings.CutSuffix(inner, ":") + sum, err := base64.StdEncoding.DecodeString(inner) + if !pre || !post || err != nil || len(sum) != sha256.Size { + return nil, errBadContentDigest + } + found = sum + } + } + if found == nil { + return nil, errNoContentDigest + } + return found, nil +} + +func errDigestMismatch() error { + return newError(http.StatusBadRequest, "digest_mismatch", + "the bytes that arrived do not match their Content-Digest, so they were changed on the way; send them again") } // auditFile records a file change. The target is ":", as file.write diff --git a/internal/api/handlers_files_test.go b/internal/api/handlers_files_test.go index df3db49..9e82cb0 100644 --- a/internal/api/handlers_files_test.go +++ b/internal/api/handlers_files_test.go @@ -3,6 +3,7 @@ package api import ( "context" "crypto/sha256" + "encoding/base64" "encoding/hex" "encoding/json" "fmt" @@ -125,12 +126,12 @@ func (f *fakeFileEditor) Ops(_ context.Context, server string) ([]fileedit.OpSta return f.ops, f.err } -// fileRouteHeader is the Content-Type a file route's body goes with: raw bytes -// for an upload, JSON for any other body. +// fileRouteHeader is the headers a file route's body goes with: raw bytes and +// their Content-Digest for an upload, JSON for any other body. func fileRouteHeader(name, body string) map[string]string { switch { case name == "upload": - return ctHeader("application/octet-stream") + return map[string]string{"Content-Type": "application/octet-stream", "Content-Digest": contentDigestOf(body)} case body != "": return jsonHeader } @@ -397,6 +398,21 @@ func TestFileEditorHandlers(t *testing.T) { } }) + t.Run("a full volume lists 0 free and one the Job could not measure null", func(t *testing.T) { + for _, c := range []struct { + free int64 + want string + }{{0, `"free_bytes":0`}, {-1, `"free_bytes":null`}} { + api, _, _, files := mkFiles(t) + files.free = c.free + api.External = staticExternal{p: owner} + w := do(api.ExternalHandler(), "GET", "/api/v1/servers/survival/files?path=config", "", nil) + if w.Code != http.StatusOK || !strings.Contains(w.Body.String(), c.want) { + t.Fatalf("free %d: code = %d body %s, want %s", c.free, w.Code, w.Body.String(), c.want) + } + } + }) + t.Run("list without a path lists the world root", func(t *testing.T) { api, _, _, files := mkFiles(t) api.External = staticExternal{p: owner} @@ -744,11 +760,19 @@ func TestFileManagerHandlers(t *testing.T) { }) } +// contentDigestOf is the Content-Digest a client sends with body. +func contentDigestOf(body string) string { + sum := sha256.Sum256([]byte(body)) + return "sha-256=:" + base64.StdEncoding.EncodeToString(sum[:]) + ":" +} + // doUpload sends an upload whose Content-Length is declared, not measured, the -// way a client that streams or lies would send it. +// way a client that streams or lies would send it. Its Content-Digest is that +// of the body. func doUpload(h http.Handler, body string, length int64) *httptest.ResponseRecorder { r := httptest.NewRequest("PUT", "/api/v1/servers/survival/files/upload?path=plugins/x.jar", strings.NewReader(body)) r.Header.Set("Content-Type", "application/octet-stream") + r.Header.Set("Content-Digest", contentDigestOf(body)) r.ContentLength = length w := httptest.NewRecorder() h.ServeHTTP(w, r) @@ -777,7 +801,7 @@ func TestFileUpload(t *testing.T) { digest := sha256.Sum256([]byte(body)) sum := hex.EncodeToString(digest[:]) const route = "/api/v1/servers/survival/files/upload?path=plugins/x.jar" - octet := ctHeader("application/octet-stream") + octet := map[string]string{"Content-Type": "application/octet-stream", "Content-Digest": contentDigestOf(body)} t.Run("the Job fetches the body once, with its token alone", func(t *testing.T) { api, repo, _, files := mkFiles(t) @@ -785,7 +809,7 @@ func TestFileUpload(t *testing.T) { // Every other internal route wants a service token; the Job has none. api.Internal = CallerTokens{CallerVelocity: "s3cr3t"} prefix := api.InternalBaseURL + "/api/v1/internal/file-uploads/" - var bare, wrong, unschemed, fetched, again *httptest.ResponseRecorder + var bare, wrong, unschemed, fetched, again, landedWrong, landed *httptest.ResponseRecorder files.onUpload = func(src fileedit.UploadSource) { id, ok := strings.CutPrefix(src.URL, prefix) if !ok || len(id) != 32 { @@ -799,6 +823,10 @@ func TestFileUpload(t *testing.T) { unschemed = do(h, "GET", at, "", map[string]string{"Authorization": src.Token}) fetched = do(h, "GET", at, "", map[string]string{"Authorization": "Bearer " + src.Token}) again = do(h, "GET", at, "", map[string]string{"Authorization": "Bearer " + src.Token}) + // The Job reports it landed. A single upload goes when its request + // ends, so the report takes nothing away early. + landedWrong = do(h, "DELETE", at, "", map[string]string{"Authorization": "Bearer " + strings.Repeat("0", len(src.Token))}) + landed = do(h, "DELETE", at, "", map[string]string{"Authorization": "Bearer " + src.Token}) } w := do(api.ExternalHandler(), "PUT", route, body, octet) @@ -810,6 +838,7 @@ func TestFileUpload(t *testing.T) { } for name, r := range map[string]*httptest.ResponseRecorder{ "no token": bare, "wrong token": wrong, "the token without Bearer": unschemed, "second fetch": again, + "a landed report with the wrong token": landedWrong, } { if r.Code != http.StatusNotFound || decodeErr(t, r) != "not_found" { t.Errorf("%s: code = %d (%s), want 404 not_found", name, r.Code, r.Body.String()) @@ -819,6 +848,9 @@ func TestFileUpload(t *testing.T) { fetched.Header().Get("Content-Length") != strconv.Itoa(len(body)) { t.Fatalf("fetch: code = %d, %q, Content-Length %q", fetched.Code, fetched.Body.String(), fetched.Header().Get("Content-Length")) } + if landed.Code != http.StatusNoContent { + t.Fatalf("landed: code = %d (%s)", landed.Code, landed.Body.String()) + } if files.gotPath != "plugins/x.jar" || files.gotSource.Size != int64(len(body)) || files.gotSource.SHA256 != sum || files.gotOverwrite { t.Fatalf("executor saw path %q, source %+v, overwrite %v", files.gotPath, files.gotSource, files.gotOverwrite) @@ -868,6 +900,51 @@ func TestFileUpload(t *testing.T) { stageEmpty(t, api) }) + t.Run("the body comes with its SHA-256, checked before anything is asked of the world", func(t *testing.T) { + other := sha256.Sum256([]byte("PK\x03\x04 another jar")) + for _, c := range []struct { + name string + digest []string + code int + errCode string + }{ + {"none", nil, http.StatusBadRequest, "digest_required"}, + {"only another algorithm", []string{"sha-512=:" + base64.StdEncoding.EncodeToString(make([]byte, 64)) + ":"}, http.StatusBadRequest, "digest_required"}, + {"hex in place of base64", []string{"sha-256=:" + sum + ":"}, http.StatusBadRequest, "bad_digest"}, + {"a bare base64 value", []string{strings.Trim(strings.TrimPrefix(contentDigestOf(body), "sha-256="), ":")}, http.StatusBadRequest, "digest_required"}, + {"no closing colon", []string{strings.TrimSuffix(contentDigestOf(body), ":")}, http.StatusBadRequest, "bad_digest"}, + {"base64 without colons", []string{"sha-256=" + strings.Trim(strings.TrimPrefix(contentDigestOf(body), "sha-256="), ":")}, http.StatusBadRequest, "bad_digest"}, + {"31 bytes", []string{"sha-256=:" + base64.StdEncoding.EncodeToString(digest[:31]) + ":"}, http.StatusBadRequest, "bad_digest"}, + {"another body's", []string{"sha-256=:" + base64.StdEncoding.EncodeToString(other[:]) + ":"}, http.StatusBadRequest, "digest_mismatch"}, + {"the right one, then a wrong one", []string{contentDigestOf(body), "sha-256=:" + base64.StdEncoding.EncodeToString(other[:]) + ":"}, http.StatusBadRequest, "digest_mismatch"}, + {"among others, upper case, with a parameter", []string{"sha-512=:" + base64.StdEncoding.EncodeToString(make([]byte, 64)) + ":, SHA-256=" + strings.TrimPrefix(contentDigestOf(body), "sha-256=") + ";p=1"}, http.StatusOK, ""}, + } { + t.Run(c.name, func(t *testing.T) { + api, repo, cl, files := mkFiles(t) + api.External = staticExternal{p: owner} + r := httptest.NewRequest("PUT", route, strings.NewReader(body)) + r.Header.Set("Content-Type", "application/octet-stream") + for _, v := range c.digest { + r.Header.Add("Content-Digest", v) + } + w := httptest.NewRecorder() + api.ExternalHandler().ServeHTTP(w, r) + recordContract(r, body, w) + if c.code == http.StatusOK { + if w.Code != http.StatusOK || files.calls != 1 || files.gotSource.SHA256 != sum { + t.Fatalf("code = %d calls = %d source %+v (%s)", w.Code, files.calls, files.gotSource, w.Body.String()) + } + return + } + if w.Code != c.code || decodeErr(t, w) != c.errCode || files.calls != 0 || len(cl.acquired) != 0 || len(repo.audits) != 0 { + t.Fatalf("code = %d calls = %d acquired %v audits %d (%s), want %d %s", + w.Code, files.calls, cl.acquired, len(repo.audits), w.Body.String(), c.code, c.errCode) + } + stageEmpty(t, api) + }) + } + }) + t.Run("no Content-Length -> 411", func(t *testing.T) { api, _, _, files := mkFiles(t) api.External = staticExternal{p: owner} @@ -887,6 +964,28 @@ func TestFileUpload(t *testing.T) { stageEmpty(t, api) }) + t.Run("a name longer than a folder entry holds -> 400 before a byte is staged", func(t *testing.T) { + for _, c := range []struct { + name string + code int + errCode string + }{ + {strings.Repeat("n", fileedit.NameMax-3) + ".jar", http.StatusBadRequest, "bad_path"}, + {strings.Repeat("n", fileedit.NameMax-4) + ".jar", http.StatusOK, ""}, + } { + api, _, _, files := mkFiles(t) + api.External = staticExternal{p: owner} + w := do(api.ExternalHandler(), "PUT", "/api/v1/servers/survival/files/upload?path=plugins/"+c.name, body, octet) + if w.Code != c.code || (c.errCode != "" && decodeErr(t, w) != c.errCode) { + t.Fatalf("%d-byte name: code = %d (%s), want %d", len(c.name), w.Code, w.Body.String(), c.code) + } + if wantCalls := map[bool]int{true: 1, false: 0}[c.code == http.StatusOK]; files.calls != wantCalls { + t.Fatalf("%d-byte name: executor calls = %d, want %d", len(c.name), files.calls, wantCalls) + } + stageEmpty(t, api) + } + }) + // Staged, and so short: the body is a few bytes of a declared 64 MiB. t.Run("a declared size at the cap is taken", func(t *testing.T) { api, _, _, files := mkFiles(t) @@ -935,10 +1034,12 @@ func TestFileUpload(t *testing.T) { t.Run("the internal route without a stage -> 404", func(t *testing.T) { api, _, _, _ := mkFiles(t) api.FileStage = nil - w := do(api.InternalHandler(), "GET", "/api/v1/internal/file-uploads/00112233445566778899aabbccddeeff", "", - map[string]string{"Authorization": "Bearer t"}) - if w.Code != http.StatusNotFound || decodeErr(t, w) != "not_found" { - t.Fatalf("code = %d (%s)", w.Code, w.Body.String()) + for _, method := range []string{"GET", "DELETE"} { + w := do(api.InternalHandler(), method, "/api/v1/internal/file-uploads/00112233445566778899aabbccddeeff", "", + map[string]string{"Authorization": "Bearer t"}) + if w.Code != http.StatusNotFound || decodeErr(t, w) != "not_found" { + t.Fatalf("%s: code = %d (%s)", method, w.Code, w.Body.String()) + } } }) } diff --git a/internal/api/jobstatus_test.go b/internal/api/jobstatus_test.go index f9fb503..fc1b9b7 100644 --- a/internal/api/jobstatus_test.go +++ b/internal/api/jobstatus_test.go @@ -179,6 +179,9 @@ func TestLatestJobsExplainsFailures(t *testing.T) { }}}, } } + // Killed for memory mid-walk: the last line it printed says nothing of that. + oom := pod("c-1", "backup-survival-c", 4, 137, "archiving survival\n") + oom.Status.ContainerStatuses[0].State.Terminated.Reason = "OOMKilled" c := fake.NewClientBuilder().WithScheme(scheme).WithObjects( failedJob("backup-survival-a", 1), failedJob("backup-survival-b", 2), @@ -186,6 +189,7 @@ func TestLatestJobsExplainsFailures(t *testing.T) { pod("a-2", "backup-survival-a", 3, 1, "archiving survival\nfelis backup: backup: not enough free disk for the archive: the world is 2.0 GiB\n"), pod("b-1", "backup-survival-b", 2, 0, "done"), + failedJob("backup-survival-c", 4), oom, ).Build() jobs, err := NewK8sJobStatus(c, "minecraft").LatestJobs(context.Background(), "survival") @@ -202,4 +206,7 @@ func TestLatestJobsExplainsFailures(t *testing.T) { if want := "Job has reached the specified backoff limit"; got["backup-survival-b"] != want { t.Errorf("b: message = %q, want the condition text", got["backup-survival-b"]) } + if want := "the job ran out of memory and the system stopped it (OOMKilled)"; got["backup-survival-c"] != want { + t.Errorf("c: message = %q, want %q", got["backup-survival-c"], want) + } } diff --git a/internal/api/k8sjobstatus.go b/internal/api/k8sjobstatus.go index d2acaf3..cbc15be 100644 --- a/internal/api/k8sjobstatus.go +++ b/internal/api/k8sjobstatus.go @@ -7,6 +7,7 @@ import ( "strings" "time" + "felis.lolicon.best/internal/fileedit" "felis.lolicon.best/internal/maintenance" batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" @@ -107,14 +108,23 @@ func (k *K8sJobStatus) explainFailures(ctx context.Context, serverName string, j } } +// outOfMemoryLine is a failed Job's message when the kernel killed it for +// memory. The reason in brackets is what the panel knows it by. +const outOfMemoryLine = "the job ran out of memory and the system stopped it (OOMKilled)" + // lastTerminationLine returns the last non-empty line of the pod's terminated -// container message, capped for display. +// container message, capped for display. A container the kernel killed for +// going over its memory limit printed nothing about it, so that is said instead +// of whatever line it happened to print last. func lastTerminationLine(pod *corev1.Pod) string { for _, cs := range pod.Status.ContainerStatuses { t := cs.State.Terminated if t == nil || t.ExitCode == 0 { continue } + if t.Reason == fileedit.ReasonOOMKilled { + return outOfMemoryLine + } lines := strings.Split(strings.TrimSpace(t.Message), "\n") line := strings.TrimSpace(lines[len(lines)-1]) if len(line) > 400 { diff --git a/internal/api/maintenance.go b/internal/api/maintenance.go index 2e2b366..6e2cf2b 100644 --- a/internal/api/maintenance.go +++ b/internal/api/maintenance.go @@ -38,7 +38,7 @@ func maintenanceLabel(kind string) string { case maintenance.KindFileWrite: return "a file write" case maintenance.KindExport: - return "a world export" + return "a world export or file download" case maintenance.KindReap: return "the idle-world reaper" } diff --git a/internal/api/submissions.go b/internal/api/submissions.go index 33a5faf..fc8c706 100644 --- a/internal/api/submissions.go +++ b/internal/api/submissions.go @@ -166,6 +166,10 @@ func (a *API) handleCreateSubmission(w http.ResponseWriter, r *http.Request) { // caller does not own is reported as 404, so this endpoint cannot upload to — or // probe the existence of — another user's submission. // +// The body carries its SHA-256 as Content-Digest (contentDigest); bytes that hash +// to anything else were changed on the way, and none of them are kept +// (submit.VerifyDigest). +// // Uploading does not change the submission row (there is no "uploaded" column): // the blob store is the presence source of truth, which admin approval consults. func (a *API) handleUploadSubmissionContext(w http.ResponseWriter, r *http.Request) { @@ -173,6 +177,10 @@ func (a *API) handleUploadSubmissionContext(w http.ResponseWriter, r *http.Reque writeError(w, r, errSubmissionsUnavailable) return } + want, ok := contentDigest(w, r) + if !ok { + return + } p := principalFromContext(r.Context()) // Reserve the per-user upload cooldown BEFORE streaming. The body is the // expensive part (up to the 1 GiB blob cap), so without a reservation the @@ -188,7 +196,7 @@ func (a *API) handleUploadSubmissionContext(w http.ResponseWriter, r *http.Reque } defer release() id := r.PathValue("id") - sub, err := a.Submissions.UploadContext(r.Context(), id, p.UserID, r.Body) + sub, err := a.Submissions.UploadContext(r.Context(), id, p.UserID, submit.VerifyDigest(r.Body, want)) if err != nil { writeSubmitError(w, r, err) return @@ -240,7 +248,8 @@ func (a *API) handleContextUploadStatus(w http.ResponseWriter, r *http.Request) // else must equal the staged length (409 upload_offset_mismatch otherwise). A // part is small enough for any edge, so no cooldown applies here: the staged // total is bounded by the context cap and the storage budget, and completion -// holds the cooldown. +// holds the cooldown. Each part carries its own Content-Digest, and one that +// does not hash to it is cut back off, as a part that breaks off is. func (a *API) handleContextUploadPart(w http.ResponseWriter, r *http.Request) { if a.Submissions == nil { writeError(w, r, errSubmissionsUnavailable) @@ -252,8 +261,12 @@ func (a *API) handleContextUploadPart(w http.ResponseWriter, r *http.Request) { "offset must be the byte position the part starts at")) return } + want, ok := contentDigest(w, r) + if !ok { + return + } p := principalFromContext(r.Context()) - prog, err := a.Submissions.UploadPart(r.Context(), r.PathValue("id"), p.UserID, offset, r.Body) + prog, err := a.Submissions.UploadPart(r.Context(), r.PathValue("id"), p.UserID, offset, submit.VerifyDigest(r.Body, want)) if err != nil { writeSubmitError(w, r, err) return @@ -557,6 +570,8 @@ func writeSubmitError(w http.ResponseWriter, r *http.Request, err error) { case errors.Is(err, submit.ErrUploadBusy): writeError(w, r, newError(http.StatusConflict, "upload_busy", "another request is still writing this upload; read where it stands and continue from there")) + case errors.Is(err, submit.ErrDigestMismatch): + writeError(w, r, errDigestMismatch()) case errors.As(err, &mismatch): writeError(w, r, newError(http.StatusConflict, "upload_offset_mismatch", "the upload holds %d bytes; send the part that starts there", mismatch.Received)) diff --git a/internal/api/submissions_chunked_test.go b/internal/api/submissions_chunked_test.go index 8c426b7..328ffe4 100644 --- a/internal/api/submissions_chunked_test.go +++ b/internal/api/submissions_chunked_test.go @@ -14,7 +14,7 @@ func TestContextUploadPartForwardsOffsetBodyAndPrincipal(t *testing.T) { fs := &fakeSubmissions{progress: submit.UploadProgress{Received: 8, PartMaxBytes: 33554432, MaxContextBytes: 1073741824}} api := appSubAPI(fs) w := do(api.ExternalHandler(), "PUT", "/api/v1/me/submissions/sub-9/context/upload?offset=4", "abcd", - ctHeader("application/octet-stream")) + map[string]string{"Content-Type": "application/octet-stream", "Content-Digest": contentDigestOf("abcd")}) if w.Code != http.StatusOK { t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String()) } @@ -72,7 +72,8 @@ func TestContextUploadErrors(t *testing.T) { 503, "uploads_store_unavailable", "send the request again", "5"}, } { fs := &fakeSubmissions{chunkErr: tc.err} - w := do(appSubAPI(fs).ExternalHandler(), "PUT", "/api/v1/me/submissions/sub-9/context/upload?offset=12", "abcd", nil) + w := do(appSubAPI(fs).ExternalHandler(), "PUT", "/api/v1/me/submissions/sub-9/context/upload?offset=12", "abcd", + map[string]string{"Content-Digest": contentDigestOf("abcd")}) if w.Code != tc.code || decodeErr(t, w) != tc.want { t.Errorf("%s: %d %s, want %d %s", tc.name, w.Code, w.Body.String(), tc.code, tc.want) } @@ -122,7 +123,8 @@ func TestContextUploadCompleteHoldsTheCooldownAndAudits(t *testing.T) { t.Fatalf("second completion in the window: %d %s, want 429 submission_cooldown", w.Code, w.Body.String()) } // A part never waits on the cooldown. - if w := do(eh, "PUT", "/api/v1/me/submissions/sub-9/context/upload?offset=0", "\x1f\x8b", nil); w.Code != http.StatusOK { + if w := do(eh, "PUT", "/api/v1/me/submissions/sub-9/context/upload?offset=0", "\x1f\x8b", + map[string]string{"Content-Digest": contentDigestOf("\x1f\x8b")}); w.Code != http.StatusOK { t.Fatalf("part inside the cooldown: code = %d, want 200 (%s)", w.Code, w.Body.String()) } } @@ -141,3 +143,42 @@ func TestContextUploadWithoutServiceIs503(t *testing.T) { } } } + +// A context upload, whole or in parts, carries the SHA-256 of its body: one +// without it is refused before the lane sees it, and bytes that do not hash to +// it are refused as changed on the way (the lane keeps none of them), so the +// client sends them again. +func TestContextUploadsCheckTheBodyDigest(t *testing.T) { + body := "\x1f\x8b\x08\x00 the modpack bytes" + for _, rq := range []struct{ name, method, target string }{ + {"a part", "PUT", "/api/v1/me/submissions/sub-9/context/upload?offset=0"}, + {"a whole context", "POST", "/api/v1/me/submissions/sub-9/context"}, + } { + for _, tc := range []struct { + name string + sent string + headers map[string]string + code int + want string + }{ + {"without a digest", body, nil, http.StatusBadRequest, "digest_required"}, + {"changed on the way", body[:len(body)-1] + "X", map[string]string{"Content-Digest": contentDigestOf(body)}, http.StatusBadRequest, "digest_mismatch"}, + {"as sent", body, map[string]string{"Content-Digest": contentDigestOf(body)}, http.StatusOK, ""}, + } { + t.Run(rq.name+" "+tc.name, func(t *testing.T) { + fs := &fakeSubmissions{} + w := do(appSubAPI(fs).ExternalHandler(), rq.method, rq.target, tc.sent, tc.headers) + if w.Code != tc.code { + t.Fatalf("code = %d (%s), want %d", w.Code, w.Body.String(), tc.code) + } + if tc.want != "" && decodeErr(t, w) != tc.want { + t.Fatalf("error = %s, want %s", w.Body.String(), tc.want) + } + reached := fs.chunkID != "" || fs.uploadedID != "" + if reached != (tc.headers != nil) { + t.Fatalf("the body reached the lane: %v, want %v", reached, tc.headers != nil) + } + }) + } + } +} diff --git a/internal/api/submissions_test.go b/internal/api/submissions_test.go index 8241982..0cce31d 100644 --- a/internal/api/submissions_test.go +++ b/internal/api/submissions_test.go @@ -73,8 +73,13 @@ func (f *fakeSubmissions) UploadStatus(_ context.Context, id, submittedBy string func (f *fakeSubmissions) UploadPart(_ context.Context, id, submittedBy string, offset int64, r io.Reader) (submit.UploadProgress, error) { f.chunkID, f.chunkBy, f.partOffset = id, submittedBy, offset - b, _ := io.ReadAll(r) + b, err := io.ReadAll(r) f.partBody = string(b) + if err != nil { + // A read that fails (a body changed on the way) keeps nothing, as in + // PartStore. + return submit.UploadProgress{}, err + } return f.progress, f.chunkErr } @@ -101,7 +106,10 @@ func (f *fakeSubmissions) UploadContext(_ context.Context, id, submittedBy strin if f.uploadErr != nil { return nil, f.uploadErr } - n, _ := io.Copy(io.Discard, r) + n, err := io.Copy(io.Discard, r) + if err != nil { + return nil, err + } f.uploadedN = n return &submit.Submission{ID: id, SubmittedBy: submittedBy, Status: submit.StatusPendingReview}, nil } @@ -235,7 +243,7 @@ func TestUploadSubmissionContextStreamsBody(t *testing.T) { // A tiny gzip-magic-prefixed body stands in for a real context.tar.gz. body := "\x1f\x8b\x08\x00 the modpack bytes" w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", body, - map[string]string{"Content-Type": "application/gzip"}) + map[string]string{"Content-Type": "application/gzip", "Content-Digest": contentDigestOf(body)}) if w.Code != http.StatusOK { t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String()) } @@ -255,7 +263,7 @@ func TestUploadSubmissionContextStreamsBody(t *testing.T) { func TestUploadSubmissionContextNotOwnedIs404(t *testing.T) { fs := &fakeSubmissions{uploadErr: submit.ErrNotFound} api := appSubAPI(fs) - w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-x/context", "\x1f\x8bdata", nil) + w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-x/context", "\x1f\x8bdata", map[string]string{"Content-Digest": contentDigestOf("\x1f\x8bdata")}) if w.Code != http.StatusNotFound { t.Fatalf("code = %d, want 404 (%s)", w.Code, w.Body.String()) } @@ -265,7 +273,7 @@ func TestUploadSubmissionContextNotOwnedIs404(t *testing.T) { func TestUploadSubmissionContextAlreadyReviewedIs409(t *testing.T) { fs := &fakeSubmissions{uploadErr: submit.ErrAlreadyReviewed} api := appSubAPI(fs) - w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", "\x1f\x8bdata", nil) + w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", "\x1f\x8bdata", map[string]string{"Content-Digest": contentDigestOf("\x1f\x8bdata")}) if w.Code != http.StatusConflict { t.Fatalf("code = %d, want 409 (%s)", w.Code, w.Body.String()) } @@ -275,7 +283,7 @@ func TestUploadSubmissionContextAlreadyReviewedIs409(t *testing.T) { func TestUploadSubmissionContextBadFormatIs400(t *testing.T) { fs := &fakeSubmissions{uploadErr: fmt.Errorf("%w: build context must be a gzip-compressed tarball (.tar.gz)", submit.ErrInvalid)} api := appSubAPI(fs) - w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", "not gzip", nil) + w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", "not gzip", map[string]string{"Content-Digest": contentDigestOf("not gzip")}) if w.Code != http.StatusBadRequest { t.Fatalf("code = %d, want 400 (%s)", w.Code, w.Body.String()) } @@ -289,7 +297,7 @@ func TestUploadSubmissionContextBadFormatIs400(t *testing.T) { func TestUploadSubmissionContextNoTransportIs503(t *testing.T) { fs := &fakeSubmissions{uploadErr: submit.ErrUploadsUnavailable} api := appSubAPI(fs) - w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", "\x1f\x8bdata", nil) + w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", "\x1f\x8bdata", map[string]string{"Content-Digest": contentDigestOf("\x1f\x8bdata")}) if w.Code != http.StatusServiceUnavailable { t.Fatalf("code = %d, want 503 (%s)", w.Code, w.Body.String()) } @@ -303,7 +311,7 @@ func TestUploadSubmissionContextNoTransportIs503(t *testing.T) { func TestUploadSubmissionContextWithoutServiceIs503(t *testing.T) { app := appSubAPI(nil) app.Submissions = nil - w := do(app.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", "\x1f\x8bdata", nil) + w := do(app.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", "\x1f\x8bdata", map[string]string{"Content-Digest": contentDigestOf("\x1f\x8bdata")}) if w.Code != http.StatusServiceUnavailable { t.Fatalf("code = %d, want 503 (%s)", w.Code, w.Body.String()) } @@ -871,7 +879,7 @@ func TestSubmissionQuotaIs403(t *testing.T) { }) t.Run("upload", func(t *testing.T) { fs := &fakeSubmissions{uploadErr: fmt.Errorf("%w: exceeds your remaining storage allowance", submit.ErrQuotaExceeded)} - w := do(appSubAPI(fs).ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", "\x1f\x8bdata", nil) + w := do(appSubAPI(fs).ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", "\x1f\x8bdata", map[string]string{"Content-Digest": contentDigestOf("\x1f\x8bdata")}) if w.Code != http.StatusForbidden { t.Fatalf("code = %d, want 403 (%s)", w.Code, w.Body.String()) } @@ -939,11 +947,12 @@ func TestUploadSubmissionContextRateLimited(t *testing.T) { api.SubmitUploadCooldown = time.Minute eh := api.ExternalHandler() body := "\x1f\x8b\x08\x00 the modpack bytes" + sent := map[string]string{"Content-Digest": contentDigestOf(body)} - if w := do(eh, "POST", "/api/v1/me/submissions/sub-9/context", body, nil); w.Code != http.StatusOK { + if w := do(eh, "POST", "/api/v1/me/submissions/sub-9/context", body, sent); w.Code != http.StatusOK { t.Fatalf("first upload: code = %d, want 200 (%s)", w.Code, w.Body.String()) } - w := do(eh, "POST", "/api/v1/me/submissions/sub-9/context", body, nil) + w := do(eh, "POST", "/api/v1/me/submissions/sub-9/context", body, sent) if w.Code != http.StatusTooManyRequests || decodeErr(t, w) != "submission_cooldown" { t.Fatalf("immediate second upload: code = %d body %s, want 429 submission_cooldown", w.Code, w.Body.String()) } @@ -955,11 +964,11 @@ func TestUploadSubmissionContextRateLimited(t *testing.T) { api2.Now = func() time.Time { return clock } api2.SubmitUploadCooldown = time.Minute eh2 := api2.ExternalHandler() - if w := do(eh2, "POST", "/api/v1/me/submissions/sub-9/context", body, nil); w.Code != http.StatusServiceUnavailable { + if w := do(eh2, "POST", "/api/v1/me/submissions/sub-9/context", body, sent); w.Code != http.StatusServiceUnavailable { t.Fatalf("failed upload: code = %d, want 503", w.Code) } fs2.uploadErr = nil - if w := do(eh2, "POST", "/api/v1/me/submissions/sub-9/context", body, nil); w.Code != http.StatusOK { + if w := do(eh2, "POST", "/api/v1/me/submissions/sub-9/context", body, sent); w.Code != http.StatusOK { t.Fatalf("retry at the same instant after failure: code = %d, want 200 (%s)", w.Code, w.Body.String()) } } diff --git a/internal/fileedit/editor.go b/internal/fileedit/editor.go index 898bebc..38fca7a 100644 --- a/internal/fileedit/editor.go +++ b/internal/fileedit/editor.go @@ -221,8 +221,8 @@ type Listing struct { Entries []Entry // Truncated reports that the directory holds more than MaxEntries. Truncated bool - // Free is the bytes free on the server's volume, 0 when the Job could not - // tell. + // Free is the bytes free on the server's volume, negative when the Job could + // not tell. Free int64 } @@ -394,7 +394,8 @@ type OpState struct { Done, Total int64 // Result is what the Job printed once it finished. It is nil while the Job // runs, and for a Job that ended without printing one (killed at its - // deadline, out of memory, its bytes unfetchable), whose Reason says why. + // deadline, out of memory, its bytes unfetchable), whose Reason says why + // (ReasonOOMKilled for memory). Result *Result Reason string } diff --git a/internal/fileedit/exec.go b/internal/fileedit/exec.go index 8dc25f8..8afc37d 100644 --- a/internal/fileedit/exec.go +++ b/internal/fileedit/exec.go @@ -188,15 +188,16 @@ type Result struct { SHA256 string `json:"sha256,omitempty"` // Conflicts lists, relative to the root and sorted, the existing files an - // unzip would replace: the first MaxConflicts of them. ConflictCount is how - // many there are in all. + // unzip would replace: the first of them, up to MaxConflicts and 8 KiB of + // names (see maxConflictBytes). ConflictCount is how many there are in all. Conflicts []string `json:"conflicts,omitempty"` ConflictCount int `json:"conflict_count,omitempty"` // Entry names what an unzip refused: the archive entry, or the path on the // server it collides with. Entry string `json:"entry,omitempty"` // Need and Avail are, on a no_space an upload or unzip saw coming, the bytes - // it needs and the bytes the volume has free. A listing sets Avail too. + // it needs and the bytes the volume has free. A listing sets Avail too, to + // -1 when it could not read it. Need int64 `json:"need,omitempty"` Avail int64 `json:"avail,omitempty"` // Files and Bytes are what a successful unzip extracted. @@ -239,6 +240,10 @@ type Upload struct { // Open starts the transfer. It runs only once the target has passed every // check, so a refused upload never pulls the bytes. Open func() (io.ReadCloser, error) + // Landed, when set, runs once the bytes are in place, so felis-api can let + // go of the copy it staged (Stage.Landed). Until then felis-api keeps it, + // and a failed landing is started again without the bytes being sent again. + Landed func() } // Execute performs one operation inside root and returns the Result to print. @@ -346,7 +351,9 @@ func list(r *os.Root, rootPath, path string) Result { } entries = append(entries, e) } - res := Result{Entries: entries, Truncated: truncated} + // A full volume is Avail 0, which the JSON leaves out; -1 is a volume whose + // free space could not be read, so the two stay apart. + res := Result{Entries: entries, Truncated: truncated, Avail: -1} if avail, _, err := statfs(rootPath); err == nil { res.Avail = int64(min(avail, math.MaxInt64)) } @@ -557,6 +564,10 @@ type transferError struct{ err error } func (e *transferError) Error() string { return "fetch upload: " + e.err.Error() } func (e *transferError) Unwrap() error { return e.err } +// NameMax is the longest name a folder entry can have on the volumes a world +// lives on (NAME_MAX). +const NameMax = 255 + // land atomically puts the bytes fill writes at target, the path landingTarget // returned for name. Write and upload both land through it; unzip lands a whole // tree at once and has its own path (unzip.go). @@ -567,8 +578,9 @@ func (e *transferError) Unwrap() error { return e.err } // server.properties an in-place truncate would, which is a server that no longer // boots. The sibling gets mode and is handed to the game uid before the rename, so // the file the server finds is never root's. On failure it is removed; only a kill -// between create and rename leaves one behind, named "..felis-edit-" so -// no loader mistakes it for a plugin jar or a config. +// between create and rename leaves one behind, named ".felis-edit-" so no +// loader mistakes it for a plugin jar or a config. The name is its own rather than +// the target's with a suffix, so a target named up to NameMax bytes can be written. // // A *transferError from fill comes back as the error; every other failure is a // Result. @@ -577,7 +589,7 @@ func land(r *os.Root, name, target string, mode fs.FileMode, fill func(io.Writer if _, err := rand.Read(suffix[:]); err != nil { return Result{Code: CodeBadPath, Error: fmt.Sprintf("generate a temporary name: %v", err)}, nil } - tmp := path.Join(path.Dir(target), "."+path.Base(target)+".felis-edit-"+hex.EncodeToString(suffix[:])) + tmp := path.Join(path.Dir(target), ".felis-edit-"+hex.EncodeToString(suffix[:])) f, err := r.OpenFile(tmp, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode) if err != nil { return writeFailure(err, name), nil @@ -652,7 +664,7 @@ func upload(r *os.Root, rootPath, name string, u Upload, overwrite bool, progres return Result{Code: CodeNoSpace, Need: u.Size, Avail: int64(min(avail, math.MaxInt64)), Error: fmt.Sprintf( "%s is %d bytes and the server's volume has %d free; nothing was changed", name, u.Size, avail)}, nil } - return land(r, name, target, mode, func(w io.Writer) error { + res, err := land(r, name, target, mode, func(w io.Writer) error { body, err := u.Open() if err != nil { return &transferError{err} @@ -677,6 +689,10 @@ func upload(r *os.Root, rootPath, name string, u Upload, overwrite bool, progres } return nil }) + if err == nil && res.Code == "" && u.Landed != nil { + u.Landed() + } + return res, err } // sourceReader tags the source's read errors as transfer errors, so land can tell diff --git a/internal/fileedit/exec_test.go b/internal/fileedit/exec_test.go index c1897a5..c37a7f7 100644 --- a/internal/fileedit/exec_test.go +++ b/internal/fileedit/exec_test.go @@ -198,10 +198,15 @@ func TestExecuteHappyPath(t *testing.T) { t.Fatalf("avail = %d, want it clamped to %d", res.Avail, int64(math.MaxInt64)) } + statfs = func(string) (uint64, uint64, error) { return 0, 99999, nil } + if res, _ := run(root, OpList, "config", nil, ""); res.Avail != 0 { + t.Fatalf("avail = %d on a full volume, want 0", res.Avail) + } + statfs = func(string) (uint64, uint64, error) { return 1, 1, errors.New("no statfs") } res, err = run(root, OpList, "config", nil, "") - if err != nil || res.Code != "" || len(res.Entries) != 1 || res.Avail != 0 { - t.Fatalf("a volume that cannot be measured still lists, with no room reported: %v %+v", err, res) + if err != nil || res.Code != "" || len(res.Entries) != 1 || res.Avail != -1 { + t.Fatalf("a volume that cannot be measured still lists, with its room -1 (unknown): %v %+v", err, res) } }) @@ -239,7 +244,7 @@ func TestExecuteHappyPath(t *testing.T) { if res, err := run(root, OpWrite, "ops.json", []byte("[]"), ""); err != nil || res.Code != "" { t.Fatalf("creating a new file should succeed: %v / %+v", err, res) } - if len(owned) != 1 || !strings.HasPrefix(owned[0], ".ops.json.felis-edit-") { + if len(owned) != 1 || !strings.HasPrefix(owned[0], ".felis-edit-") { t.Errorf("files handed to the game uid = %v, want the one temporary sibling of ops.json", owned) } res, err := run(root, OpWrite, "nope/deep.txt", []byte("x"), "") @@ -537,6 +542,18 @@ func TestWriteIsAtomic(t *testing.T) { assertNoTemporaries(t, root) }) + t.Run("a name as long as a folder allows is written", func(t *testing.T) { + long := strings.Repeat("n", NameMax-4) + ".yml" + res, err := Execute(root, Request{Op: OpWrite, Path: "config/" + long, Content: []byte("a: 1\n"), CreateOnly: true}) + if err != nil || res.Code != "" { + t.Fatalf("write a %d-byte name: %v / %+v", len(long), err, res) + } + if b, _ := os.ReadFile(filepath.Join(root, "config", long)); string(b) != "a: 1\n" { + t.Fatalf("content = %q", b) + } + assertNoTemporaries(t, filepath.Join(root, "config")) + }) + t.Run("a link inside the root is written through, not replaced", func(t *testing.T) { if err := os.Symlink("config/paper.yml", filepath.Join(root, "paper-link.yml")); err != nil { t.Skipf("symlinks unavailable: %v", err) diff --git a/internal/fileedit/k8sjobs.go b/internal/fileedit/k8sjobs.go index 818b0d0..949e163 100644 --- a/internal/fileedit/k8sjobs.go +++ b/internal/fileedit/k8sjobs.go @@ -246,10 +246,11 @@ func (k *K8sRunner) Ops(ctx context.Context, namespace, server string) ([]OpStat out := make([]OpState, 0, len(items)) for i := range items { log := "" - if pod := podOf[items[i].Labels[LabelOpID]]; pod != nil && pod.Status.Phase != corev1.PodPending { + pod := podOf[items[i].Labels[LabelOpID]] + if pod != nil && pod.Status.Phase != corev1.PodPending { log, _ = k.logTail(ctx, namespace, pod.Name) } - out = append(out, opState(&items[i], log)) + out = append(out, opState(&items[i], pod, log)) } return out, nil } @@ -272,9 +273,11 @@ func (k *K8sRunner) logTail(ctx context.Context, namespace, pod string) (string, // OpState. The printed result decides the outcome whatever the Job's condition // says: a Job killed at its deadline just after printing did finish its work. // A Job that ended without one failed, and the condition's reason says how -// (DeadlineExceeded, BackoffLimitExceeded); a Job that completed but whose log -// could not be read has an outcome no one can tell, ResultUnavailable. -func opState(job *batchv1.Job, log string) OpState { +// (DeadlineExceeded, BackoffLimitExceeded), unless its Pod says the kernel +// killed it for memory (ReasonOOMKilled), which the condition does not tell; a +// Job that completed but whose log could not be read has an outcome no one can +// tell, ResultUnavailable. +func opState(job *batchv1.Job, pod *corev1.Pod, log string) OpState { st := OpState{ ID: job.Labels[LabelOpID], Op: job.Labels[LabelMode], Path: job.Annotations[AnnotationPath], State: OpRunning, Started: job.CreationTimestamp.Time, @@ -312,9 +315,30 @@ func opState(job *batchv1.Job, log string) OpState { st.State = OpFailed default: st.State, st.Reason = OpFailed, reason - if st.Reason == "" { + if killedForMemory(pod) { + st.Reason = ReasonOOMKilled + } else if st.Reason == "" { st.Reason = "Failed" } } return st } + +// ReasonOOMKilled is an op's Reason when the kernel killed its Job for going over +// the Job's memory limit: an archive of more entries than the Job can hold the +// list of. +const ReasonOOMKilled = "OOMKilled" + +// killedForMemory reports whether pod's container was killed for going over its +// memory limit. +func killedForMemory(pod *corev1.Pod) bool { + if pod == nil { + return false + } + for _, cs := range pod.Status.ContainerStatuses { + if t := cs.State.Terminated; cs.Name == containerName && t != nil && t.Reason == ReasonOOMKilled { + return true + } + } + return false +} diff --git a/internal/fileedit/k8sjobs_test.go b/internal/fileedit/k8sjobs_test.go index 12098bb..de22b6e 100644 --- a/internal/fileedit/k8sjobs_test.go +++ b/internal/fileedit/k8sjobs_test.go @@ -45,10 +45,17 @@ func TestOpState(t *testing.T) { ok := ResultPrefix + `{"files":3,"bytes":40}` + "\n" conflict := ResultPrefix + `{"code":"exists","conflicts":["a.txt"],"conflict_count":1}` + "\n" complete := cond(batchv1.JobComplete, corev1.ConditionTrue, "") + backoff := cond(batchv1.JobFailed, corev1.ConditionTrue, "BackoffLimitExceeded") + killed := func(container, reason string) *corev1.Pod { + return &corev1.Pod{Status: corev1.PodStatus{Phase: corev1.PodFailed, ContainerStatuses: []corev1.ContainerStatus{{ + Name: container, State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 137, Reason: reason}}, + }}}} + } cases := []struct { name string conds []batchv1.JobCondition + pod *corev1.Pod log string state string reason string @@ -90,10 +97,22 @@ func TestOpState(t *testing.T) { {name: "complete with a result that does not parse", conds: []batchv1.JobCondition{complete}, log: ResultPrefix + "{\n", state: OpFailed, reason: "ResultUnavailable", finished: true}, + {name: "killed for memory without a result", + conds: []batchv1.JobCondition{backoff}, pod: killed(containerName, "OOMKilled"), log: progress, + state: OpFailed, reason: "OOMKilled", done: 40, finished: true}, + {name: "killed for memory after printing a clean result", + conds: []batchv1.JobCondition{backoff}, pod: killed(containerName, "OOMKilled"), log: ok, + state: OpSucceeded, files: 3, finished: true, wantResult: true}, + {name: "killed another way", + conds: []batchv1.JobCondition{backoff}, pod: killed(containerName, "Error"), + state: OpFailed, reason: "BackoffLimitExceeded", finished: true}, + {name: "another container killed for memory", + conds: []batchv1.JobCondition{backoff}, pod: killed("sidecar", "OOMKilled"), + state: OpFailed, reason: "BackoffLimitExceeded", finished: true}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - st := opState(asyncJob(tc.conds...), tc.log) + st := opState(asyncJob(tc.conds...), tc.pod, tc.log) if st.ID != "0a" || st.Op != OpUnzip || st.Path != "maps/world.zip" || !st.Started.Equal(opCreated) { t.Fatalf("identity = %q %q %q %v", st.ID, st.Op, st.Path, st.Started) } @@ -186,6 +205,28 @@ func TestK8sRunnerOps(t *testing.T) { } } +// TestK8sRunnerOpsReadsAMemoryKill checks Ops hands each Job's Pod to opState: a +// Pod the kernel killed for memory is what names an op's reason OOMKilled. +func TestK8sRunnerOpsReadsAMemoryKill(t *testing.T) { + p := testParams(OpUnzip) + p.Server, p.OpID, p.Path, p.Async = "survival", "oom", "maps/tiles.zip", true + j, err := FilesJob(p) + if err != nil { + t.Fatal(err) + } + j.Status.Conditions = []batchv1.JobCondition{cond(batchv1.JobFailed, corev1.ConditionTrue, "BackoffLimitExceeded")} + pod := &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: j.Name + "-x", Namespace: "minecraft", Labels: j.Spec.Template.Labels}, + Status: corev1.PodStatus{Phase: corev1.PodFailed, ContainerStatuses: []corev1.ContainerStatus{{ + Name: containerName, State: corev1.ContainerState{Terminated: &corev1.ContainerStateTerminated{ExitCode: 137, Reason: "OOMKilled"}}, + }}}, + } + ops, err := NewK8sRunner(fake.NewSimpleClientset(j, pod)).Ops(context.Background(), "minecraft", "survival") + if err != nil || len(ops) != 1 || ops[0].State != OpFailed || ops[0].Reason != "OOMKilled" { + t.Fatalf("Ops = %+v, %v; want the one op failed for OOMKilled", ops, err) + } +} + // TestK8sRunnerOpsFailsLoudly checks a listing the cluster refused is an error, // never an empty list that would read as nothing running. func TestK8sRunnerOpsFailsLoudly(t *testing.T) { diff --git a/internal/fileedit/manage_test.go b/internal/fileedit/manage_test.go index 6671d45..a3230b5 100644 --- a/internal/fileedit/manage_test.go +++ b/internal/fileedit/manage_test.go @@ -397,16 +397,18 @@ func TestRename(t *testing.T) { }) } -// fakeSource is an upload's bytes as the Job would fetch them. +// fakeSource is an upload's bytes as the Job would fetch them. landed counts +// the reports that they are in place. type fakeSource struct { body string opened int + landed int err error // returned by Open readErr error // returned by the body once it runs out } func (s *fakeSource) upload(size int64, sum string) *Upload { - return &Upload{Size: size, SHA256: sum, Open: func() (io.ReadCloser, error) { + return &Upload{Size: size, SHA256: sum, Landed: func() { s.landed++ }, Open: func() (io.ReadCloser, error) { s.opened++ if s.err != nil { return nil, s.err @@ -434,9 +436,17 @@ func TestUpload(t *testing.T) { t.Run("lands the bytes as a new file", func(t *testing.T) { root, _ := worldRoot(t) src := &fakeSource{body: jar} - res, err := send(t, root, "config/Geyser.jar", whole(src), false) - if err != nil || res.Code != "" { - t.Fatalf("result = %+v, %v", res, err) + u := whole(src) + // Reported once the file is in place, never before. + var there string + u.Landed = func() { + src.landed++ + b, _ := os.ReadFile(filepath.Join(root, "config", "Geyser.jar")) + there = string(b) + } + res, err := send(t, root, "config/Geyser.jar", u, false) + if err != nil || res.Code != "" || src.landed != 1 || there != jar { + t.Fatalf("result = %+v, %v; landed %d with %q in place", res, err, src.landed, there) } if got := mustRead(t, filepath.Join(root, "config", "Geyser.jar")); got != jar { t.Fatalf("content = %q", got) @@ -454,8 +464,8 @@ func TestUpload(t *testing.T) { for _, name := range []string{"server.properties", "dangling.jar"} { src := &fakeSource{body: jar} res, err := send(t, root, name, whole(src), false) - if err != nil || res.Code != CodeExists || src.opened != 0 { - t.Fatalf("%s: result = %+v, %v, opened %d; want exists and no fetch", name, res, err, src.opened) + if err != nil || res.Code != CodeExists || src.opened != 0 || src.landed != 0 { + t.Fatalf("%s: result = %+v, %v, opened %d, landed %d; want exists and no fetch", name, res, err, src.opened, src.landed) } } if got := mustRead(t, filepath.Join(root, "server.properties")); got != "motd=hello\n" { @@ -541,10 +551,11 @@ func TestUpload(t *testing.T) { } { t.Run(name, func(t *testing.T) { root, _ := worldRoot(t) - res, err := send(t, root, "server.properties", u(&fakeSource{body: jar}), true) + src := &fakeSource{body: jar} + res, err := send(t, root, "server.properties", u(src), true) var te *transferError - if !errors.As(err, &te) || res.Code != "" { - t.Fatalf("result = %+v, err = %v; want a transfer error", res, err) + if !errors.As(err, &te) || res.Code != "" || src.landed != 0 { + t.Fatalf("result = %+v, err = %v, landed %d; want a transfer error and no report", res, err, src.landed) } if got := mustRead(t, filepath.Join(root, "server.properties")); got != "motd=hello\n" { t.Fatalf("server.properties became %q", got) @@ -559,9 +570,11 @@ func TestUpload(t *testing.T) { prev := syncWritten syncWritten = func(*os.File) error { return syscall.ENOSPC } defer func() { syncWritten = prev }() - res, err := send(t, root, "server.properties", whole(&fakeSource{body: jar}), true) - if err != nil || res.Code != CodeNoSpace { - t.Fatalf("result = %+v, %v; want no_space", res, err) + src := &fakeSource{body: jar} + res, err := send(t, root, "server.properties", whole(src), true) + // The bytes were fetched but never landed, so felis-api keeps them. + if err != nil || res.Code != CodeNoSpace || src.opened != 1 || src.landed != 0 { + t.Fatalf("result = %+v, %v, opened %d, landed %d; want no_space after a fetch and no report", res, err, src.opened, src.landed) } if got := mustRead(t, filepath.Join(root, "server.properties")); got != "motd=hello\n" { t.Fatalf("server.properties became %q", got) diff --git a/internal/fileedit/session.go b/internal/fileedit/session.go index ff556e5..4d56281 100644 --- a/internal/fileedit/session.go +++ b/internal/fileedit/session.go @@ -4,11 +4,13 @@ import ( "crypto/sha256" "crypto/subtle" "encoding" + "encoding/hex" "errors" "fmt" "hash" "io" "os" + "slices" "time" ) @@ -23,11 +25,18 @@ import ( // Every call names the user and the server the session was begun for, and a // session answers no one else: an id that is someone else's reads as unknown. // -// A part that fails midway (the connection dropped, the edge cut it off) is -// rolled back to where it started, so the session's length is always the resume -// point. A sealed session stays until it has been served whole once (Served), so -// a Job that failed before it had every byte can be started again without the -// file being sent again; one left idle for SessionIdle is dropped (Expire). +// Each part carries the SHA-256 the client computed over it, and a part whose +// bytes hash to anything else was changed on the way and is refused. A part that +// fails midway (the connection dropped, the edge cut it off, the digest did not +// match) is rolled back to where it started, so the session's length is always +// the resume point. The session keeps each part's size and digest, so a client +// resuming with a file from disk can check the file still holds the bytes +// already sent before it sends the rest. +// +// A sealed session stays until the Job that fetched it says its file has landed +// (Landed), so a Job that failed at any point before that (a broken fetch, a +// full volume, a file in the way) can be started again without the file being +// sent again; one left idle for SessionIdle is dropped (Expire). // PartBytes is the largest part Append takes, matching the modpack upload's // parts (submit.DefaultPartMaxBytes). @@ -71,6 +80,15 @@ type Session struct { Path string Size int64 Received int64 + // Parts are the parts that make up Received, in order. + Parts []Part +} + +// Part is one part a session took: its length and the SHA-256 (lowercase hex) +// it arrived with and matched. +type Part struct { + Size int64 + SHA256 string } type session struct { @@ -78,11 +96,13 @@ type session struct { file string size, received int64 h hash.Hash + parts []Part busy bool touched time.Time // armed is set by Seal with the digest of the token it minted and cleared by - // the Open that spends it. + // the Open that spends it. tokenHash stays until the next Seal, so the Job + // holding that token can still report its file landed (Landed). armed bool tokenHash [sha256.Size]byte } @@ -150,7 +170,7 @@ func (s *Stage) lookup(user, server, id string) (*session, error) { } func (ss *session) view(id string) Session { - return Session{ID: id, Path: ss.path, Size: ss.size, Received: ss.received} + return Session{ID: id, Path: ss.path, Size: ss.size, Received: ss.received, Parts: slices.Clone(ss.parts)} } // Status reports where the caller's session stands. @@ -166,12 +186,16 @@ func (s *Stage) Status(user, server, id string) (Session, error) { // Append adds the n bytes of body at offset, which must be where the session // ends. body must end right after them (an HTTP body of that Content-Length -// does). On any failure the session is left as it was before the call. -func (s *Stage) Append(user, server, id string, offset int64, body io.Reader, n int64) (Session, error) { +// does), and they must hash to want, the SHA-256 the client computed over them +// (ErrDigestMismatch otherwise). On any failure the session is left as it was +// before the call. +func (s *Stage) Append(user, server, id string, offset int64, body io.Reader, n int64, want []byte) (Session, error) { s.mu.Lock() ss, err := s.lookup(user, server, id) switch { case err != nil: + case len(want) != sha256.Size: + err = ErrNoDigest case ss.busy: err = ErrUploadBusy case offset != ss.received: @@ -195,7 +219,11 @@ func (s *Stage) Append(user, server, id string, offset int64, body io.Reader, n // touched without the lock. The hash's state is kept to undo a failed part. before, err := ss.h.(encoding.BinaryMarshaler).MarshalBinary() if err == nil { - err = appendPart(ss.file, offset, body, n, ss.h) + part := sha256.New() + err = appendPart(ss.file, offset, body, n, io.MultiWriter(ss.h, part)) + if err == nil { + err = checkDigest(part.Sum(nil), want) + } if err != nil { _ = os.Truncate(ss.file, offset) _ = ss.h.(encoding.BinaryUnmarshaler).UnmarshalBinary(before) @@ -211,12 +239,13 @@ func (s *Stage) Append(user, server, id string, offset int64, body io.Reader, n } ss.received += n s.reserved -= n + ss.parts = append(ss.parts, Part{Size: n, SHA256: hex.EncodeToString(want)}) return ss.view(id), nil } // appendPart writes exactly n bytes of body at offset in the file named file, // feeding them to h as well. -func appendPart(file string, offset int64, body io.Reader, n int64, h hash.Hash) error { +func appendPart(file string, offset int64, body io.Reader, n int64, h io.Writer) error { f, err := os.OpenFile(file, os.O_WRONLY, 0) if err != nil { return fmt.Errorf("fileedit: open the staged upload: %w", err) @@ -269,19 +298,34 @@ func (s *Stage) openSession(id string, sum [sha256.Size]byte) (path string, size return ss.file, ss.size, true, nil } -// Served tells the stage the session id was sent whole to the Job that opened -// it, and deletes it: its bytes are on the Job's side now. An id that names no -// session (an upload staged by Put, which its own release deletes) is ignored. -func (s *Stage) Served(id string) { +// Landed tells the stage the Job holding token has put session id's file in +// place, and deletes the session: nothing will fetch it again. Only the token +// the latest Seal minted says so, spent or not, so a Job an earlier commit +// started, or anyone else, changes nothing (ErrNotStaged). An upload staged by +// Put answers to its own token too and is left to the release that deletes it. +func (s *Stage) Landed(id, token string) error { + sum := sha256.Sum256([]byte(token)) s.mu.Lock() ss, ok := s.sessions[id] - if ok { - delete(s.sessions, id) + if !ok { + it, found := s.items[id] + s.mu.Unlock() + if !found || subtle.ConstantTimeCompare(sum[:], it.tokenHash[:]) != 1 { + return ErrNotStaged + } + return nil } + // Before the first Seal tokenHash is zero, which no token hashes to. A sealed + // session has every byte, so a part arriving now could only be an empty one + // and leaves nothing to account for: no busy check, unlike Drop. + if subtle.ConstantTimeCompare(sum[:], ss.tokenHash[:]) != 1 { + s.mu.Unlock() + return ErrNotStaged + } + s.dropLocked(id, ss) s.mu.Unlock() - if ok { - os.Remove(ss.file) - } + os.Remove(ss.file) + return nil } // Drop cancels the caller's session and deletes what it holds. diff --git a/internal/fileedit/session_test.go b/internal/fileedit/session_test.go index 7fbab0a..4df940d 100644 --- a/internal/fileedit/session_test.go +++ b/internal/fileedit/session_test.go @@ -5,6 +5,7 @@ import ( "io" "os" "path/filepath" + "reflect" "strings" "testing" "time" @@ -31,7 +32,7 @@ func diskStage(t *testing.T, free, total uint64, minFree float64) *Stage { } func appendString(s *Stage, user, server, id string, offset int64, part string) (Session, error) { - return s.Append(user, server, id, offset, strings.NewReader(part), int64(len(part))) + return s.Append(user, server, id, offset, strings.NewReader(part), int64(len(part)), sumOf(part)) } func readStaged(t *testing.T, s *Stage, id, token string) string { @@ -51,8 +52,9 @@ func readStaged(t *testing.T, s *Stage, id, token string) string { return string(b) } -// TestSessionArrivesInParts: parts land in order, Seal hands the Job a token for -// exactly those bytes, and Served deletes them. +// TestSessionArrivesInParts: parts land in order and are listed with their +// digests, Seal hands the Job a token for exactly those bytes, and they stay +// until that Job reports them landed. func TestSessionArrivesInParts(t *testing.T) { s := roomyStage(t) const whole = "PK\x03\x04 first part, second part" @@ -60,19 +62,24 @@ func TestSessionArrivesInParts(t *testing.T) { if err != nil { t.Fatalf("Begin: %v", err) } - if !hexID.MatchString(sess.ID) || sess != (Session{ID: sess.ID, Path: "plugins/big.jar", Size: int64(len(whole))}) { + if !hexID.MatchString(sess.ID) || !reflect.DeepEqual(sess, Session{ID: sess.ID, Path: "plugins/big.jar", Size: int64(len(whole))}) { t.Fatalf("Begin = %+v", sess) } got, err := appendString(s, "u1", "survival", sess.ID, 0, whole[:16]) if err != nil || got.Received != 16 || got.Size != int64(len(whole)) { t.Fatalf("first part: %+v, %v", got, err) } - if at, err := s.Status("u1", "survival", sess.ID); err != nil || at.Received != 16 || at.Path != "plugins/big.jar" { + first := Part{Size: 16, SHA256: digest([]byte(whole[:16]))} + if at, err := s.Status("u1", "survival", sess.ID); err != nil || at.Received != 16 || at.Path != "plugins/big.jar" || + !reflect.DeepEqual(at.Parts, []Part{first}) { t.Fatalf("Status = %+v, %v", at, err) } if got, err = appendString(s, "u1", "survival", sess.ID, 16, whole[16:]); err != nil || got.Received != int64(len(whole)) { t.Fatalf("second part: %+v, %v", got, err) } + if want := []Part{first, {Size: int64(len(whole) - 16), SHA256: digest([]byte(whole[16:]))}}; !reflect.DeepEqual(got.Parts, want) { + t.Fatalf("parts = %+v, want %+v", got.Parts, want) + } st, err := s.Seal("u1", "survival", sess.ID) if err != nil { @@ -88,12 +95,28 @@ func TestSessionArrivesInParts(t *testing.T) { t.Fatalf("second Open with the same token: err = %v, want ErrNotStaged", err) } - s.Served(st.ID) + // Served whole, and still here: the Job has yet to check the bytes and put + // them in place, and a Job that fails at either is started again on them. + if at, err := s.Status("u1", "survival", sess.ID); err != nil || at.Received != int64(len(whole)) { + t.Fatalf("Status once served: %+v, %v", at, err) + } + if err := s.Landed(st.ID, strings.Repeat("0", 64)); !errors.Is(err, ErrNotStaged) { + t.Fatalf("Landed with a wrong token: err = %v, want ErrNotStaged", err) + } + if names := stagedNames(t, s); len(names) != 1 { + t.Fatalf("after a wrong token: %v", names) + } + if err := s.Landed(st.ID, st.Token); err != nil { + t.Fatalf("Landed: %v", err) + } if names := stagedNames(t, s); len(names) != 0 { - t.Fatalf("after Served: %v", names) + t.Fatalf("after Landed: %v", names) } if _, err := s.Status("u1", "survival", sess.ID); !errors.Is(err, ErrNotStaged) { - t.Fatalf("Status after Served: err = %v, want ErrNotStaged", err) + t.Fatalf("Status after Landed: err = %v, want ErrNotStaged", err) + } + if err := s.Landed(st.ID, st.Token); !errors.Is(err, ErrNotStaged) { + t.Fatalf("Landed twice: err = %v, want ErrNotStaged", err) } } @@ -149,7 +172,7 @@ func TestSessionRefusesAPartThatDoesNotFit(t *testing.T) { if _, err := appendString(s, "u1", "survival", sess.ID, 3, "defg"); !errors.Is(err, ErrPartTooLarge) { t.Fatalf("past the declared size: err = %v, want ErrPartTooLarge", err) } - if _, err := s.Append("u1", "survival", sess.ID, 3, strings.NewReader(""), -1); !errors.Is(err, ErrPartTooLarge) { + if _, err := s.Append("u1", "survival", sess.ID, 3, strings.NewReader(""), -1, sumOf("")); !errors.Is(err, ErrPartTooLarge) { t.Fatalf("negative length: err = %v, want ErrPartTooLarge", err) } if at, err := appendString(s, "u1", "survival", sess.ID, 3, "def"); err != nil || at.Received != 6 { @@ -160,21 +183,24 @@ func TestSessionRefusesAPartThatDoesNotFit(t *testing.T) { if err != nil { t.Fatal(err) } - if _, err := s.Append("u1", "survival", big.ID, 0, strings.NewReader(""), PartBytes+1); !errors.Is(err, ErrPartTooLarge) { + if _, err := s.Append("u1", "survival", big.ID, 0, strings.NewReader(""), PartBytes+1, sumOf("")); !errors.Is(err, ErrPartTooLarge) { t.Fatalf("a part over PartBytes: err = %v, want ErrPartTooLarge", err) } } -// A part that breaks or runs long leaves the session as it was: the file is cut -// back and the digest forgets it, so the resent part makes the right file. +// A part that breaks, runs long or does not match its digest leaves the session +// as it was: the file is cut back and the digest forgets it, so the resent part +// makes the right file. func TestSessionRollsBackAFailedPart(t *testing.T) { for name, tc := range map[string]struct { - body io.Reader - short bool + body io.Reader + want error // nil: any other failure }{ - "breaks": {io.MultiReader(strings.NewReader("XY"), errReader{io.ErrUnexpectedEOF}), true}, - "ends": {strings.NewReader("XY"), true}, - "runs long": {strings.NewReader("XYZWV"), false}, + "breaks": {io.MultiReader(strings.NewReader("XY"), errReader{io.ErrUnexpectedEOF}), ErrShortUpload}, + "ends": {strings.NewReader("XY"), ErrShortUpload}, + "runs long": {strings.NewReader("XYZWV"), nil}, + // Four bytes, as declared, that are not the four the digest was made of. + "changed on the way": {strings.NewReader("dXfg"), ErrDigestMismatch}, } { t.Run(name, func(t *testing.T) { s := roomyStage(t) @@ -185,9 +211,13 @@ func TestSessionRollsBackAFailedPart(t *testing.T) { if _, err := appendString(s, "u1", "survival", sess.ID, 0, "abc"); err != nil { t.Fatal(err) } - at, err := s.Append("u1", "survival", sess.ID, 3, tc.body, 4) - if err == nil || errors.Is(err, ErrShortUpload) != tc.short || at.Received != 3 { - t.Fatalf("%+v, err = %v; want a failure at 3 (short = %v)", at, err, tc.short) + at, err := s.Append("u1", "survival", sess.ID, 3, tc.body, 4, sumOf("defg")) + kind := tc.want + if kind == nil { + kind = ErrShortUpload // must not be it + } + if err == nil || errors.Is(err, kind) != (tc.want != nil) || at.Received != 3 || len(at.Parts) != 1 { + t.Fatalf("%+v, err = %v; want a failure at 3 (%v)", at, err, tc.want) } info, err := os.Stat(filepath.Join(s.Dir, stagedNames(t, s)[0])) if err != nil || info.Size() != 3 { @@ -220,7 +250,7 @@ func TestSessionIsBusyWhileAPartArrives(t *testing.T) { pr, pw := io.Pipe() done := make(chan error, 1) go func() { - _, err := s.Append("u1", "survival", sess.ID, 0, pr, 4) + _, err := s.Append("u1", "survival", sess.ID, 0, pr, 4, sumOf("abcd")) done <- err }() // The write returns once Append is copying, which is after it marked busy. @@ -309,16 +339,61 @@ func TestSessionSealArmsOneFetch(t *testing.T) { } } -// Served deletes only sessions: an upload staged by Put belongs to the release -// func Put returned. -func TestServedLeavesPutAlone(t *testing.T) { +// Only the token the latest Seal minted reports a session landed: a Job an +// earlier commit started changes nothing, and neither does anyone before the +// first Seal. +func TestLandedTakesTheLatestToken(t *testing.T) { s := roomyStage(t) - st, release, err := s.Put(strings.NewReader("abc"), 3) + sess, err := s.Begin("u1", "survival", "a.zip", 4) + if err != nil { + t.Fatal(err) + } + if _, err := appendString(s, "u1", "survival", sess.ID, 0, "abcd"); err != nil { + t.Fatal(err) + } + if err := s.Landed(sess.ID, ""); !errors.Is(err, ErrNotStaged) { + t.Fatalf("Landed before any Seal: err = %v, want ErrNotStaged", err) + } + first, err := s.Seal("u1", "survival", sess.ID) + if err != nil { + t.Fatal(err) + } + readStaged(t, s, sess.ID, first.Token) + second, err := s.Seal("u1", "survival", sess.ID) + if err != nil { + t.Fatal(err) + } + if err := s.Landed(sess.ID, first.Token); !errors.Is(err, ErrNotStaged) { + t.Fatalf("the replaced token: err = %v, want ErrNotStaged", err) + } + if _, err := s.Status("u1", "survival", sess.ID); err != nil { + t.Fatalf("after the replaced token: %v", err) + } + // Not yet fetched with it, and it still says so: the Job holds the token + // whatever became of its fetch. + if err := s.Landed(sess.ID, second.Token); err != nil { + t.Fatalf("the latest token: %v", err) + } + if names := stagedNames(t, s); len(names) != 0 { + t.Fatalf("after Landed: %v", names) + } +} + +// An upload staged by Put answers Landed to its own token and is left to the +// release func Put returned. +func TestLandedLeavesPutAlone(t *testing.T) { + s := roomyStage(t) + st, release, err := s.Put(strings.NewReader("abc"), 3, sumOf("abc")) if err != nil { t.Fatal(err) } defer release() - s.Served(st.ID) + if err := s.Landed(st.ID, strings.Repeat("0", 64)); !errors.Is(err, ErrNotStaged) { + t.Fatalf("a wrong token: err = %v, want ErrNotStaged", err) + } + if err := s.Landed(st.ID, st.Token); err != nil { + t.Fatalf("Landed: %v", err) + } if body := readStaged(t, s, st.ID, st.Token); body != "abc" { t.Fatalf("served %q", body) } diff --git a/internal/fileedit/stage.go b/internal/fileedit/stage.go index 8f4fcbe..f904fe3 100644 --- a/internal/fileedit/stage.go +++ b/internal/fileedit/stage.go @@ -79,8 +79,22 @@ var ( // ErrNotStaged is an Open with an unknown id, a wrong token, or a spent one. // They are one error on purpose: the internal face answers all three the same. ErrNotStaged = errors.New("fileedit: no such staged upload") + // ErrNoDigest is bytes sent without the SHA-256 the client computed over + // them, so what arrived cannot be told apart from what was sent. + ErrNoDigest = errors.New("fileedit: the upload carries no SHA-256 digest") + // ErrDigestMismatch is bytes that do not hash to the digest they were sent + // with: they were changed on the way. + ErrDigestMismatch = errors.New("fileedit: the bytes that arrived do not match the digest they were sent with") ) +// checkDigest compares the digest of what arrived with the one it was sent with. +func checkDigest(got, want []byte) error { + if subtle.ConstantTimeCompare(got, want) != 1 { + return fmt.Errorf("%w: they hash to %x, sent as %x", ErrDigestMismatch, got, want) + } + return nil +} + // statfs reports a filesystem's available and total bytes. A var so a test can // stage against a disk of a chosen size. var statfs = func(dir string) (avail, total uint64, err error) { @@ -104,10 +118,17 @@ func (s *Stage) Sweep() error { // it by, plus the func that deletes it. body must end right after size bytes (an // HTTP body with that Content-Length does): Put reads to its end, which is also // what tells the server the body is done. -func (s *Stage) Put(body io.Reader, size int64) (Staged, func(), error) { +// +// want is the SHA-256 the client computed over the bytes it sent (the request's +// Content-Digest). Bytes that hash to anything else were changed on the way and +// are refused with ErrDigestMismatch; nothing is staged without one. +func (s *Stage) Put(body io.Reader, size int64, want []byte) (Staged, func(), error) { if size < 0 { return Staged{}, nil, fmt.Errorf("fileedit: an upload of %d bytes", size) } + if len(want) != sha256.Size { + return Staged{}, nil, ErrNoDigest + } if err := os.MkdirAll(s.Dir, 0o700); err != nil { return Staged{}, nil, fmt.Errorf("fileedit: create the upload stage: %w", err) } @@ -125,7 +146,11 @@ func (s *Stage) Put(body io.Reader, size int64) (Staged, func(), error) { // One byte past size, so the read that finds the end happens here. n, copyErr := io.Copy(io.MultiWriter(f, h), io.LimitReader(src, size+1)) closeErr := f.Close() - if err := stageFailure(src.err, copyErr, closeErr, n, size); err != nil { + err = stageFailure(src.err, copyErr, closeErr, n, size) + if err == nil { + err = checkDigest(h.Sum(nil), want) + } + if err != nil { os.Remove(f.Name()) return Staged{}, nil, err } diff --git a/internal/fileedit/stage_test.go b/internal/fileedit/stage_test.go index 74d29c3..48c284f 100644 --- a/internal/fileedit/stage_test.go +++ b/internal/fileedit/stage_test.go @@ -1,6 +1,7 @@ package fileedit import ( + "crypto/sha256" "errors" "io" "os" @@ -39,6 +40,12 @@ func stagedNames(t *testing.T, s *Stage) []string { return names } +// sumOf is the SHA-256 a client sends with s. +func sumOf(s string) []byte { + sum := sha256.Sum256([]byte(s)) + return sum[:] +} + var hexID = regexp.MustCompile(`^[0-9a-f]{32}$`) var hexToken = regexp.MustCompile(`^[0-9a-f]{64}$`) @@ -47,7 +54,7 @@ var hexToken = regexp.MustCompile(`^[0-9a-f]{64}$`) func TestStageOpensOnce(t *testing.T) { s := roomyStage(t) const body = "PK\x03\x04 staged" - st, release, err := s.Put(strings.NewReader(body), int64(len(body))) + st, release, err := s.Put(strings.NewReader(body), int64(len(body)), sumOf(body)) if err != nil { t.Fatalf("Put: %v", err) } @@ -55,7 +62,7 @@ func TestStageOpensOnce(t *testing.T) { st.Size != int64(len(body)) || st.SHA256 != digest([]byte(body)) { t.Fatalf("staged = %+v", st) } - other, releaseOther, err := s.Put(strings.NewReader(body), int64(len(body))) + other, releaseOther, err := s.Put(strings.NewReader(body), int64(len(body)), sumOf(body)) if err != nil { t.Fatalf("Put: %v", err) } @@ -104,7 +111,7 @@ func TestStageOpensOnce(t *testing.T) { // is readable by anyone but felis-api's own uid. func TestStageIsPrivate(t *testing.T) { s := roomyStage(t) - _, release, err := s.Put(strings.NewReader("x"), 1) + _, release, err := s.Put(strings.NewReader("x"), 1, sumOf("x")) if err != nil { t.Fatalf("Put: %v", err) } @@ -120,13 +127,14 @@ func TestStageIsPrivate(t *testing.T) { } } -// eofReader serves body and records whether it was read to its end. +// eofReader serves r and records whether it was read at all, and to its end. type eofReader struct { - r io.Reader - hitEOF bool + r io.Reader + read, hitEOF bool } func (e *eofReader) Read(p []byte) (int, error) { + e.read = true n, err := e.r.Read(p) if err == io.EOF { e.hitEOF = true @@ -140,7 +148,7 @@ func (e *eofReader) Read(p []byte) (int, error) { func TestStagePutReadsToTheEnd(t *testing.T) { s := roomyStage(t) body := &eofReader{r: strings.NewReader("abc")} - _, release, err := s.Put(body, 3) + _, release, err := s.Put(body, 3, sumOf("abc")) if err != nil { t.Fatalf("Put: %v", err) } @@ -150,6 +158,37 @@ func TestStagePutReadsToTheEnd(t *testing.T) { } } +// Bytes that do not hash to the digest they came with were changed on the way: +// nothing is staged and their room is given back. Without a digest the body is +// not read at all. +func TestStageChecksTheDigest(t *testing.T) { + stubStatfs(t, 1000, 1200) // floor 600 at MinFree 0.5: room for 400 + s := &Stage{Dir: t.TempDir(), MinFree: 0.5} + body := strings.Repeat("x", 400) + _, _, err := s.Put(strings.NewReader(body), 400, sumOf(strings.Repeat("y", 400))) + if !errors.Is(err, ErrDigestMismatch) { + t.Fatalf("a wrong digest: err = %v, want ErrDigestMismatch", err) + } + if names := stagedNames(t, s); len(names) != 0 { + t.Fatalf("left behind: %v", names) + } + st, release, err := s.Put(strings.NewReader(body), 400, sumOf(body)) + if err != nil { + t.Fatalf("the same bytes with their digest, in the room the refused ones held: %v", err) + } + defer release() + if st.SHA256 != digest([]byte(body)) { + t.Fatalf("staged %+v", st) + } + + for name, want := range map[string][]byte{"none": nil, "too short": sumOf("x")[:31]} { + unread := &eofReader{r: strings.NewReader("abc")} + if _, _, err := roomyStage(t).Put(unread, 3, want); !errors.Is(err, ErrNoDigest) || unread.read { + t.Errorf("%s: err = %v, body read = %v; want ErrNoDigest before any read", name, err, unread.read) + } + } +} + func TestStageRefusesABodyOfTheWrongLength(t *testing.T) { for name, tc := range map[string]struct { body io.Reader @@ -164,7 +203,7 @@ func TestStageRefusesABodyOfTheWrongLength(t *testing.T) { } { t.Run(name, func(t *testing.T) { s := roomyStage(t) - _, release, err := s.Put(tc.body, tc.size) + _, release, err := s.Put(tc.body, tc.size, sumOf("")) if err == nil { release() t.Fatal("Put accepted it") @@ -177,7 +216,7 @@ func TestStageRefusesABodyOfTheWrongLength(t *testing.T) { } }) } - if _, _, err := roomyStage(t).Put(strings.NewReader(""), -1); err == nil { + if _, _, err := roomyStage(t).Put(strings.NewReader(""), -1, sumOf("")); err == nil { t.Fatal("a negative size was accepted") } } @@ -186,7 +225,7 @@ func TestStageRefusesABodyOfTheWrongLength(t *testing.T) { // MinFree of the disk free, counting uploads still arriving. func TestStageKeepsItsFloor(t *testing.T) { put := func(s *Stage, size int64) error { - _, release, err := s.Put(strings.NewReader(strings.Repeat("x", int(size))), size) + _, release, err := s.Put(strings.NewReader(strings.Repeat("x", int(size))), size, sumOf(strings.Repeat("x", int(size)))) if err == nil { release() } @@ -231,7 +270,7 @@ func TestStageKeepsItsFloor(t *testing.T) { pr, pw := io.Pipe() done := make(chan error, 1) go func() { - _, release, err := s.Put(pr, 300) + _, release, err := s.Put(pr, 300, sumOf(strings.Repeat("x", 300))) if err == nil { release() } @@ -271,7 +310,7 @@ func TestStageSweep(t *testing.T) { if names := stagedNames(t, s); len(names) != 0 { t.Fatalf("after Sweep: %v", names) } - if _, release, err := s.Put(strings.NewReader("x"), 1); err != nil { + if _, release, err := s.Put(strings.NewReader("x"), 1, sumOf("x")); err != nil { t.Fatalf("Put after Sweep: %v", err) } else { release() diff --git a/internal/fileedit/unzip.go b/internal/fileedit/unzip.go index a1dc0ba..3aac3aa 100644 --- a/internal/fileedit/unzip.go +++ b/internal/fileedit/unzip.go @@ -40,8 +40,16 @@ const ( // MaxConflicts bounds Result.Conflicts, keeping the result line bounded when an // archive would replace a whole world; ConflictCount still says how many there -// are. -const MaxConflicts = 200 +// are. maxConflictBytes bounds the names listed as well: felis-api reads an +// unzip's result from the last opLogLines lines of its Pod's log, and the +// container runtime splits a line longer than 16 KiB into several, so 200 long +// paths would push the result marker out of that tail and the op would read as +// ended without a result. A file that exists has a path under PATH_MAX (4 KiB), +// so the first conflict always fits. +const ( + MaxConflicts = 200 + maxConflictBytes = 8 << 10 +) // unzipEntryOverhead is what the space check adds per entry for the inode and // directory block it takes beyond its bytes. @@ -113,10 +121,12 @@ func unzip(r *os.Root, rootPath, name string, overwrite bool, progress func(done list = append(list, path.Join(dest, n)) } sort.Strings(list) - count := len(list) - if count > MaxConflicts { - list = list[:MaxConflicts] + count, n, size := len(list), 0, 0 + for n < count && n < MaxConflicts && size+len(list[n]) <= maxConflictBytes { + size += len(list[n]) + n++ } + list = list[:n] return Result{Code: CodeExists, Conflicts: list, ConflictCount: count, Error: fmt.Sprintf( "%d files in the archive already exist on the server; extract again with overwrite to replace them", count)} } @@ -180,7 +190,6 @@ type zipFile struct { // makes. Nothing about the server is consulted yet. func planUnzip(entries []*zip.File) (unzipPlan, Result) { p := unzipPlan{isDir: map[string]bool{}} - byName := map[string]bool{} for _, f := range entries { raw := entryName(f) name, ok := cleanEntry(raw) @@ -210,14 +219,10 @@ func planUnzip(entries []*zip.File) (unzipPlan, Result) { case name == ".": return p, Result{Code: CodeArchiveUnsafe, Entry: raw, Error: fmt.Sprintf( "%s names the destination folder itself", raw)} - case byName[name]: - return p, Result{Code: CodeArchiveInvalid, Entry: raw, Error: fmt.Sprintf( - "%s appears in the archive twice", raw)} case f.UncompressedSize64 > uint64(math.MaxInt64-p.bytes): return p, Result{Code: CodeArchiveInvalid, Entry: raw, Error: fmt.Sprintf( "%s declares an impossible size", raw)} } - byName[name] = true p.bytes += int64(f.UncompressedSize64) // Anything the archive marks executable (a start.sh) stays executable; // every other permission is the server's usual. @@ -227,6 +232,15 @@ func planUnzip(entries []*zip.File) (unzipPlan, Result) { } p.files = append(p.files, zipFile{f: f, name: name, mode: perm}) } + // Sorted, a name the archive holds twice sits next to itself; a set of names + // would cost as much again as the entries for an archive of many small files. + sort.Slice(p.files, func(i, j int) bool { return p.files[i].name < p.files[j].name }) + for i := 1; i < len(p.files); i++ { + if p.files[i].name == p.files[i-1].name { + return p, Result{Code: CodeArchiveInvalid, Entry: p.files[i].name, Error: fmt.Sprintf( + "%s appears in the archive twice", p.files[i].name)} + } + } for _, zf := range p.files { // cleanEntry already refused a rooted name; stopping at "/" as well keeps // this loop finite should that check ever move. @@ -248,7 +262,6 @@ func planUnzip(entries []*zip.File) (unzipPlan, Result) { } // A folder's name is a prefix of everything in it, and a prefix sorts first. sort.Strings(p.dirs) - sort.Slice(p.files, func(i, j int) bool { return p.files[i].name < p.files[j].name }) return p, Result{} } @@ -410,12 +423,19 @@ func extractOne(r *os.Root, at string, zf zipFile, count func(int)) Result { // whole; one it has is descended into. A file it has is first moved aside into // old, so undoing the journal puts it back. func placeAll(r *os.Root, dest, staged, old string, p unzipPlan, present, replaced map[string]bool) Result { + // Only the destination and the folders the server has are descended into; + // everything else moves with the folder it is in, so its name is not kept. kids := map[string][]string{} + add := func(name string) { + if parent := path.Dir(name); parent == "." || present[parent] { + kids[parent] = append(kids[parent], name) + } + } for _, d := range p.dirs { - kids[path.Dir(d)] = append(kids[path.Dir(d)], d) + add(d) } for _, zf := range p.files { - kids[path.Dir(zf.name)] = append(kids[path.Dir(zf.name)], zf.name) + add(zf.name) } type move struct{ from, to string } diff --git a/internal/fileedit/unzip_test.go b/internal/fileedit/unzip_test.go index 5599cb1..2bd12dc 100644 --- a/internal/fileedit/unzip_test.go +++ b/internal/fileedit/unzip_test.go @@ -3,6 +3,7 @@ package fileedit import ( "archive/zip" "bytes" + "encoding/json" "errors" "fmt" "hash/crc32" @@ -369,6 +370,28 @@ func TestUnzip(t *testing.T) { } }) + // The result comes back through the tail of the Pod's log, where a line past + // 16 KiB is split and its head can fall out of the tail. + t.Run("the list stops at 8 KiB of names too, keeping the result line whole", func(t *testing.T) { + root, _ := worldRoot(t) + var entries []zent + for i := range 40 { + name := fmt.Sprintf("%02d", i) + strings.Repeat("n", 248) // 250 bytes + if err := os.WriteFile(filepath.Join(root, name), []byte("old"), 0o644); err != nil { + t.Fatal(err) + } + entries = append(entries, file(name, "new")) + } + writeZip(t, filepath.Join(root, "long.zip"), entries...) + res := unzipAt(t, root, "long.zip", false) + line, _ := json.Marshal(res) + // 32 names are 8000 bytes; a 33rd would pass 8192. + if res.Code != CodeExists || res.ConflictCount != 40 || len(res.Conflicts) != 32 || + !strings.HasPrefix(res.Conflicts[31], "31n") || len(line) >= 16<<10 { + t.Fatalf("code %q, count %d, %d listed, result line %d bytes", res.Code, res.ConflictCount, len(res.Conflicts), len(line)) + } + }) + t.Run("overwrite replaces files, merges folders and keeps everything else", func(t *testing.T) { root, _ := worldRoot(t) if err := os.WriteFile(filepath.Join(root, "config", "keep.yml"), []byte("keep"), 0o644); err != nil { diff --git a/internal/submit/digest.go b/internal/submit/digest.go new file mode 100644 index 0000000..1f08673 --- /dev/null +++ b/internal/submit/digest.go @@ -0,0 +1,41 @@ +package submit + +import ( + "bytes" + "crypto/sha256" + "errors" + "fmt" + "hash" + "io" +) + +// ErrDigestMismatch reports bytes that do not hash to the SHA-256 their sender +// computed over them (the request's Content-Digest): they changed on the way. +// The API answers 400 digest_mismatch, and the client sends them again. +var ErrDigestMismatch = errors.New("submit: the bytes that arrived do not match the digest they were sent with") + +// VerifyDigest passes r through and, where it ends, answers ErrDigestMismatch in +// place of io.EOF unless what went by hashes to want, a SHA-256. The stores +// treat that like any read that breaks off: UploadPart cuts the part back off, +// and UploadContext never lets the blob replace the one before it. So bytes +// changed on the way are never kept. +func VerifyDigest(r io.Reader, want []byte) io.Reader { + return &digestReader{r: r, h: sha256.New(), want: want} +} + +type digestReader struct { + r io.Reader + h hash.Hash + want []byte +} + +func (d *digestReader) Read(p []byte) (int, error) { + n, err := d.r.Read(p) + d.h.Write(p[:n]) + if err == io.EOF { + if got := d.h.Sum(nil); !bytes.Equal(got, d.want) { + return n, fmt.Errorf("%w: they hash to %x, sent as %x", ErrDigestMismatch, got, d.want) + } + } + return n, err +} diff --git a/internal/submit/digest_test.go b/internal/submit/digest_test.go new file mode 100644 index 0000000..f191de0 --- /dev/null +++ b/internal/submit/digest_test.go @@ -0,0 +1,126 @@ +package submit + +import ( + "bytes" + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "io" + "math/rand" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +// sumOf is the SHA-256 a client sends with body. +func sumOf(body string) []byte { + s := sha256.Sum256([]byte(body)) + return s[:] +} + +func TestVerifyDigest(t *testing.T) { + t.Run("bytes that hash to the digest end cleanly", func(t *testing.T) { + b, err := io.ReadAll(VerifyDigest(strings.NewReader("modpack"), sumOf("modpack"))) + if err != nil || string(b) != "modpack" { + t.Fatalf("ReadAll = (%q, %v), want (modpack, nil)", b, err) + } + }) + t.Run("bytes that do not end in ErrDigestMismatch", func(t *testing.T) { + b, err := io.ReadAll(VerifyDigest(strings.NewReader("modpacX"), sumOf("modpack"))) + if !errors.Is(err, ErrDigestMismatch) { + t.Fatalf("err = %v, want ErrDigestMismatch", err) + } + if string(b) != "modpacX" { + t.Fatalf("passed on %q, want every byte (the store decides what to keep)", b) + } + }) + t.Run("a read that breaks off is passed on as it is", func(t *testing.T) { + _, err := io.ReadAll(VerifyDigest(&failingReader{data: "mod"}, sumOf("mod"))) + if err == nil || errors.Is(err, ErrDigestMismatch) || err.Error() != "connection reset" { + t.Fatalf("err = %v, want the connection reset itself", err) + } + }) +} + +// A part that changed on the way is cut back off like one that broke off, below +// the part cap and exactly at it (where the cap reads one byte past the part). +func TestChunkedUploadCutsBackAPartChangedOnTheWay(t *testing.T) { + for _, tc := range []struct{ name, sent, meant string }{ + {"under the part cap", "abX", "abc"}, + {"at the part cap", "abcX", "abcd"}, + } { + t.Run(tc.name, func(t *testing.T) { + m, _, _, id := newChunkedManager(t) + sendPart(t, m, id, 0, "\x1f\x8b\x08\x00") + + _, err := m.UploadPart(context.Background(), id, "user-1", 4, VerifyDigest(strings.NewReader(tc.sent), sumOf(tc.meant))) + if !errors.Is(err, ErrDigestMismatch) { + t.Fatalf("err = %v, want ErrDigestMismatch", err) + } + if got := staged(t, m, id); got != 4 { + t.Fatalf("staged after a changed part = %d, want 4", got) + } + p, err := m.UploadPart(context.Background(), id, "user-1", 4, VerifyDigest(strings.NewReader(tc.meant), sumOf(tc.meant))) + if err != nil || p.Received != int64(4+len(tc.meant)) { + t.Fatalf("the part sent again = (%d, %v), want (%d, nil)", p.Received, err, 4+len(tc.meant)) + } + }) + } +} + +// A context that changed on the way leaves the one before it stored, and its +// digest recorded. +func TestUploadContextChangedOnTheWayKeepsThePreviousContext(t *testing.T) { + m, st, _ := newManager() + base := t.TempDir() + m.Blobs = &LocalContextStore{Base: base, MinFree: 1e-9} + ctx := context.Background() + seed, _ := m.Create(ctx, CreateRequest{DisplayName: "Pack", SubmittedBy: "user-1"}) + first, second := gzBody("first"), gzBody("second") + if _, err := m.UploadContext(ctx, seed.ID, "user-1", VerifyDigest(strings.NewReader(first), sumOf(first))); err != nil { + t.Fatalf("first upload: %v", err) + } + + _, err := m.UploadContext(ctx, seed.ID, "user-1", VerifyDigest(strings.NewReader(second+"X"), sumOf(second))) + + if !errors.Is(err, ErrDigestMismatch) { + t.Fatalf("err = %v, want ErrDigestMismatch", err) + } + if got, _ := os.ReadFile(filepath.Join(base, seed.ID, contextBlobName)); string(got) != first { + t.Fatalf("stored %q, want the first upload %q", got, first) + } + if got := st.subs[seed.ID].ContextSHA256; got != hex.EncodeToString(sumOf(first)) { + t.Fatalf("recorded digest %s, want the first upload's", got) + } +} + +// Through minio-go, a mismatch found at the end of the stream aborts the +// multipart upload, whether the last part is short or the stream ends right at a +// part boundary. +func TestS3ContextStorePutKeepsNothingChangedOnTheWay(t *testing.T) { + for _, size := range []int{2*s3PartSize + 5, 2 * s3PartSize} { + fake := &multipartS3{objects: map[string]string{}} + ts := httptest.NewServer(fake) + s, err := NewS3ContextStore(S3StoreConfig{Base: "s3://felis-uploads/builds", Endpoint: ts.URL, Region: "us-east-1", AccessKey: "a", SecretKey: "b"}) + if err != nil { + t.Fatal(err) + } + payload := make([]byte, size) + rand.New(rand.NewSource(2)).Read(payload) + meant := sumOf(string(payload)) + payload[size-1] ^= 1 + + _, err = s.Put(context.Background(), "sub-big", VerifyDigest(bytes.NewReader(payload), meant)) + ts.Close() + + if !errors.Is(err, ErrDigestMismatch) { + t.Errorf("%d bytes: err = %v, want ErrDigestMismatch", size, err) + } + if len(fake.objects) != 0 { + t.Errorf("%d bytes: an object was completed: %v", size, fake.objects) + } + } +} diff --git a/internal/worldexport/jobspec.go b/internal/worldexport/jobspec.go index 8ebb2ec..f4fef3b 100644 --- a/internal/worldexport/jobspec.go +++ b/internal/worldexport/jobspec.go @@ -43,6 +43,17 @@ const ( // process listing on the node would show it. const TokenEnv = "FELIS_EXPORT_TOKEN" +// The Job's PUT goes chunked, so that it can end with a trailer: DigestTrailer +// carries the SHA-256 of every byte it sent (sha-256=::, RFC 9530), +// which felis-api checks before the last of them reaches the browser. +// LengthHeader declares the length up front when the Job knows it (one file), +// which chunked encoding cannot carry, so the browser still gets a +// Content-Length. +const ( + DigestTrailer = "Content-Digest" + LengthHeader = "X-Felis-Export-Length" +) + // JobParams are the rendered inputs to an export Job. ExportJob is a pure // function of them, so the Job shape is unit-tested without a cluster. type JobParams struct { diff --git a/internal/worldexport/jobspec_test.go b/internal/worldexport/jobspec_test.go index a8e9f4e..775026c 100644 --- a/internal/worldexport/jobspec_test.go +++ b/internal/worldexport/jobspec_test.go @@ -8,8 +8,11 @@ import ( "time" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/kubernetes/fake" + k8stesting "k8s.io/client-go/testing" ) const secretToken = "5ecret5ecret5ecret5ecret5ecret5ecret5ecret5ecret5ecret5ecret5ecr" @@ -236,3 +239,34 @@ func TestStartCreatesTheJob(t *testing.T) { } } } + +// A Job felis-api gave up on goes with its Pods, and one already gone is no +// error: the sweep that stops it may run after the Job's own TTL took it. +func TestStopDeletesTheJobAndItsPods(t *testing.T) { + cs := fake.NewSimpleClientset() + var policy metav1.DeletionPropagation + cs.PrependReactor("delete", "jobs", func(a k8stesting.Action) (bool, runtime.Object, error) { + if o := a.(k8stesting.DeleteActionImpl).GetDeleteOptions().PropagationPolicy; o != nil { + policy = *o + } + return false, nil, nil + }) + e := New(cs, Config{Image: "felis:1", BackupPVC: "felis-backups"}) + name, err := e.Start(context.Background(), Request{Server: "survival", Mode: ModeWorld, ID: "0011223344556677", + TargetURL: "http://api:8081/x", Token: secretToken}) + if err != nil { + t.Fatal(err) + } + if err := e.Stop(context.Background(), name); err != nil { + t.Fatalf("Stop: %v", err) + } + if _, err := cs.BatchV1().Jobs("minecraft").Get(context.Background(), name, metav1.GetOptions{}); !apierrors.IsNotFound(err) { + t.Fatalf("the Job is still there: %v", err) + } + if policy != metav1.DeletePropagationBackground { + t.Errorf("propagation = %q, want Background so its Pods go too", policy) + } + if err := e.Stop(context.Background(), name); err != nil { + t.Errorf("stopping a Job already gone: %v", err) + } +} diff --git a/internal/worldexport/worldexport.go b/internal/worldexport/worldexport.go index 1158319..d207e2d 100644 --- a/internal/worldexport/worldexport.go +++ b/internal/worldexport/worldexport.go @@ -20,6 +20,7 @@ import ( "time" "felis.lolicon.best/internal/naming" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" ) @@ -130,3 +131,15 @@ func (e *Exporter) Start(ctx context.Context, r Request) (string, error) { } return job.Name, nil } + +// Stop deletes an export Job felis-api has given up on, so a Pod that never got +// going stops holding the world. Its Pods go with it; one already gone is not +// an error. +func (e *Exporter) Stop(ctx context.Context, job string) error { + bg := metav1.DeletePropagationBackground + err := e.cs.BatchV1().Jobs(e.cfg.Namespace).Delete(ctx, job, metav1.DeleteOptions{PropagationPolicy: &bg}) + if err != nil && !apierrors.IsNotFound(err) { + return fmt.Errorf("worldexport: delete export job: %w", err) + } + return nil +} diff --git a/panel/src/components/files/names.ts b/panel/src/components/files/names.ts index 1fe44e0..4c050fe 100644 --- a/panel/src/components/files/names.ts +++ b/panel/src/components/files/names.ts @@ -25,6 +25,12 @@ export function isManaged(path: string): boolean { /** The longest name a Linux filesystem takes, in bytes (NAME_MAX). */ const NAME_MAX = 255; +/** nameTooLong says whether a name is past NAME_MAX. A Mac or Windows disk + * counts in characters, so a file picked there can be. */ +export function nameTooLong(name: string): boolean { + return new TextEncoder().encode(name).length > NAME_MAX; +} + export type NameProblem = "name_required" | "name_slash" | "name_dots" | "name_too_long" | "name_taken"; /** nameProblem says why name cannot be used for a new entry in a folder listing @@ -40,7 +46,7 @@ export function nameProblem( if (name === "") return "name_required"; if (name.includes("/")) return "name_slash"; if (name === "." || name === "..") return "name_dots"; - if (new TextEncoder().encode(name).length > NAME_MAX) return "name_too_long"; + if (nameTooLong(name)) return "name_too_long"; if (name !== current && entries?.some((e) => e.name === name)) return "name_taken"; return null; } diff --git a/panel/src/components/files/opText.test.ts b/panel/src/components/files/opText.test.ts index 529cd52..8923c13 100644 --- a/panel/src/components/files/opText.test.ts +++ b/panel/src/components/files/opText.test.ts @@ -22,6 +22,10 @@ describe("opErrorText", () => { expect(opErrorText("unzip", e("volume_full", { need: 3 * 1024 * 1024, avail: 1024 }))).toBe( "Not enough room on the world volume: 3.0 MiB needed, 1.0 KiB free. Nothing was changed.", ); + // A full volume: the Job leaves the zero out. + expect(opErrorText("upload", e("volume_full", { need: 2048 }))).toBe( + "Not enough room on the world volume: 2.0 KiB needed, 0 B free. Nothing was changed.", + ); expect(opErrorText("upload", e("volume_full"))).toBe(humanizeError({ status: 0, code: "volume_full", message: "raw words" })); }); @@ -50,6 +54,17 @@ describe("opErrorText", () => { ); }); + it("tells an extraction killed for memory to split the archive", () => { + // The message felis-api words a memory kill with. + const oom = "the file operation ran out of memory (OOMKilled); an archive of this many files has to be split into smaller ones"; + expect(opErrorText("unzip", e("job_failed", { message: oom }))).toBe( + "The archive holds more files than the extraction task has memory to list, so the system stopped it. The files on the server were not changed. Split it into several smaller zips and extract each one.", + ); + expect(opErrorText("upload", e("job_failed", { message: oom }))).toBe( + "The background task ran out of memory and the system stopped it. The files on the server were not changed. Try again.", + ); + }); + it("words any other code as the file routes do", () => { expect(opErrorText("upload", e("file_changed"))).toBe(humanizeError({ status: 0, code: "file_changed", message: "raw words" })); expect(opErrorText("upload", e("file_changed"))).not.toBe("raw words"); diff --git a/panel/src/components/files/opText.ts b/panel/src/components/files/opText.ts index 6945300..42291d6 100644 --- a/panel/src/components/files/opText.ts +++ b/panel/src/components/files/opText.ts @@ -14,9 +14,11 @@ export function opErrorText(op: FileOp["op"], e: FileOpError): string { return op === "upload" ? t("op_upload_exists") : t("op_unzip_conflicts", { count: e.conflict_count ?? e.conflicts?.length ?? 0 }); + // A volume with nothing left leaves avail out of the error, the way the Job + // omits a zero. case "volume_full": - return e.need !== undefined && e.avail !== undefined - ? t("op_volume_full", { need: formatBytes(e.need), avail: formatBytes(e.avail) }) + return e.need !== undefined + ? t("op_volume_full", { need: formatBytes(e.need), avail: formatBytes(e.avail ?? 0) }) : humanizeError({ status: 0, code: e.code, message: e.message }); case "archive_invalid": return entry ? t("archive_invalid_entry", { entry }) : t("archive_invalid"); @@ -24,9 +26,10 @@ export function opErrorText(op: FileOp["op"], e: FileOpError): string { case "archive_symlink": case "type_conflict": return t(e.code, { entry }); - // The Job's own condition reason rides in the message; a deadline is the - // one worth its own words. + // The Job's reason rides in the message; a deadline and a memory kill are + // the ones worth their own words. case "job_failed": + if (e.message.includes("OOMKilled")) return op === "unzip" ? t("job_out_of_memory_unzip") : t("job_out_of_memory"); return e.message.includes("DeadlineExceeded") ? t("job_timed_out") : t("job_failed"); default: return humanizeError({ status: 0, code: e.code, message: e.message }); diff --git a/panel/src/components/files/sessionUpload.test.ts b/panel/src/components/files/sessionUpload.test.ts index e8c684c..106d9e7 100644 --- a/panel/src/components/files/sessionUpload.test.ts +++ b/panel/src/components/files/sessionUpload.test.ts @@ -1,4 +1,5 @@ // @vitest-environment jsdom +import { createHash } from "node:crypto"; import { describe, it, expect, vi, beforeEach } from "vitest"; import type { FileOp, FileUploadSession } from "@/lib/types"; import { MAX_ATTEMPTS } from "@/lib/contextUpload"; @@ -29,14 +30,26 @@ vi.mock("@/lib/api", async (importOriginal) => { const KEY = "felis-file-upload:lobby:world.zip"; const PART = 4; const NOW = 1_000_000_000; +const BODY = "0123456789"; // A 10-byte file sent in parts of 4: offsets 0, 4 and 8. -function file(body = "0123456789", modified = 111) { +function file(body = BODY, modified = 111) { return new File([body], "world.zip", { lastModified: modified }); } +// The parts a server holding the first `received` bytes of body lists, as +// they went up: PART bytes each, the last one short, hashed by Node. +function partsOf(received: number, body = BODY) { + const parts: FileUploadSession["parts"] = []; + for (let at = 0; at < received; at += PART) { + const end = Math.min(at + PART, received); + parts.push({ size: end - at, sha256: createHash("sha256").update(body.slice(at, end)).digest("hex") }); + } + return parts; +} + function session(received: number, over: Partial = {}): FileUploadSession { - return { id: "s1", path: "world.zip", size: 10, received, part_max_bytes: PART, ...over }; + return { id: "s1", path: "world.zip", size: 10, received, part_max_bytes: PART, parts: partsOf(received), ...over }; } function op(over: Partial = {}): FileOp { @@ -134,6 +147,75 @@ describe("sendInParts", () => { expect(mocks.commitServerFileUpload.mock.calls).toEqual([["lobby", "s7", false]]); }); + it("hashes the parts a remembered session holds against the file before carrying on", async () => { + localStorage.setItem(KEY, JSON.stringify({ server: "lobby", path: "world.zip", id: "s7", size: 10, modified: 111, touched: 0 })); + mocks.getServerFileUpload.mockResolvedValue(session(8, { id: "s7" })); + const seen: number[] = []; + + await sendInParts("lobby", "world.zip", file(), opts({ onProgress: (n) => seen.push(n) })); + + // 4 and 8 as each held part checks out, 8 where the session stands, 10 sent. + expect(seen).toEqual([4, 8, 8, 10]); + expect(mocks.deleteServerFileUpload).not.toHaveBeenCalled(); + expect(offsets()).toEqual([8]); + }); + + it.each([ + ["a part of another file of the same name, size and time", partsOf(8, "abcd4567")], + ["a later part of another file", partsOf(8, "0123x567")], + ["fewer parts than the bytes it says it holds", partsOf(4)], + ])("gives back a remembered session holding %s, and starts the file over", async (_label, parts) => { + localStorage.setItem(KEY, JSON.stringify({ server: "lobby", path: "world.zip", id: "s7", size: 10, modified: 111, touched: 0 })); + mocks.getServerFileUpload.mockResolvedValue(session(8, { id: "s7", parts })); + mocks.deleteServerFileUpload.mockResolvedValue(null); + + await sendInParts("lobby", "world.zip", file(), opts()); + + expect(mocks.deleteServerFileUpload.mock.calls).toEqual([["lobby", "s7"]]); + expect(mocks.beginServerFileUpload.mock.calls).toEqual([["lobby", "world.zip", 10]]); + expect(offsets()).toEqual([0, 4, 8]); + expect(mocks.commitServerFileUpload.mock.calls).toEqual([["lobby", "s1", false]]); + expect(stored().id).toBe("s1"); + }); + + it("stops while hashing what a remembered session holds, and keeps it for later", async () => { + localStorage.setItem(KEY, JSON.stringify({ server: "lobby", path: "world.zip", id: "s7", size: 10, modified: 111, touched: 0 })); + const ctrl = new AbortController(); + mocks.getServerFileUpload.mockImplementation(async () => { + ctrl.abort(); + return session(8, { id: "s7" }); + }); + + await expect(sendInParts("lobby", "world.zip", file(), opts({ signal: ctrl.signal }))).rejects.toMatchObject({ + name: "AbortError", + }); + + expect(offsets()).toEqual([]); + expect(mocks.deleteServerFileUpload).not.toHaveBeenCalled(); + expect(stored().id).toBe("s7"); + }); + + it("sends a part changed on the way again, hashing none of the parts already known", async () => { + let first = true; + mocks.putServerFileUploadPart.mockImplementation(async (_n: string, id: string, offset: number, part: Blob) => { + if (offset === 4 && first) { + first = false; + throw { status: 400, code: "digest_mismatch", message: "" }; + } + return session(offset + part.size, { id }); + }); + mocks.getServerFileUpload.mockResolvedValue(session(4)); + const seen: number[] = []; + + await sendInParts("lobby", "world.zip", file(), opts({ onProgress: (n) => seen.push(n) })); + + expect(offsets()).toEqual([0, 4, 4, 8]); + expect(sleep.mock.calls.map((c) => c[0])).toEqual([1000]); + // No 4 from hashing the first part again: this run sent it. + expect(seen).toEqual([0, 4, 4, 8, 10]); + expect(mocks.commitServerFileUpload.mock.calls).toEqual([["lobby", "s1", false]]); + }); + it("commits at once when the server already holds the whole file", async () => { localStorage.setItem(KEY, JSON.stringify({ server: "lobby", path: "world.zip", id: "s7", size: 10, modified: 111, touched: 0 })); mocks.getServerFileUpload.mockResolvedValue(session(10, { id: "s7" })); diff --git a/panel/src/components/files/sessionUpload.ts b/panel/src/components/files/sessionUpload.ts index 7d930e6..97522e1 100644 --- a/panel/src/components/files/sessionUpload.ts +++ b/panel/src/components/files/sessionUpload.ts @@ -1,4 +1,5 @@ import { api, clientError } from "@/lib/api"; +import { sha256Of } from "@/lib/digest"; import { MAX_ATTEMPTS, isTransient, retryDelay, wait } from "@/lib/contextUpload"; import type { FileOp, FileUploadSession } from "@/lib/types"; @@ -11,8 +12,14 @@ import type { FileOp, FileUploadSession } from "@/lib/types"; // The session is also remembered in this browser, under the server and path it // lands at, with the file's size and modification time. Choosing the same file // again after a reload, a closed tab or a lost connection carries on from the -// bytes already sent. A remembered session no page has touched for a while is -// what a refusal for too many sessions gives back first. +// bytes already sent, once those have been hashed again against the file: the +// session lists each part it holds with its SHA-256, and a session holding +// anything else (another file of the same name, size and time) is given back +// and the file starts over. A remembered session no page has touched for a +// while is what a refusal for too many sessions gives back first. +// +// Every part goes with its SHA-256, and one the server hashes differently +// (digest_mismatch: changed on the way) is sent again. type Sleep = (ms: number, signal?: AbortSignal) => Promise; @@ -153,12 +160,23 @@ export async function sendInParts(server: string, path: string, file: File, opts let offset: number | null = null; let partMax = 0; let failures = 0; + // known is how many bytes at the head of the session this run has seen to be + // the file's own: parts it sent, or held parts it hashed again. A session + // begun during the run holds only parts the run sent, so a new one needs no + // reset. + let known = 0; for (;;) { try { if (offset === null) { const at = await standing(server, path, file, id, now); id = at.id; opts.onSession?.(id); + if (!(await holdsTheFile(file, at, known, onProgress, signal))) { + await discardSession(server, path, at.id, sleep); + id = null; + continue; + } + known = at.received; partMax = at.part_max_bytes; offset = at.received; onProgress?.(offset); @@ -171,12 +189,14 @@ export async function sendInParts(server: string, path: string, file: File, opts onProgress: (sent) => onProgress?.(start + sent), }); offset = at.received; + known = at.received; failures = 0; remember({ server, path, id: id!, size, modified: file.lastModified, touched: now() }); onProgress?.(offset); } catch (e) { // The session went away under the upload (felis-api restarted, or it sat - // idle too long): the next pass begins a new one. + // idle too long): the next pass begins a new one. A part changed on the + // way was not kept, so the next pass sends it again (isTransient). if (code(e) === "upload_not_found") { id = null; } else if (!isTransient(e)) { @@ -220,11 +240,34 @@ async function standing( return at; } +// holdsTheFile hashes the parts session at holds past the first known bytes +// against the same ranges of file, and answers whether all of them match. +// onProgress walks up through the parts as they check out. +async function holdsTheFile( + file: File, + at: FileUploadSession, + known: number, + onProgress?: (sent: number) => void, + signal?: AbortSignal, +): Promise { + let start = 0; + for (const part of at.parts) { + const end = start + part.size; + if (end > known) { + if (signal?.aborted) throw new DOMException("The upload was cancelled", "AbortError"); + if ((await sha256Of(file.slice(start, end))).hex !== part.sha256) return false; + onProgress?.(end); + } + start = end; + } + return start === at.received; +} + // commit lands the session. A commit whose answer was lost may still have // started the Job, so asking again is read in that light: the world held // (maintenance_in_progress) by an upload of this path running now, or the -// session gone because that Job already fetched it, means the first commit -// went through, and its op is the answer. +// session gone because that Job reported the file landed, means the first +// commit went through, and its op is the answer. async function commit( server: string, path: string, diff --git a/panel/src/components/files/useUploads.ts b/panel/src/components/files/useUploads.ts index 337b71f..ea6a968 100644 --- a/panel/src/components/files/useUploads.ts +++ b/panel/src/components/files/useUploads.ts @@ -3,7 +3,7 @@ import i18next from "i18next"; import { api, humanizeError } from "@/lib/api"; import { formatBytes } from "@/lib/format"; import type { FileOpError, ServerFileEntry } from "@/lib/types"; -import { joinPath } from "./names"; +import { joinPath, nameTooLong } from "./names"; import { opErrorText } from "./opText"; import { discardSession, forgetSession, sendInParts, watchOp } from "./sessionUpload"; @@ -187,6 +187,10 @@ export function useUploads(server: string, { onLanded, onOp, hold = false, free retryable: false, landing: null, }; + // The server would refuse it only after the bytes were sent. + if (nameTooLong(file.name)) { + return { ...base, state: "failed", error: t("upload_name_too_long") }; + } const there = entries?.find((e) => e.name === file.name); if (there?.is_dir) { return { ...base, state: "failed", error: t("upload_folder_there") }; diff --git a/panel/src/components/files/useWorldJobs.ts b/panel/src/components/files/useWorldJobs.ts new file mode 100644 index 0000000..15abb70 --- /dev/null +++ b/panel/src/components/files/useWorldJobs.ts @@ -0,0 +1,76 @@ +import { useCallback, useEffect, useRef, useState } from "react"; +import { api } from "@/lib/api"; +import { usePolling } from "@/lib/hooks"; +import type { ServerJob } from "@/lib/types"; +import { OP_POLL_MS } from "./sessionUpload"; + +/** holdsWorld says whether a Job keeps the world from being changed, as the + * server counts it (maintenance.JobKind): a running backup, restore, world + * export or file download. A backup export reads only the backup store. A + * safety snapshot whose restore has yet to start holds it for that restore. */ +export function holdsWorld(j: ServerJob): boolean { + return j.then_restore === "pending" || (j.state === "running" && j.kind !== "export_backup"); +} + +/** restores says whether the holder is (or leads to) a restore, which replaces + * the files the page lists. */ +function restores(j: ServerJob): boolean { + return j.kind === "restore" || j.then_restore === "pending"; +} + +/** holderText is the files key naming what holds the world and what to wait + * for. */ +export function holderText(j: ServerJob): string { + if (restores(j)) return "wait_for_restore"; + switch (j.kind) { + case "backup": + return "wait_for_backup"; + case "export_world": + return "wait_for_world_export"; + default: + return "wait_for_file_download"; + } +} + +// useWorldJobs follows the Jobs that hold a server's world besides the file +// manager's own ops (useFileOps): read once when `enabled` turns on, then every +// OP_POLL_MS while one holds it. onRestored runs when a restore seen here ends. +export function useWorldJobs(server: string, enabled: boolean, onRestored: () => void) { + const [holder, setHolder] = useState(null); + const restoring = useRef(false); + const restored = useRef(onRestored); + restored.current = onRestored; + // As in useFileOps: only the newest read lands, and none starts over another. + const seq = useRef(0); + const inFlight = useRef(false); + + const read = useCallback(async () => { + if (inFlight.current) return; + inFlight.current = true; + const ticket = ++seq.current; + try { + const jobs = await api.serverJobs(server); + if (ticket !== seq.current) return; + const h = jobs.find(holdsWorld) ?? null; + setHolder(h); + const now = h !== null && restores(h); + if (restoring.current && !now) restored.current(); + restoring.current = now; + } catch { + // A list that fails leaves the page as it was: a change the world cannot + // take is still refused, in the server's words. + } finally { + inFlight.current = false; + } + }, [server]); + + useEffect(() => { + if (enabled) void read(); + else setHolder(null); + }, [enabled, read]); + + const poll = useCallback(() => void read(), [read]); + usePolling(poll, enabled && holder !== null ? OP_POLL_MS : null); + + return { holder, refresh: poll }; +} diff --git a/panel/src/i18n/resources/en-US/errors.json b/panel/src/i18n/resources/en-US/errors.json index 0850b3e..7ca7f5a 100644 --- a/panel/src/i18n/resources/en-US/errors.json +++ b/panel/src/i18n/resources/en-US/errors.json @@ -25,14 +25,14 @@ "no_backup": "There's no restorable backup for this server yet.", "backup_corrupt": "This backup failed its read-back check and can't be restored intact — pick another backup.", "not_stopped": "Stop the server completely first — this changes its world volume, which the running server holds.", - "maintenance_in_progress": "This server's world is busy with a restore, backup, world export, file write or idle reclaim — try again once it finishes. A restore, backup or file write usually takes a minute or two; a world export lasts until its download ends, and an idle reclaim can take longer on a large world.", + "maintenance_in_progress": "This server's world is busy with a restore, backup, world export, file download, file change or idle reclaim. Try again once it finishes. A restore, backup or file change usually takes a minute or two; an export or a download lasts until the browser has all of it, and an idle reclaim can take longer on a large world.", "no_world_volume": "This server has no world volume yet — start it once so it is created, then retry.", "file_changed": "This file changed after you opened it (another manager saved it, or the server rewrote it on its last run), so your save was not written, to keep that change.", "volume_full": "The server's volume is full, so the change was not written and the files there are unchanged. Delete files it no longer needs, or ask an admin to grow its volume.", "backup_cooldown": "This server was backed up moments ago, and manual backups have a cooldown — try again in a few minutes.", "backup_store_full": "The backup store is full, so manual backups are paused — ask an administrator to free space.", "restore_unavailable": "Restore isn't available right now — try again later.", - "export_busy": "Too many downloads are being prepared right now (one at a time per person, six an hour) — try again in a few minutes.", + "export_busy": "Too many exports are being prepared: one at a time per person, two at a time across the platform, and six an hour per person. Try again in a few minutes.", "export_expired": "This download has expired or was already used — start the export again.", "export_not_ready": "The download isn't ready yet — wait a moment and try again.", "export_unavailable": "Downloading worlds and backups isn't set up on this deployment — ask an administrator.", @@ -71,6 +71,10 @@ "upload_staging_full": "The panel's upload space is nearly full right now, so the file was not passed on. Try again later, or ask an admin to free space on the uploads volume.", "upload_incomplete": "The upload stopped before the whole file arrived, so nothing was changed. Try again.", "length_required": "The upload did not say how large it is, so it was refused. Upload it again from the panel.", + "digest_mismatch": "The file was changed on its way to the server, so it was refused and nothing was written. Try again.", + "digest_required": "The upload came without a checksum, so it was refused. Reload the panel and upload it again.", + "bad_digest": "The upload's checksum was malformed, so it was refused. Reload the panel and upload it again.", + "file_unreadable": "The file could not be read: it was changed, moved or deleted after it was picked. Pick it again and upload.", "upload_not_found": "This upload is gone: it was cancelled, already landed, sat idle for 6 hours, or the panel service restarted. Upload the file again.", "too_many_uploads": "You already have 4 large uploads in progress. Wait for one to finish, or cancel one, and try again.", "op_lost": "The operation's progress can no longer be read. Refresh the list to see whether the file landed.", diff --git a/panel/src/i18n/resources/en-US/files.json b/panel/src/i18n/resources/en-US/files.json index 845239d..eb7762a 100644 --- a/panel/src/i18n/resources/en-US/files.json +++ b/panel/src/i18n/resources/en-US/files.json @@ -79,10 +79,15 @@ "upload_clear_done": "Clear finished", "upload_progress_label": "Uploading {{name}}", "upload_folder_there": "A folder with this name is already here.", + "upload_name_too_long": "This file's name is longer than the 255 bytes a file name on the server can hold. Rename it shorter, then upload it.", "upload_no_room": "The world volume has {{free}} free, not enough for this {{size}} file. Delete files you do not need, then retry.", "upload_landing_progress": "Writing it to the server… {{percent}}%", "wait_for_op": "Wait for the background operation to finish first.", "wait_for_download": "Wait until the download is ready first.", + "wait_for_backup": "A backup of this world is running. Change files once it finishes.", + "wait_for_restore": "This world is being restored. Change files once the restore finishes.", + "wait_for_world_export": "This world is being exported. Change files once that download ends.", + "wait_for_file_download": "A download is reading this world. Change files once it has finished.", "unzip_item": "Extract {{name}} here", "unzip_conflicts_title_one": "Extracting {{name}} replaces {{count}} file", "unzip_conflicts_title_other": "Extracting {{name}} replaces {{count}} files", @@ -97,7 +102,8 @@ "download_started_config": "Downloading {{filename}}. paper-global.yml, the proxy forwarding secret every server shares, is left out.", "download_failed": "The download could not be prepared.", "download_failed_because": "The download could not be prepared: {{reason}}", - "download_busy": "Too many file downloads are being prepared (two at a time per person, thirty an hour). Try again in a few minutes.", + "download_out_of_memory": "This folder holds more files than the zipping task has memory to list, so the system stopped it and nothing was downloaded. Download its folders one by one instead.", + "download_busy": "Too many file downloads are being prepared: two at a time per person, four at a time across the platform, and thirty an hour per person. Try again in a few minutes.", "secret_config_no_download": "config/paper-global.yml holds the proxy forwarding secret every server shares, so it cannot be downloaded.", "ops_label": "Background operations", "ops_refresh_failed": "Could not read the background operations: {{reason}}", @@ -121,5 +127,7 @@ "archive_symlink": "{{entry}} in the archive is a symbolic link. Nothing in the archive was extracted.", "type_conflict": "{{entry}} is a file on one side and a folder on the other, which replacing cannot resolve. Rename or delete {{entry}} here, then extract again.", "job_failed": "The background task ended without saying why. Refresh the list to check, then try again.", - "job_timed_out": "The background task did not finish within 2 hours and was stopped. Refresh the list to check, then try again." + "job_timed_out": "The background task did not finish within 2 hours and was stopped. Refresh the list to check, then try again.", + "job_out_of_memory": "The background task ran out of memory and the system stopped it. The files on the server were not changed. Try again.", + "job_out_of_memory_unzip": "The archive holds more files than the extraction task has memory to list, so the system stopped it. The files on the server were not changed. Split it into several smaller zips and extract each one." } diff --git a/panel/src/i18n/resources/zh-CN/errors.json b/panel/src/i18n/resources/zh-CN/errors.json index 106a3b8..77fb34a 100644 --- a/panel/src/i18n/resources/zh-CN/errors.json +++ b/panel/src/i18n/resources/zh-CN/errors.json @@ -25,14 +25,14 @@ "no_backup": "这台服务器暂时没有可回档的备份。", "backup_corrupt": "这份备份回读校验未通过，已无法完整恢复——请选择另一份备份。", "not_stopped": "请先把服务器完全停止——这项操作要改动世界存储卷，运行中的服务器独占着它。", - "maintenance_in_progress": "这台服务器的世界正在回档、备份、导出、写入文件或闲置回收——等它完成后再试。回档、备份和写文件通常一两分钟，导出要等下载结束，闲置回收视世界大小可能更久。", + "maintenance_in_progress": "这台服务器的世界正在回档、备份、导出、下载文件、改动文件或闲置回收，等它完成后再试。回档、备份和改动文件通常一两分钟，导出和下载要等浏览器下载完，闲置回收视世界大小可能更久。", "no_world_volume": "这台服务器还没有世界卷——先启动一次让它创建，然后再试。", "file_changed": "这个文件在你打开之后被改过了（另一位管理者保存过，或者服务器上次运行时改写了它）。为了不覆盖那次修改，这次保存没有写入。", "volume_full": "服务器的存储卷已满，这次改动没有写入，原有文件保持不变。删掉用不着的文件，或者请管理员给它扩容。", "backup_cooldown": "这台服务器刚备份过，手动备份之间有冷却时间——请过几分钟再试。", "backup_store_full": "备份存储已满，暂时无法手动备份——请联系管理员清理空间。", "restore_unavailable": "回档功能当前不可用，请稍后再试。", - "export_busy": "现在正在准备的下载太多了（每人同时一个、每小时最多六个）——请过几分钟再试。", + "export_busy": "正在准备的导出太多了：每人同时一个，整个平台同时最多两个，每人每小时最多六个。请过几分钟再试。", "export_expired": "这个下载已过期或已经用过了——请重新导出。", "export_not_ready": "下载还没准备好——请稍等片刻再试。", "export_unavailable": "当前部署没有开通世界和备份下载——请联系管理员。", @@ -71,6 +71,10 @@ "upload_staging_full": "面板的上传暂存空间快满了，这个文件没有转存过去。稍后再试，或者请管理员清理上传卷。", "upload_incomplete": "文件还没传完上传就中断了，什么都没有改动。请重试。", "length_required": "这次上传没有声明文件大小，被拒绝了。请从面板重新上传。", + "digest_mismatch": "文件在传输途中被改动了，服务器已拒收，什么都没有写入。请重试。", + "digest_required": "这次上传没有附带校验值，被拒绝了。请刷新面板后重新上传。", + "bad_digest": "这次上传附带的校验值格式不对，被拒绝了。请刷新面板后重新上传。", + "file_unreadable": "读不出这个文件：它在选中之后被改动、移走或删除了。请重新选择文件再上传。", "upload_not_found": "这次分片上传已经不在了（取消过、已经写入、闲置超过 6 小时，或者面板服务重启过）。请重新上传。", "too_many_uploads": "你同时进行的大文件上传已经有 4 个了。等其中一个完成，或者取消一个再试。", "op_lost": "看不到这次操作的进度了。刷新列表看看文件有没有写入。", diff --git a/panel/src/i18n/resources/zh-CN/files.json b/panel/src/i18n/resources/zh-CN/files.json index b99c13b..71999f4 100644 --- a/panel/src/i18n/resources/zh-CN/files.json +++ b/panel/src/i18n/resources/zh-CN/files.json @@ -78,10 +78,15 @@ "upload_clear_done": "清除已完成", "upload_progress_label": "正在上传 {{name}}", "upload_folder_there": "这里已经有同名文件夹。", + "upload_name_too_long": "这个文件名超过 255 字节，服务器上的文件名存不下。先改短再上传。", "upload_no_room": "世界卷只剩 {{free}}，放不下这个 {{size}} 的文件。先删掉些用不着的文件再重试。", "upload_landing_progress": "正在写入服务器… {{percent}}%", "wait_for_op": "等后台操作完成后再改动。", "wait_for_download": "等下载准备好后再操作。", + "wait_for_backup": "正在备份这台服务器的世界，备份结束后才能改动文件。", + "wait_for_restore": "正在回档这台服务器的世界，回档结束后才能改动文件。", + "wait_for_world_export": "正在导出这台服务器的世界，那边的下载结束后才能改动文件。", + "wait_for_file_download": "有一个下载正在读取这台服务器的世界，传完后才能改动文件。", "unzip_item": "把 {{name}} 解压到此处", "unzip_conflicts_title": "解压 {{name}} 会覆盖 {{count}} 个文件", "unzip_conflicts_body": "压缩包里的这些文件在这里已经存在。确认后会用压缩包里的版本替换它们，其余文件照常解压。旧版本之后可能还要用的话，先做一次备份。", @@ -95,7 +100,8 @@ "download_started_config": "已开始下载 {{filename}}。所有服务器共用的代理转发密钥 paper-global.yml 不在里面。", "download_failed": "下载没有准备好。", "download_failed_because": "下载没有准备好：{{reason}}", - "download_busy": "正在准备的文件下载太多了（每人同时两个、每小时最多三十个），请过几分钟再试。", + "download_out_of_memory": "这个文件夹里的文件太多，打包任务的内存装不下它的文件清单，被系统停掉了，什么都没下载。请分成几个小一些的文件夹分别下载。", + "download_busy": "正在准备的文件下载太多了：每人同时最多两个，整个平台同时最多四个，每人每小时最多三十个。请过几分钟再试。", "secret_config_no_download": "config/paper-global.yml 里有所有服务器共用的代理转发密钥，不能下载。", "ops_label": "后台操作", "ops_refresh_failed": "读取后台操作失败：{{reason}}", @@ -117,5 +123,7 @@ "archive_symlink": "压缩包里的 {{entry}} 是符号链接。整个压缩包都没有解压。", "type_conflict": "压缩包里的 {{entry}} 和这里已有的同名项一个是文件、一个是文件夹，覆盖解决不了。先把这里的 {{entry}} 改名或删掉再解压。", "job_failed": "后台任务没说明原因就结束了。刷新列表确认一下，再试一次。", - "job_timed_out": "后台任务 2 小时还没做完，被停下了。刷新列表确认一下，再试一次。" + "job_timed_out": "后台任务 2 小时还没做完，被停下了。刷新列表确认一下，再试一次。", + "job_out_of_memory": "后台任务用完了内存，被系统停掉了。服务器上的文件没有被改动，再试一次。", + "job_out_of_memory_unzip": "压缩包里的文件太多，解压任务的内存装不下它的文件清单，被系统停掉了。服务器上的文件没有被改动。把它拆成几个小一些的 zip，分别上传解压。" } diff --git a/panel/src/lib/api.test.ts b/panel/src/lib/api.test.ts index 7d687e1..b5fe9d4 100644 --- a/panel/src/lib/api.test.ts +++ b/panel/src/lib/api.test.ts @@ -1,3 +1,4 @@ +import { createHash } from "node:crypto"; import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; // Pin the GET /me wire shape. is_admin crosses an untyped fetch().json() boundary @@ -658,20 +659,6 @@ describe("image whitelist and builds wire shapes", () => { expect(JSON.parse((opts as RequestInit).body as string)).toEqual({ display_name: "new submission" }); }); - it("uploadSubmissionContext POSTs Blob to /me/submissions/{id}/context", async () => { - const sub = { id: "sub-3", display_name: "new submission", status: "pending_review" }; - const fetchSpy = fakeFetch(sub); - vi.stubGlobal("fetch", fetchSpy); - const blob = new Blob(["test"], { type: "application/x-gzip" }); - const res = await api.uploadSubmissionContext("sub-3", blob); - expect(res).toEqual(sub); - const [url, opts] = (fetchSpy as unknown as ReturnType).mock.calls[0]; - expect(String(url)).toBe("/me/submissions/sub-3/context"); - expect((opts as RequestInit).method).toBe("POST"); - expect((opts as RequestInit).body).toBe(blob); - expect((opts as RequestInit).headers).toEqual({ "Content-Type": "application/x-gzip" }); - }); - // The lane's two throttled outcomes (a spent allowance, a closed cooldown) // must surface as their own copy, not the generic forbidden/error text. it("maps the submission quota/cooldown codes to stable human copy", async () => { @@ -1214,6 +1201,10 @@ async function sentXHR(): Promise { return FakeXHR.last as FakeXHR; } +// What felis-api's parseContentDigest takes: the body's SHA-256, base64, in +// RFC 9530's sha-256=:…: form. Hashed here by Node, apart from the panel's own. +const contentDigest = (body: string) => `sha-256=:${createHash("sha256").update(body).digest("base64")}:`; + describe("chunked context upload", () => { beforeEach(() => { vi.restoreAllMocks(); @@ -1243,7 +1234,7 @@ describe("chunked context upload", () => { expect((opts as RequestInit).method).toBe("POST"); }); - it("putContextPart PUTs the part at its offset with the session cookie and reports progress", async () => { + it("putContextPart PUTs the part at its offset with the session cookie, its SHA-256 and reports progress", async () => { const part = new Blob(["abcd"]); const seen: number[] = []; const done = api.putContextPart("sub-3", 8, part, { onProgress: (n) => seen.push(n) }); @@ -1251,7 +1242,7 @@ describe("chunked context upload", () => { expect(xhr.method).toBe("PUT"); expect(xhr.url).toBe("/me/submissions/sub-3/context/upload?offset=8"); expect(xhr.withCredentials).toBe(true); - expect(xhr.headers).toEqual({ "Content-Type": "application/octet-stream" }); + expect(xhr.headers).toEqual({ "Content-Type": "application/octet-stream", "Content-Digest": contentDigest("abcd") }); expect(xhr.body).toBe(part); xhr.upload.onprogress?.({ loaded: 3 }); xhr.respond(200, JSON.stringify(progress)); @@ -1348,6 +1339,28 @@ describe("server file manager wire shapes", () => { return [String(url), opts as RequestInit]; } + it("a file the browser can no longer read is refused before anything is sent", async () => { + const changed = Object.assign(new Blob(["jar bytes"]), { + arrayBuffer: () => Promise.reject(new DOMException("the file changed on disk", "NotReadableError")), + }); + await expect(api.uploadServerFile("survival", "plugins/a.jar", changed, false)).rejects.toEqual({ + status: 0, + code: "file_unreadable", + message: "the file changed on disk", + }); + await expect(api.putServerFileUploadPart("survival", "s1", 0, changed)).rejects.toMatchObject({ + code: "file_unreadable", + }); + expect(FakeXHR.last).toBeUndefined(); + expect(humanizeError({ code: "file_unreadable" })).toMatch(/changed, moved or deleted/); + }); + + it("the upload refusals over a checksum read as what to do next", () => { + expect(humanizeError({ code: "digest_mismatch" })).toMatch(/changed on its way/); + expect(humanizeError({ code: "digest_required" })).toMatch(/without a checksum/); + expect(humanizeError({ code: "bad_digest" })).toMatch(/checksum was malformed/); + }); + it("createServerFile PUTs the content with create_only, so nothing already there is replaced", async () => { const fetchSpy = fakeFetch({ path: "plugins/new.yml", status: "written", sha256: "c".repeat(64) }); vi.stubGlobal("fetch", fetchSpy); @@ -1406,7 +1419,7 @@ describe("server file manager wire shapes", () => { expect(xhr.method).toBe("PUT"); expect(xhr.url).toBe("/servers/survival/files/upload?path=plugins%2FChunky%201.4.jar"); expect(xhr.withCredentials).toBe(true); - expect(xhr.headers).toEqual({ "Content-Type": "application/octet-stream" }); + expect(xhr.headers).toEqual({ "Content-Type": "application/octet-stream", "Content-Digest": contentDigest("jar bytes") }); expect(xhr.body).toBe(file); xhr.upload.onprogress?.({ loaded: 4 }); xhr.respond(200, JSON.stringify({ path: "plugins/Chunky 1.4.jar", status: "uploaded", sha256: "d".repeat(64), size: 9 })); @@ -1485,6 +1498,7 @@ describe("server file manager wire shapes", () => { expect(xhr.method).toBe("PUT"); expect(xhr.url).toBe("/servers/survival/files/uploads/s1?offset=33554432"); expect(xhr.withCredentials).toBe(true); + expect(xhr.headers).toEqual({ "Content-Type": "application/octet-stream", "Content-Digest": contentDigest("part bytes") }); expect(xhr.body).toBe(part); xhr.upload.onprogress?.({ loaded: 3 }); xhr.respond(200, JSON.stringify({ ...session, received: 33_554_442 })); @@ -1757,7 +1771,7 @@ describe("world export wire shapes", () => { it("words the export refusals itself", () => { expect(humanizeError({ status: 429, code: "export_busy", message: "raw" })).toBe( - "Too many downloads are being prepared right now (one at a time per person, six an hour) — try again in a few minutes.", + "Too many exports are being prepared: one at a time per person, two at a time across the platform, and six an hour per person. Try again in a few minutes.", ); expect(humanizeError({ status: 410, code: "export_expired", message: "raw" })).toBe( "This download has expired or was already used — start the export again.", diff --git a/panel/src/lib/api.ts b/panel/src/lib/api.ts index 9a7bc55..d70967a 100644 --- a/panel/src/lib/api.ts +++ b/panel/src/lib/api.ts @@ -45,6 +45,7 @@ import type { UpdateReport, } from "./types"; import { loadConfig } from "./config"; +import { sha256Of } from "./digest"; import i18next from "i18next"; // Typed client for the felis-api external face (spec §7). Credentials are sent so @@ -231,28 +232,19 @@ function request(method: string, path: string, body?: unknown): Promise { }); } -function requestRaw( - method: string, - path: string, - body: Blob, - headers?: Record, -): Promise { - return send(path, { method, headers, body }); -} - // sendWithProgress sends body by XMLHttpRequest, the one browser API that // reports how much of a request body has gone out (fetch has no upload // progress), and settles the way send does: the parsed 2xx body, or the same // ApiError fetchOK would throw. onProgress gets the bytes of body sent so far; -// signal aborts the request with an AbortError. +// signal aborts the request with an AbortError; headers go along with it. async function sendWithProgress( method: string, path: string, body: Blob, - opts: { onProgress?: (sent: number) => void; signal?: AbortSignal } = {}, + opts: { onProgress?: (sent: number) => void; signal?: AbortSignal; headers?: Record } = {}, ): Promise { const { apiBase } = await loadConfig(); - const { onProgress, signal } = opts; + const { onProgress, signal, headers } = opts; return new Promise((resolve, reject) => { const xhr = new XMLHttpRequest(); const onAbort = () => xhr.abort(); @@ -260,6 +252,7 @@ async function sendWithProgress( xhr.open(method, `${apiBase}${path}`); xhr.withCredentials = true; xhr.setRequestHeader("Content-Type", "application/octet-stream"); + for (const [k, v] of Object.entries(headers ?? {})) xhr.setRequestHeader(k, v); if (onProgress) xhr.upload.onprogress = (e) => onProgress(e.loaded); xhr.onabort = () => { settle(); @@ -297,6 +290,19 @@ async function sendWithProgress( }); } +// sendDigested sends body as sendWithProgress does, with its SHA-256 in +// Content-Digest: felis-api hashes what arrives and keeps none of it on a +// mismatch (400 digest_mismatch), so bytes changed on the way never land. +async function sendDigested( + method: string, + path: string, + body: Blob, + opts: { onProgress?: (sent: number) => void; signal?: AbortSignal } = {}, +): Promise { + const { header } = await sha256Of(body); + return sendWithProgress(method, path, body, { ...opts, headers: { "Content-Digest": header } }); +} + // rejectingSync turns a synchronous throw inside an api method (urlPath refusing // a segment) into a rejected promise, so every caller handles it the way it // handles any failed call. @@ -754,7 +760,7 @@ export const api = rejectingSync({ // much room the world volume has (free_bytes), so an upload too big for it is // refused before it is sent. listServerFiles: (name: string, path: string) => - request<{ path: string; entries: ServerFileEntry[]; truncated: boolean; free_bytes: number }>( + request<{ path: string; entries: ServerFileEntry[]; truncated: boolean; free_bytes: number | null }>( "GET", urlPath`/servers/${name}/files` + `?path=${encodeURIComponent(path)}`, ), @@ -815,8 +821,9 @@ export const api = rejectingSync({ ), // uploadServerFile sends a file's raw bytes (the browser sets Content-Length - // from the Blob) with progress. Without overwrite an existing file is 409 - // file_exists; with it the file is replaced whole or not at all. + // from the Blob) and their SHA-256, with progress. Without overwrite an + // existing file is 409 file_exists; with it the file is replaced whole or not + // at all. uploadServerFile: ( name: string, path: string, @@ -824,7 +831,7 @@ export const api = rejectingSync({ overwrite: boolean, opts?: { onProgress?: (sent: number) => void; signal?: AbortSignal }, ) => - sendWithProgress<{ path: string; status: string; sha256: string; size: number }>( + sendDigested<{ path: string; status: string; sha256: string; size: number }>( "PUT", urlPath`/servers/${name}/files/upload` + `?path=${encodeURIComponent(path)}` + @@ -835,10 +842,12 @@ export const api = rejectingSync({ // A file too big for one request goes up in parts (components/files/ // sessionUpload.ts drives it): begin a session for its path and size, which - // reserves room for all of it; put each part at its byte offset; then commit, - // which answers at once with the op landing it (watch listServerFileOps). A - // session answers only the account and server it was begun for, stays until - // its file has been fetched whole once, and is dropped after 6 hours idle. + // reserves room for all of it; put each part at its byte offset, with its + // SHA-256; then commit, which answers at once with the op landing it (watch + // listServerFileOps). A session answers only the account and server it was + // begun for, and lists the parts it holds with their SHA-256. It stays until + // the Job landing its file reports it landed, so a landing that fails can be + // committed again, and is dropped after 6 hours idle. beginServerFileUpload: (name: string, path: string, size: number) => request( "POST", @@ -856,7 +865,7 @@ export const api = rejectingSync({ part: Blob, opts?: { onProgress?: (sent: number) => void; signal?: AbortSignal }, ) => - sendWithProgress( + sendDigested( "PUT", urlPath`/servers/${name}/files/uploads/${id}` + `?offset=${offset}`, part, @@ -1039,13 +1048,10 @@ export const api = rejectingSync({ createSubmission: (displayName: string) => request("POST", "/me/submissions", { display_name: displayName }), - uploadSubmissionContext: (id: string, file: Blob) => - requestRaw("POST", urlPath`/me/submissions/${id}/context`, file, { - "Content-Type": "application/x-gzip", - }), - // A chunked context upload (lib/contextUpload.ts drives it): ask where the - // staged upload stands, send each part at its byte offset, then store it. + // staged upload stands, send each part at its byte offset, then store it. Each + // part carries its SHA-256, and one changed on the way is refused + // (digest_mismatch) and sent again. getContextUpload: (id: string) => request("GET", urlPath`/me/submissions/${id}/context/upload`), @@ -1055,7 +1061,7 @@ export const api = rejectingSync({ part: Blob, opts?: { onProgress?: (sent: number) => void; signal?: AbortSignal }, ) => - sendWithProgress( + sendDigested( "PUT", urlPath`/me/submissions/${id}/context/upload` + `?offset=${offset}`, part, @@ -1371,6 +1377,17 @@ export function humanizeError(e: unknown): string { return t("upload_incomplete"); case "length_required": return t("length_required"); + // Every upload body carries its SHA-256 (sendDigested): bytes that hash + // differently on arrival are refused, and a file the browser can no longer + // read (changed on disk since it was picked) is never sent. + case "digest_mismatch": + return t("digest_mismatch"); + case "digest_required": + return t("digest_required"); + case "bad_digest": + return t("bad_digest"); + case "file_unreadable": + return t("file_unreadable"); // An upload sent in parts: the session is gone (cancelled, landed, idle for // 6 hours, or felis-api restarted), or the account holds four already. case "upload_not_found": diff --git a/panel/src/lib/contextUpload.test.ts b/panel/src/lib/contextUpload.test.ts index 2aa6012..d45b514 100644 --- a/panel/src/lib/contextUpload.test.ts +++ b/panel/src/lib/contextUpload.test.ts @@ -144,6 +144,21 @@ describe("uploadContext", () => { expect(calls.completeContextUpload).toHaveBeenCalledTimes(1); }); + it("sends a part again when it arrived changed, from where the server stands", async () => { + calls.getContextUpload.mockResolvedValueOnce(at(0)).mockResolvedValueOnce(at(4)); + acceptParts(); + calls.putContextPart.mockImplementationOnce(async (_id, offset: number, part: Blob) => at(offset + part.size)); + calls.putContextPart.mockRejectedValueOnce({ status: 400, code: "digest_mismatch", message: "" }); + await uploadContext("sub-1", FILE, { sleep }); + expect(await sentParts()).toEqual([ + [0, "0123"], + [4, "4567"], + [4, "4567"], + [8, "89"], + ]); + expect(calls.completeContextUpload).toHaveBeenCalledTimes(1); + }); + it("gives up at once on a refusal the next attempt cannot outlast", async () => { calls.getContextUpload.mockResolvedValue(at(0)); const refused = { status: 400, code: "bad_request", message: "context must be a gzip-compressed tarball" }; diff --git a/panel/src/lib/contextUpload.ts b/panel/src/lib/contextUpload.ts index 327fdd2..7b2e65f 100644 --- a/panel/src/lib/contextUpload.ts +++ b/panel/src/lib/contextUpload.ts @@ -29,8 +29,9 @@ export const MAX_COMPLETE_WAITS = 40; // A transient answer is one the next attempt can outlast: no response at all, a // tunnel or ingress page in place of the API's (upstream_unavailable), an uploads -// store that did not answer the budget check (uploads_store_unavailable), or a -// 409 that means "ask where the upload stands and send again". +// store that did not answer the budget check (uploads_store_unavailable), a part +// that arrived changed and was not kept (digest_mismatch), or a 409 that means +// "ask where the upload stands and send again". export function isTransient(e: unknown): boolean { const err = e as Partial | null; if (!err || typeof err.code !== "string") return false; @@ -39,6 +40,7 @@ export function isTransient(e: unknown): boolean { err.code === "upstream_unavailable" || err.code === "uploads_store_unavailable" || err.code === "upload_busy" || + err.code === "digest_mismatch" || err.code === "upload_offset_mismatch" ); } diff --git a/panel/src/lib/digest.ts b/panel/src/lib/digest.ts new file mode 100644 index 0000000..30e8de1 --- /dev/null +++ b/panel/src/lib/digest.ts @@ -0,0 +1,24 @@ +import type { ApiError } from "./types"; + +// sha256Of hashes the bytes of blob the way felis-api checks an upload body: +// `hex` is how the server lists the parts it holds, and `header` is the +// Content-Digest value (RFC 9530) a request carries it in. A file changed or +// removed on disk since it was picked cannot be read (the browser's +// NotReadableError), which rejects as file_unreadable before anything is sent. +export async function sha256Of(blob: Blob): Promise<{ hex: string; header: string }> { + let bytes: ArrayBuffer; + try { + bytes = await blob.arrayBuffer(); + } catch (e) { + const err: ApiError = { status: 0, code: "file_unreadable", message: e instanceof Error ? e.message : String(e) }; + throw err; + } + const sum = new Uint8Array(await crypto.subtle.digest("SHA-256", bytes)); + let bin = ""; + let hex = ""; + for (const b of sum) { + bin += String.fromCharCode(b); + hex += b.toString(16).padStart(2, "0"); + } + return { hex, header: `sha-256=:${btoa(bin)}:` }; +} diff --git a/panel/src/lib/openapi.gen.ts b/panel/src/lib/openapi.gen.ts index 7cfae63..7b5c8ef 100644 --- a/panel/src/lib/openapi.gen.ts +++ b/panel/src/lib/openapi.gen.ts @@ -148,12 +148,16 @@ export interface paths { }; /** * Stream one staged file upload to the Job landing it (one-time bearer token). - * @description PUT /api/v1/servers/{name}/files/upload stages the body on felis-api's disk and creates a Job to land it in the world volume; the Job fetches the bytes here. The Job holds no service token, so the route is public on the internal face and the bearer token minted with the upload is the whole check. The token opens its upload once. An unknown id, a wrong or missing token and a spent token are all the same 404, so the route says nothing about which uploads exist. An upload session committed through POST …/files/uploads/{id}/commit is fetched here the same way, under the session id; it stays staged until it has been sent whole once, so a Job that failed before then can be committed again. + * @description PUT /api/v1/servers/{name}/files/upload stages the body on felis-api's disk and creates a Job to land it in the world volume; the Job fetches the bytes here. The Job holds no service token, so the route is public on the internal face and the bearer token minted with the upload is the whole check. The token opens its upload once. An unknown id, a wrong or missing token and a spent token are all the same 404, so the route says nothing about which uploads exist. An upload session committed through POST …/files/uploads/{id}/commit is fetched here the same way, under the session id; it stays staged until its Job reports the file landed (DELETE), so a Job that failed at any point can be committed again. */ get: operations["internalFileUpload"]; put?: never; post?: never; - delete?: never; + /** + * The Job reports a staged upload landed (the same one-time bearer token). + * @description Sent once the file is in place. An upload session is then dropped from felis-api's disk; a single-request upload goes when its request ends in any case. Only the token of the session's latest commit is taken. As for the fetch, every refusal is the same 404. + */ + delete: operations["internalFileUploadLanded"]; options?: never; head?: never; patch?: never; @@ -169,7 +173,7 @@ export interface paths { get?: never; /** * Hand one export's archive over for download (one-time bearer token). - * @description The export Job PUTs the tar.gz here, chunked for a world and with its Content-Length for a backup. The Job holds no service token, so the route is public on the internal face and the bearer token minted with the export is the whole check; an unknown id, a wrong or missing token and a token already used are all the same 404. The request then waits, body unread, up to 90 seconds for the owner's browser to open the download, and is read at the browser's pace: the 16 KiB/s minimum body rate does not apply, and the body fails only after 2 minutes without a byte. It answers once the download has ended. + * @description The export Job PUTs the archive or file here, always chunked, with its size as X-Felis-Export-Length when it knows it and, once the body has ended, the SHA-256 of all it sent as the Content-Digest trailer (sha-256=::). felis-api holds the last bytes back from the browser until the bytes it received number and hash as the Job said, so a body changed on the way, or one without the trailer, ends the download short and the browser reports it failed. The Job holds no service token, so the route is public on the internal face and the bearer token minted with the export is the whole check; an unknown id, a wrong or missing token and a token already used are all the same 404. The request then waits, body unread, up to 90 seconds for the owner's browser to open the download, and is read at the browser's pace: the 16 KiB/s minimum body rate does not apply, and the body fails only after 2 minutes without a byte. It answers once the download has ended. */ put: operations["internalExportUpload"]; post?: never; @@ -1343,7 +1347,7 @@ export interface paths { }; /** * Download a ready export (once, by the user who started it). - * @description The first request spends the ticket, whatever becomes of it. The archive streams as the Job sends it, with Content-Length when it is known; a download that cannot finish (the Job died, or a backup did not match its recorded sha256) is cut off, so the browser reports it failed. HEAD is refused, since it would spend the ticket on no body. + * @description The first request spends the ticket, whatever becomes of it. The archive streams as the Job sends it, with Content-Length when it is known; a download that cannot finish (the Job died, a backup did not match its recorded sha256, or the bytes did not hash to the SHA-256 the Job sent with them) is cut off before its last bytes, so the browser reports it failed. HEAD is refused, since it would spend the ticket on no body. */ get: operations["exportDownload"]; put?: never; @@ -1492,7 +1496,7 @@ export interface paths { get?: never; /** * Upload a file into a server's world volume (owner-or-admin; server must be stopped). - * @description Lands the raw request body as the file at path, up to 64 MiB — a plugin jar, a datapack, a world region; a bigger file goes up as an upload session (POST …/files/uploads). Content-Length is required (411 length_required). An existing file is 409 file_exists unless overwrite=true; a folder at the path is 400 bad_path either way. The body is staged on felis-api's disk first and then fetched by the file Job with a one-time token, so the world lock is taken only after the body has arrived and a slow upload holds off no backup. The file lands atomically: a synced temporary sibling is checked against the staged size and SHA-256, then renamed into place, so a failed upload leaves the old file whole. Same stopped-gate and os.Root containment as a write. Audited as file.upload. + * @description Lands the raw request body as the file at path, up to 64 MiB — a plugin jar, a datapack, a world region; a bigger file goes up as an upload session (POST …/files/uploads). Content-Length is required (411 length_required). An existing file is 409 file_exists unless overwrite=true; a folder at the path is 400 bad_path either way. The body is staged on felis-api's disk first and then fetched by the file Job with a one-time token, so the world lock is taken only after the body has arrived and a slow upload holds off no backup. The file lands atomically: a synced temporary sibling is checked against the staged size and SHA-256, then renamed into place, so a failed upload leaves the old file whole. The body carries its SHA-256 as Content-Digest; felis-api checks it as the body arrives, and the Job checks the same digest again as it fetches the staged copy, so every hop between the browser and the world volume is verified. Same stopped-gate and os.Root containment as a write. Audited as file.upload. */ put: operations["uploadServerFile"]; post?: never; @@ -1536,7 +1540,7 @@ export interface paths { get: operations["getServerFileUpload"]; /** * Send one part of an upload session (owner-or-admin, the account that began it). - * @description The raw body is appended at offset, which must be where the session ends. Content-Length is required, and the part is taken whole or not at all: one cut short leaves the session where it was. Parts go one at a time (409 upload_busy while one arrives). Needs no stopped server, so starting the server midway costs only the commit's refusal until it is stopped again. + * @description The raw body is appended at offset, which must be where the session ends. Content-Length and the part's own Content-Digest are required, and the part is taken whole or not at all: one cut short, or one whose bytes do not hash to its digest, leaves the session where it was. Parts go one at a time (409 upload_busy while one arrives). Needs no stopped server, so starting the server midway costs only the commit's refusal until it is stopped again. */ put: operations["putServerFileUploadPart"]; post?: never; @@ -2362,7 +2366,7 @@ export interface paths { put?: never; /** * Upload the modpack build context for your own pending submission (user side; user-directed lane over §16). - * @description The request body IS the raw gzip build context (context.tar.gz) — not JSON, not multipart — streamed to the platform-derived, id-namespaced location Kaniko reads via --context. The submitter is taken from the principal; a submission the caller does not own is reported as 404, so this endpoint cannot upload to or probe another user's submission. Only a pending_review submission accepts a context (409 otherwise), and one withdrawn or deleted while its context streams in answers 404 with the bytes discarded; a wrong-format or oversize body is rejected with 400 (the per-upload cap is [registry] context_max_bytes, 1 GiB by default; GET /api/v1/me/submissions/limits reports it so a client can check a file before sending it). This request carries the whole context, so behind the Cloudflare edge, whose proxy refuses bodies over 100 MB with its own HTML 413 before they reach the API, a larger context goes through the chunked upload at /api/v1/me/submissions/{id}/context/upload instead. An upload that would push the caller past their per-user stored-context budget is refused with 403 before the excess is persisted. Returns 503 when the deployment's context store has no implemented upload transport. + * @description The request body IS the raw gzip build context (context.tar.gz) — not JSON, not multipart — streamed to the platform-derived, id-namespaced location Kaniko reads via --context. The submitter is taken from the principal; a submission the caller does not own is reported as 404, so this endpoint cannot upload to or probe another user's submission. Only a pending_review submission accepts a context (409 otherwise), and one withdrawn or deleted while its context streams in answers 404 with the bytes discarded; a wrong-format or oversize body is rejected with 400 (the per-upload cap is [registry] context_max_bytes, 1 GiB by default; GET /api/v1/me/submissions/limits reports it so a client can check a file before sending it). This request carries the whole context, so behind the Cloudflare edge, whose proxy refuses bodies over 100 MB with its own HTML 413 before they reach the API, a larger context goes through the chunked upload at /api/v1/me/submissions/{id}/context/upload instead. An upload that would push the caller past their per-user stored-context budget is refused with 403 before the excess is persisted. The body's SHA-256 is required as Content-Digest; bytes that do not hash to it were changed on the way, and none of them replace the context stored before. Returns 503 when the deployment's context store has no implemented upload transport. */ post: operations["uploadSubmissionContext"]; delete?: never; @@ -2385,7 +2389,7 @@ export interface paths { get: operations["getContextUpload"]; /** * Append one part of your chunked context upload. - * @description The body is the part's raw bytes, at most part_max_bytes (32 MiB). offset is where they start: 0 starts the upload over, and anything else must equal the staged length, or the answer is 409 upload_offset_mismatch and the client reads GET for where to resume. The first part must open with the gzip magic (400). The staged total meets the same context cap (400) and storage budget (403) as a single upload. A part that breaks off is cut back off, so the staged bytes are always a prefix of the file. One request per upload at a time (409 upload_busy). Staged bytes untouched for 24 hours are deleted. The budget check reads blob sizes remembered for up to a minute; when a size has to be read and the uploads store does not answer, the answer is 503 uploads_store_unavailable with Retry-After, and the same part can be sent again. + * @description The body is the part's raw bytes, at most part_max_bytes (32 MiB). offset is where they start: 0 starts the upload over, and anything else must equal the staged length, or the answer is 409 upload_offset_mismatch and the client reads GET for where to resume. The first part must open with the gzip magic (400). The staged total meets the same context cap (400) and storage budget (403) as a single upload. A part that breaks off is cut back off, and so is one whose bytes do not hash to its Content-Digest, so the staged bytes are always a prefix of the file. One request per upload at a time (409 upload_busy). Staged bytes untouched for 24 hours are deleted. The budget check reads blob sizes remembered for up to a minute; when a size has to be read and the uploads store does not answer, the answer is 503 uploads_store_unavailable with Retry-After, and the same part can be sent again. */ put: operations["putContextUploadPart"]; post?: never; @@ -3054,6 +3058,12 @@ export interface components { * @description The most one part may carry. */ part_max_bytes: number; + /** @description The parts taken so far, in order, each with the SHA-256 it arrived with. A client resuming from a file it still holds hashes the same ranges and starts over when one differs. */ + parts: { + /** Format: int64 */ + size: number; + sha256: string; + }[]; }; StartFileOp: { /** @description Replace files already there. */ @@ -3817,12 +3827,32 @@ export interface operations { 404: components["responses"]["NotFound"]; }; }; + internalFileUploadLanded: { + parameters: { + query?: never; + header: { + /** @description Bearer followed by the token the Job fetched the upload with. */ + Authorization: string; + }; + path: { + id: string; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + 204: components["responses"]["NoContent"]; + 404: components["responses"]["NotFound"]; + }; + }; internalExportUpload: { parameters: { query?: never; header: { /** @description Bearer followed by the token minted with the export. */ Authorization: string; + /** @description The body's length in bytes, when the Job knows it; the download then carries it as Content-Length. */ + "X-Felis-Export-Length"?: number; }; path: { id: string; @@ -3836,6 +3866,15 @@ export interface operations { }; responses: { 204: components["responses"]["NoContent"]; + /** @description X-Felis-Export-Length is not a byte count (bad_request); the token is not spent. */ + 400: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["Error"]; + }; + }; 404: components["responses"]["NotFound"]; /** @description The backup did not match the sha256 recorded when it was written, and the download was aborted (backup_corrupt). */ 409: { @@ -3846,7 +3885,7 @@ export interface operations { "application/json": components["schemas"]["Error"]; }; }; - /** @description Nobody opened the download within 90 seconds, or the browser left before the archive ended (export_expired). */ + /** @description Nobody opened the download within 90 seconds, the browser left before the archive ended, or what arrived did not number or hash as the Job declared, so the download was cut off (export_expired). */ 410: { headers: { [name: string]: unknown; @@ -6800,7 +6839,7 @@ export interface operations { * Format: int64 * @description Bytes free on the world volume */ - free_bytes: number; + free_bytes: number | null; entries: { name: string; /** Format: int64 */ @@ -7325,7 +7364,10 @@ export interface operations { /** @description true replaces an existing file, keeping its mode. Anything else refuses to. */ overwrite?: "true" | "false"; }; - header?: never; + header: { + /** @description The SHA-256 of the body as RFC 9530 sends it, sha-256=::. Other algorithms listed beside it are ignored. Bytes that do not hash to it were changed on the way and are refused whole. */ + "Content-Digest": string; + }; path: { name: string; }; @@ -7357,7 +7399,7 @@ export interface operations { }; }; }; - /** @description Missing path, invalid server name, a folder or the world root at the path, a path that escapes the world root, or a body that ended before Content-Length bytes arrived (upload_incomplete). */ + /** @description Missing path, invalid server name, a folder or the world root at the path, a path that escapes the world root, a body that ended before Content-Length bytes arrived (upload_incomplete), no Content-Digest (digest_required), a malformed one (bad_digest), or bytes that do not hash to it (digest_mismatch; nothing is staged, so send it again). */ 400: { headers: { [name: string]: unknown; @@ -7558,7 +7600,10 @@ export interface operations { /** @description The byte position the part starts at, the session's received. */ offset: number; }; - header?: never; + header: { + /** @description The SHA-256 of the body as RFC 9530 sends it, sha-256=::. Other algorithms listed beside it are ignored. Bytes that do not hash to it were changed on the way and are refused whole. */ + "Content-Digest": string; + }; path: { name: string; id: string; @@ -7580,7 +7625,7 @@ export interface operations { "application/json": components["schemas"]["FileUploadSession"]; }; }; - /** @description A missing or malformed offset (bad_request), a body that ended before its Content-Length (upload_incomplete), or a malformed server name (bad_name). */ + /** @description A missing or malformed offset (bad_request), a body that ended before its Content-Length (upload_incomplete), no Content-Digest (digest_required), a malformed one (bad_digest), bytes that do not hash to it (digest_mismatch; the part was not taken, so send it again), or a malformed server name (bad_name). */ 400: { headers: { [name: string]: unknown; @@ -9728,7 +9773,10 @@ export interface operations { uploadSubmissionContext: { parameters: { query?: never; - header?: never; + header: { + /** @description The SHA-256 of the body as RFC 9530 sends it, sha-256=::. Other algorithms listed beside it are ignored. */ + "Content-Digest": string; + }; path: { id: string; }; @@ -9749,7 +9797,15 @@ export interface operations { "application/json": components["schemas"]["Submission"]; }; }; - 400: components["responses"]["BadRequest"]; + /** @description A body that is not a gzip tarball or is over the context cap (bad_request), no Content-Digest (digest_required), a malformed one (bad_digest), or bytes that do not hash to it (digest_mismatch; nothing was stored, so send it again). */ + 400: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["Error"]; + }; + }; 401: components["responses"]["Unauthorized"]; /** @description The upload would exceed the caller's per-user stored-context budget (submission_quota_exceeded), which counts their pending and rejected uploads; approved ones leave it. */ 403: { @@ -9801,7 +9857,10 @@ export interface operations { query: { offset: number; }; - header?: never; + header: { + /** @description The SHA-256 of the part as RFC 9530 sends it, sha-256=::. Other algorithms listed beside it are ignored. */ + "Content-Digest": string; + }; path: { id: string; }; @@ -9822,7 +9881,15 @@ export interface operations { "application/json": components["schemas"]["ContextUploadProgress"]; }; }; - 400: components["responses"]["BadRequest"]; + /** @description A missing or malformed offset, a first part without the gzip magic or a total over the context cap (bad_request), no Content-Digest (digest_required), a malformed one (bad_digest), or bytes that do not hash to it (digest_mismatch; the part was cut back off, so read where the upload stands and send it again). */ + 400: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["Error"]; + }; + }; 401: components["responses"]["Unauthorized"]; /** @description The staged total would exceed the caller's per-user stored-context budget (submission_quota_exceeded). */ 403: { diff --git a/panel/src/lib/types.ts b/panel/src/lib/types.ts index 8bf0f83..949e41a 100644 --- a/panel/src/lib/types.ts +++ b/panel/src/lib/types.ts @@ -420,13 +420,15 @@ export interface ServerFileEntry { /** FileUploadSession is where an upload sent in parts stands * (internal/api/handlers_fileops.go fileSessionView): the next part starts at - * `received` and carries at most `part_max_bytes`. */ + * `received` and carries at most `part_max_bytes`. `parts` are the parts taken + * so far, in order, each with the SHA-256 (hex) it arrived with. */ export interface FileUploadSession { id: string; path: string; size: number; received: number; part_max_bytes: number; + parts: { size: number; sha256: string }[]; } /** FileOp is one background upload landing or extraction diff --git a/panel/src/pages/ServerFiles.test.tsx b/panel/src/pages/ServerFiles.test.tsx index 51edea8..3000666 100644 --- a/panel/src/pages/ServerFiles.test.tsx +++ b/panel/src/pages/ServerFiles.test.tsx @@ -1,4 +1,5 @@ // @vitest-environment jsdom +import { createHash } from "node:crypto"; import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { act, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; @@ -10,7 +11,7 @@ import { ONE_REQUEST_BYTES } from "@/components/files/useUploads"; import { OP_POLL_MS } from "@/components/files/sessionUpload"; import { EXPORT_POLL_MS } from "@/lib/download"; import { STATUS_POLL_FAST_MS } from "@/lib/hooks"; -import type { FileOp } from "@/lib/types"; +import type { FileOp, ServerJob } from "@/lib/types"; const mocks = vi.hoisted(() => ({ writeServerFile: vi.fn(), @@ -33,6 +34,7 @@ const mocks = vi.hoisted(() => ({ putServerFileUploadPart: vi.fn(), deleteServerFileUpload: vi.fn(), commitServerFileUpload: vi.fn(), + serverJobs: vi.fn(), })); vi.mock("@/lib/api", async (importOriginal) => { @@ -61,6 +63,7 @@ vi.mock("@/lib/api", async (importOriginal) => { putServerFileUploadPart: mocks.putServerFileUploadPart, deleteServerFileUpload: mocks.deleteServerFileUpload, commitServerFileUpload: mocks.commitServerFileUpload, + serverJobs: mocks.serverJobs, }, }; }); @@ -86,6 +89,8 @@ async function openEditor() { } let opsNow: FileOp[] = []; +// The server's backup, restore and export Jobs, as each read finds them. +let jobsNow: ServerJob[] = []; const fileOp = (over: Partial = {}): FileOp => ({ id: "op1", op: "unzip", @@ -136,6 +141,9 @@ beforeEach(() => { opsNow = []; mocks.listServerFileOps.mockReset(); mocks.listServerFileOps.mockImplementation(async () => ({ ops: opsNow })); + jobsNow = []; + mocks.serverJobs.mockReset(); + mocks.serverJobs.mockImplementation(async () => jobsNow); localStorage.clear(); mocks.stop.mockReset(); mocks.status.mockReset(); @@ -679,7 +687,7 @@ describe("ServerFiles uploads", () => { Object.defineProperty(f, "size", { value: size }); return f; }; - const withFree = (free: number) => + const withFree = (free: number | null) => mocks.listServerFiles.mockImplementation((_name: string, path: string) => Promise.resolve({ path, @@ -700,6 +708,18 @@ describe("ServerFiles uploads", () => { expect(mocks.uploadServerFile).not.toHaveBeenCalled(); }); + it("refuses a name past 255 bytes without sending it, and sends one at 255", async () => { + renderFiles(); + await screen.findByText("world"); + + // A Mac counts 86 characters, the server 258 bytes; 85 of them are 255. + pick(file("界".repeat(86)), file("界".repeat(85))); + + expect(await within(queue()).findByText(t("files:upload_name_too_long"))).toBeTruthy(); + expect(within(queue()).queryByRole("button", { name: t("files:upload_retry") })).toBeNull(); + await waitFor(() => expect(sentAs()).toEqual([["界".repeat(85), false]])); + }); + it("refuses what the volume has no room for, counting the files ahead in the batch, and retries once there is room", async () => { withFree(100); renderFiles(); @@ -720,7 +740,7 @@ describe("ServerFiles uploads", () => { }); it("sends when the listing could not tell how much room there is", async () => { - withFree(0); + withFree(null); renderFiles(); await screen.findByText("world"); @@ -729,6 +749,17 @@ describe("ServerFiles uploads", () => { await waitFor(() => expect(sentAs()).toEqual([["a.jar", false]])); }); + it("refuses everything but an empty file on a full volume", async () => { + withFree(0); + renderFiles(); + await screen.findByText("world"); + + pick(sized("a.jar", 1), sized("empty.txt", 0)); + + expect(await within(queue()).findByText(i18next.t("files:upload_no_room", { free: "0 B", size: "1 B" }))).toBeTruthy(); + await waitFor(() => expect(sentAs()).toEqual([["empty.txt", false]])); + }); + it("keeps a retry refused while the latest listing has no room for the file", async () => { withFree(10); renderFiles(); @@ -759,7 +790,24 @@ describe("ServerFiles uploads", () => { const PART = 32 * 1024 * 1024; const SIZE = ONE_REQUEST_BYTES + 1; const KEY = "felis-file-upload:lobby:world.zip"; - const at = (received: number) => ({ id: "s1", path: "world.zip", size: SIZE, received, part_max_bytes: PART }); + // sized() keeps the five bytes file() made, and a slice past them is empty: + // those are the bytes a resume hashes the held parts against. + const held = (received: number) => { + const out: { size: number; sha256: string }[] = []; + for (let start = 0; start < received; start += PART) { + const end = Math.min(start + PART, received); + out.push({ size: end - start, sha256: createHash("sha256").update("bytes".slice(start, end)).digest("hex") }); + } + return out; + }; + const at = (received: number) => ({ + id: "s1", + path: "world.zip", + size: SIZE, + received, + part_max_bytes: PART, + parts: held(received), + }); let parts: { offset: number; signal?: AbortSignal }[]; beforeEach(() => { @@ -1425,6 +1473,15 @@ describe("ServerFiles archives and downloads", () => { () => i18next.t("files:download_failed_because", { reason: "tar: world: Cannot open" }), ], ["failed without one", () => mocks.exportStatus.mockResolvedValue({ state: "failed" }), () => t("files:download_failed")], + [ + "killed for memory", + () => + mocks.exportStatus.mockResolvedValue({ + state: "failed", + message: "the job ran out of memory and the system stopped it (OOMKilled)", + }), + () => t("files:download_out_of_memory"), + ], ])("says why a download could not be prepared when %s", async (_label, arrange, words) => { const clicked = watchLinks(); mocks.downloadServerFile.mockResolvedValue({ ticket: "t1", state: "pending", filename: "world.zip" }); @@ -1444,3 +1501,118 @@ describe("ServerFiles archives and downloads", () => { expect(button("download_folder_item", "world").disabled).toBe(false); }); }); + +describe("ServerFiles with the world held elsewhere", () => { + const job = (over: Partial): ServerJob => ({ name: "j1", kind: "backup", state: "running", ...over }); + const poll = () => act(() => vi.advanceTimersByTimeAsync(OP_POLL_MS)); + // The notice sits in a status region; other messages can be status regions too. + const notice = (key: string) => screen.queryByText(t(key)); + const shown = async (key: string) => (await screen.findByText(t(key))).closest('[role="status"]') !== null; + + it("holds every change while a backup runs, says so, and lets go once it ends", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + jobsNow = [job({ kind: "backup" })]; + mocks.uploadServerFile.mockResolvedValue({ path: "a.jar", status: "uploaded", sha256: "a", size: 5 }); + renderFiles(); + await screen.findByText("server.properties"); + + expect(await shown("files:wait_for_backup")).toBe(true); + for (const b of [button("new_file"), button("new_folder"), button("delete_item", "world")]) { + expect(b.disabled).toBe(true); + expect(b.title).toBe(t("files:wait_for_backup")); + } + fireEvent.change(screen.getByTestId("upload-input"), { target: { files: [new File(["bytes"], "a.jar")] } }); + await poll(); + expect(mocks.uploadServerFile).not.toHaveBeenCalled(); + const lists = mocks.listServerFiles.mock.calls.length; + + jobsNow = [job({ kind: "backup", state: "succeeded" })]; + await poll(); + + await waitFor(() => expect(notice("files:wait_for_backup")).toBeNull()); + expect(button("new_folder").disabled).toBe(false); + await waitFor(() => expect(mocks.uploadServerFile.mock.calls.map((c) => c[1])).toEqual(["a.jar"])); + // A backup changed nothing, so only the upload's own landing rereads. + expect(mocks.listServerFiles.mock.calls.length).toBe(lists + 1); + }); + + it("rereads the folder once a restore ends", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + jobsNow = [job({ kind: "restore" })]; + renderFiles(); + await screen.findByText("server.properties"); + expect(await shown("files:wait_for_restore")).toBe(true); + const lists = mocks.listServerFiles.mock.calls.length; + + jobsNow = [job({ kind: "restore", state: "succeeded" })]; + await poll(); + + await waitFor(() => expect(mocks.listServerFiles.mock.calls.length).toBe(lists + 1)); + expect(notice("files:wait_for_restore")).toBeNull(); + }); + + it.each([ + ["a safety snapshot whose restore has yet to start", job({ kind: "backup", state: "succeeded", then_restore: "pending" }), "files:wait_for_restore"], + ["a world export", job({ kind: "export_world" }), "files:wait_for_world_export"], + ["a file download", job({ kind: "export_files" }), "files:wait_for_file_download"], + ])("names %s as what holds it", async (_what, holder, text) => { + jobsNow = [job({ name: "old", kind: "restore", state: "failed" }), holder]; + renderFiles(); + await screen.findByText("server.properties"); + + expect(await shown(text)).toBe(true); + expect(button("new_file").title).toBe(t(text)); + }); + + it("is not held by a backup being downloaded, which reads only the backup store", async () => { + jobsNow = [job({ kind: "export_backup" })]; + renderFiles(); + await screen.findByText("server.properties"); + await waitFor(() => expect(mocks.serverJobs).toHaveBeenCalled()); + + for (const key of ["wait_for_backup", "wait_for_restore", "wait_for_world_export", "wait_for_file_download"]) expect(notice(`files:${key}`)).toBeNull(); + expect(button("new_file").disabled).toBe(false); + }); + + it.each([ + ["an extraction", "unzip_item", mocks.unzipServerFile, humanizeError], + ["a download", "download_item", mocks.downloadServerFile, (e: unknown) => i18next.t("files:download_failed_because", { reason: humanizeError(e) })], + ] as const)("looks again when %s is refused because the world is held", async (_what, key, call, said) => { + mocks.listServerFiles.mockImplementation((_name: string, path: string) => + Promise.resolve({ path, truncated: false, entries: [{ name: "pack.zip", size: 8, is_dir: false, mod_time: "2026-09-01T00:00:00Z" }] }), + ); + const held = { status: 409, code: "maintenance_in_progress", message: "a world export or file download is running" }; + call.mockRejectedValueOnce(held); + renderFiles(); + await screen.findByText("pack.zip"); + await waitFor(() => expect(mocks.serverJobs).toHaveBeenCalledTimes(1)); + jobsNow = [job({ kind: "export_world" })]; + + fireEvent.click(button(key, "pack.zip")); + + expect(await shown("files:wait_for_world_export")).toBe(true); + expect(screen.getByText(said(held))).toBeTruthy(); + expect(button(key, "pack.zip").disabled).toBe(true); + }); + + it("holds changes while its own download streams to the browser", async () => { + vi.useFakeTimers({ shouldAdvanceTime: true }); + vi.spyOn(HTMLAnchorElement.prototype, "click").mockImplementation(() => {}); + mocks.downloadServerFile.mockResolvedValue({ ticket: "t1", state: "pending", filename: "server.properties" }); + mocks.exportStatus.mockResolvedValue({ state: "ready" }); + mocks.exportDownloadURL.mockResolvedValue("/api/v1/exports/t1/download"); + renderFiles(); + await screen.findByText("server.properties"); + await waitFor(() => expect(mocks.serverJobs).toHaveBeenCalledTimes(1)); + jobsNow = [job({ kind: "export_files" })]; + + fireEvent.click(button("download_item", "server.properties")); + await waitFor(() => expect(mocks.downloadServerFile).toHaveBeenCalledTimes(1)); + expect(notice("files:wait_for_file_download")).toBeNull(); + await act(() => vi.advanceTimersByTimeAsync(EXPORT_POLL_MS)); + expect(mocks.exportDownloadURL).toHaveBeenCalledTimes(1); + + expect(await shown("files:wait_for_file_download")).toBe(true); + expect(button("new_file").disabled).toBe(true); + }); +}); diff --git a/panel/src/pages/ServerFiles.tsx b/panel/src/pages/ServerFiles.tsx index 2dcab63..52e2840 100644 --- a/panel/src/pages/ServerFiles.tsx +++ b/panel/src/pages/ServerFiles.tsx @@ -31,6 +31,7 @@ import { FileOps } from "@/components/files/FileOps"; import { UploadQueue } from "@/components/files/UploadQueue"; import { useFileOps } from "@/components/files/useFileOps"; import { useUploads } from "@/components/files/useUploads"; +import { holderText, useWorldJobs } from "@/components/files/useWorldJobs"; import { SECRET_CONFIG_PATH, isManaged, @@ -133,7 +134,7 @@ export function ServerFiles() { const [listErr, setListErr] = useState(null); const [listLoading, setListLoading] = useState(false); // Bytes free on the world volume by the latest listing; null while unknown - // (the Job reports 0 when it could not tell). + // (the listing says null when the Job could not tell; 0 is a full volume). const [free, setFree] = useState(null); const [msg, setMsg] = useState<{ kind: "success" | "error"; text: string } | null>(null); @@ -151,7 +152,7 @@ export function ServerFiles() { if (ticket !== loadSeq.current) return; setEntries(sortEntries(r.entries ?? [])); setTruncated(r.truncated === true); - setFree(r.free_bytes > 0 ? r.free_bytes : null); + setFree(r.free_bytes ?? null); setDir(p); } catch (e) { if (ticket !== loadSeq.current) return; @@ -308,6 +309,10 @@ export function ServerFiles() { } } const fileOps = useFileOps(name, owned && stopped, opEnded); + // Backups, restores, world exports and downloads hold the world as well, + // whichever tab or person started them. A restore replaces the files listed. + const worldJobs = useWorldJobs(name, owned && stopped, () => void load(dir)); + const held = worldJobs.holder; // A download holds the world while felis-api gets it ready, and a change // sent meanwhile could only be refused. @@ -329,20 +334,22 @@ export function ServerFiles() { landedSince.current = true; }, onOp: fileOps.ignore, - hold: fileOps.running || downloading !== null || unzipping !== null, + hold: fileOps.running || downloading !== null || unzipping !== null || held !== null, free, }); // Each change is a Job holding the world lock, so while anything else holds // it a change could only be refused. - const changing = uploads.busy || fileOps.running || downloading !== null || unzipping !== null; - // Names what holds the lock now: queued uploads wait on an op or a download - // too, so those come first. + const changing = uploads.busy || fileOps.running || downloading !== null || unzipping !== null || held !== null; + // Names what holds the lock now: queued uploads wait on the rest too, so + // those come first. const waitTitle = fileOps.running || unzipping !== null ? t("wait_for_op") : downloading !== null ? t("wait_for_download") - : t("wait_for_uploads"); + : held !== null + ? t(holderText(held)) + : t("wait_for_uploads"); useEffect(() => { if (uploads.busy || !landedSince.current) return; landedSince.current = false; @@ -456,6 +463,12 @@ export function ServerFiles() { if (op.state !== "running") opEnded(op); } + // A refusal because the world is held means something this page has not seen + // holds it: reading the Jobs again names it and holds the buttons. + function heldElsewhere(e: unknown) { + if ((e as { code?: string }).code === "maintenance_in_progress") worldJobs.refresh(); + } + async function startUnzip(entry: ServerFileEntry) { const p = joinPath(dir, entry.name); setMsg(null); @@ -463,6 +476,7 @@ export function ServerFiles() { try { await unzip(p, false); } catch (e) { + heldElsewhere(e); setMsg({ kind: "error", text: humanizeError(e) }); } finally { setUnzipping(null); @@ -488,13 +502,23 @@ export function ServerFiles() { const s = await awaitExport(tk.ticket, () => alive.current); if (s === null) return; if (s.state === "failed") { + // A folder of more files than the zipping Job has memory to list gets it + // killed, and felis-api names that OOMKilled; a single file streams through + // in constant memory. + const oom = s.message?.includes("OOMKilled"); setMsg({ kind: "error", - text: s.message ? t("download_failed_because", { reason: s.message }) : t("download_failed"), + text: oom + ? t("download_out_of_memory") + : s.message + ? t("download_failed_because", { reason: s.message }) + : t("download_failed"), }); return; } saveDownload(await api.exportDownloadURL(tk.ticket), tk.filename); + // Its Job holds the world until the browser has all of it. + worldJobs.refresh(); const note = p === "server.properties" ? "download_started_props" @@ -504,6 +528,7 @@ export function ServerFiles() { setMsg({ kind: "success", text: t(note, { filename: tk.filename }) }); } catch (e) { if (!alive.current) return; + heldElsewhere(e); setMsg({ kind: "error", text: @@ -734,6 +759,15 @@ export function ServerFiles() { + {held !== null && ( +
+

+ + {t(holderText(held))} +

+
+ )} +