diff --git a/b2/live_only_calls_wire_test.go b/b2/live_only_calls_wire_test.go new file mode 100644 index 00000000..1c352190 --- /dev/null +++ b/b2/live_only_calls_wire_test.go @@ -0,0 +1,244 @@ +package b2 + +import ( + "context" + "errors" + "io" + "net/http" + "testing" + "time" + + "github.com/Backblaze/blazer/base" + "github.com/Backblaze/blazer/internal/b2wiretest" +) + +// Wire fields below follow https://www.backblaze.com/apidocs/b2-hide-file +// and https://www.backblaze.com/apidocs/b2-delete-file-version. +func TestObjectHideWire(t *testing.T) { + t.Run("sends name and bucket", func(t *testing.T) { + srv, bucket := liveOnlyBucket(t) + srv.Handle("gone", liveOnlyHead("file-id", "gone")) + srv.Reply("b2_hide_file", http.StatusOK, `{"fileId":"hide-id","action":"hide","uploadTimestamp":0}`) + if err := bucket.Object("gone").Hide(context.Background()); err != nil { + t.Fatal(err) + } + calls := srv.Calls("b2_hide_file") + if len(calls) != 1 { + t.Fatalf("b2_hide_file calls = %d, want 1", len(calls)) + } + b2wiretest.AssertJSON(t, calls[0], "bucketId", "bid") + b2wiretest.AssertJSON(t, calls[0], "fileName", "gone") + }) + t.Run("returns service errors", func(t *testing.T) { + srv, bucket := liveOnlyBucket(t) + srv.Handle("gone", liveOnlyHead("file-id", "gone")) + srv.Reply("b2_hide_file", http.StatusBadRequest, `{"status":400,"code":"bad_request","message":"cannot hide"}`) + err := bucket.Object("gone").Hide(context.Background()) + if status, code, message := base.MsgCode(err); status != 400 || code != "bad_request" || message != "cannot hide" { + t.Errorf("Hide error = %v (status=%d code=%q message=%q), want 400 bad_request: cannot hide", err, status, code, message) + } + if calls := srv.Calls("b2_hide_file"); len(calls) != 1 { + t.Errorf("b2_hide_file calls = %d, want 1", len(calls)) + } + }) +} + +func TestBucketRevealWire(t *testing.T) { + t.Run("deletes newest hide marker", func(t *testing.T) { + srv, bucket := liveOnlyBucket(t) + // b2_list_file_versions returns versions in newest-first order. + srv.Reply("b2_list_file_versions", http.StatusOK, `{"files":[{"fileId":"hide-id","fileName":"gone","action":"hide","uploadTimestamp":2},{"fileId":"upload-id","fileName":"gone","action":"upload","uploadTimestamp":1}],"nextFileName":"","nextFileId":""}`) + srv.Reply("b2_delete_file_version", http.StatusOK, `{}`) + if err := bucket.Reveal(context.Background(), "gone"); err != nil { + t.Fatal(err) + } + listCalls := srv.Calls("b2_list_file_versions") + if len(listCalls) != 1 { + t.Fatalf("b2_list_file_versions calls = %d, want 1", len(listCalls)) + } + b2wiretest.AssertJSON(t, listCalls[0], "prefix", "gone") + deleteCalls := srv.Calls("b2_delete_file_version") + if len(deleteCalls) != 1 { + t.Fatalf("b2_delete_file_version calls = %d, want 1", len(deleteCalls)) + } + b2wiretest.AssertJSON(t, deleteCalls[0], "fileId", "hide-id") + b2wiretest.AssertJSON(t, deleteCalls[0], "fileName", "gone") + }) + t.Run("leaves a current upload alone", func(t *testing.T) { + srv, bucket := liveOnlyBucket(t) + // b2_list_file_versions returns versions in newest-first order. + srv.Reply("b2_list_file_versions", http.StatusOK, `{"files":[{"fileId":"upload-id","fileName":"gone","action":"upload","uploadTimestamp":2},{"fileId":"hide-id","fileName":"gone","action":"hide","uploadTimestamp":1}],"nextFileName":"","nextFileId":""}`) + srv.Reply("b2_delete_file_version", http.StatusOK, `{}`) + if err := bucket.Reveal(context.Background(), "gone"); err != nil { + t.Fatal(err) + } + calls := srv.Calls("b2_list_file_versions") + if len(calls) != 1 { + t.Fatalf("b2_list_file_versions calls = %d, want 1", len(calls)) + } + b2wiretest.AssertJSON(t, calls[0], "prefix", "gone") + if calls := srv.Calls("b2_delete_file_version"); len(calls) != 0 { + t.Fatalf("b2_delete_file_version calls = %d, want 0", len(calls)) + } + }) +} + +// Request fields follow https://www.backblaze.com/apidocs/b2-get-download-authorization. +// URL arguments follow https://www.backblaze.com/docs/cloud-storage-download-authorization. +func TestBucketAuthTokenWire(t *testing.T) { + srv, bucket := liveOnlyBucket(t) + srv.Reply("b2_get_download_authorization", http.StatusOK, `{"authorizationToken":"download-token"}`) + token, err := bucket.AuthToken(context.Background(), "p/", time.Hour) + if err != nil { + t.Fatal(err) + } + if token != "download-token" { + t.Errorf("token = %q, want %q", token, "download-token") + } + request := srv.Last(t, "b2_get_download_authorization") + b2wiretest.AssertJSON(t, request, "bucketId", "bid") + b2wiretest.AssertJSON(t, request, "fileNamePrefix", "p/") + b2wiretest.AssertJSON(t, request, "validDurationInSeconds", 3600) + b2wiretest.AssertNoJSON(t, request, "b2ContentDisposition") +} + +func TestObjectAuthURLWire(t *testing.T) { + srv, bucket := liveOnlyBucket(t) + srv.Reply("b2_get_download_authorization", http.StatusOK, `{"authorizationToken":"download-token"}`) + const ( + objectName = "folder/a b.txt" + disposition = `attachment; filename="a b.txt"` + ) + object := bucket.Object(objectName) + u, err := object.AuthURL(context.Background(), time.Hour, disposition) + if err != nil { + t.Fatal(err) + } + if got := u.Query().Get("Authorization"); got != "download-token" { + t.Errorf("Authorization = %q, want %q", got, "download-token") + } + if got := u.Query().Get("b2ContentDisposition"); got != disposition { + t.Errorf("b2ContentDisposition = %q, want %q", got, disposition) + } + request := srv.Last(t, "b2_get_download_authorization") + b2wiretest.AssertJSON(t, request, "fileNamePrefix", objectName) + b2wiretest.AssertJSON(t, request, "b2ContentDisposition", disposition) + u, err = object.AuthURL(context.Background(), time.Hour, "") + if err != nil { + t.Fatal(err) + } + if _, ok := u.Query()["b2ContentDisposition"]; ok { + t.Errorf("empty disposition URL query = %q, want b2ContentDisposition omitted", u.RawQuery) + } + if got := u.Query().Get("Authorization"); got != "download-token" { + t.Errorf("empty disposition Authorization = %q, want %q", got, "download-token") + } + request = srv.Last(t, "b2_get_download_authorization") + b2wiretest.AssertNoJSON(t, request, "b2ContentDisposition") +} + +// Request and response fields follow https://www.backblaze.com/apidocs/b2-list-keys +// and https://www.backblaze.com/apidocs/b2-delete-key. +func TestClientListKeysAndDeleteWire(t *testing.T) { + srv := b2wiretest.New(t) + client := liveOnlyClient(t, srv) + // #244's null expiry mapping is fixed by #250; this test does not assert it. + srv.ReplySequence("b2_list_keys", + b2wiretest.Response{Status: http.StatusOK, Body: `{"keys":[{"applicationKeyId":"k-1","keyName":"first","capabilities":["listBuckets"],"expirationTimestamp":1700000000123},{"applicationKeyId":"k-2","keyName":"second","capabilities":["readFiles"],"expirationTimestamp":null}],"nextApplicationKeyId":"k-3"}`}, + b2wiretest.Response{Status: http.StatusOK, Body: `{"keys":[]}`}, + ) + srv.Reply("b2_delete_key", http.StatusOK, `{}`) + keys, next, err := client.ListKeys(context.Background(), 10, "") + if err != nil { + t.Fatal(err) + } + if len(keys) != 2 || next != "k-3" { + t.Fatalf("ListKeys = %d keys, %q, want 2 keys, k-3", len(keys), next) + } + // Capabilities mapping is tracked separately in #220. + if keys[0].ID() != "k-1" || keys[0].Name() != "first" || keys[0].Expires().UnixMilli() != 1700000000123 { + t.Errorf("first key = id=%q name=%q expires=%d", keys[0].ID(), keys[0].Name(), keys[0].Expires().UnixMilli()) + } + if keys[1].ID() != "k-2" || keys[1].Name() != "second" { + t.Errorf("second key = id=%q name=%q, want k-2, second", keys[1].ID(), keys[1].Name()) + } + request := srv.Last(t, "b2_list_keys") + b2wiretest.AssertJSON(t, request, "accountId", "a") + b2wiretest.AssertJSON(t, request, "maxKeyCount", 10) + b2wiretest.AssertNoJSON(t, request, "startApplicationKeyId") + if _, _, err := client.ListKeys(context.Background(), 10, next); !errors.Is(err, io.EOF) { + t.Errorf("second ListKeys error = %v, want io.EOF", err) + } + request = srv.Last(t, "b2_list_keys") + b2wiretest.AssertJSON(t, request, "startApplicationKeyId", "k-3") + if err := keys[0].Delete(context.Background()); err != nil { + t.Fatal(err) + } + request = srv.Last(t, "b2_delete_key") + b2wiretest.AssertJSON(t, request, "applicationKeyId", "k-1") +} + +// Request fields follow https://www.backblaze.com/apidocs/b2-cancel-large-file. +func TestObjectCancelWire(t *testing.T) { + t.Run("unfinished file", func(t *testing.T) { + srv, bucket := liveOnlyBucket(t) + srv.Reply("b2_list_unfinished_large_files", http.StatusOK, `{"files":[{"fileId":"large-id","fileName":"pending","action":"start","uploadTimestamp":0}],"nextFileId":""}`) + srv.Reply("b2_cancel_large_file", http.StatusOK, `{}`) + iter := bucket.List(context.Background(), ListUnfinished()) + if !iter.Next() { + t.Fatalf("unfinished list returned no object: %v", iter.Err()) + } + if err := iter.Object().Cancel(context.Background()); err != nil { + t.Fatal(err) + } + request := srv.Last(t, "b2_cancel_large_file") + b2wiretest.AssertJSON(t, request, "fileId", "large-id") + }) + t.Run("uploaded file", func(t *testing.T) { + srv, bucket := liveOnlyBucket(t) + srv.Reply("b2_list_file_names", http.StatusOK, `{"files":[{"fileId":"upload-id","fileName":"complete","action":"upload","uploadTimestamp":0}],"nextFileName":""}`) + srv.Reply("b2_cancel_large_file", http.StatusOK, `{}`) + iter := bucket.List(context.Background()) + if !iter.Next() { + t.Fatalf("current list returned no object: %v", iter.Err()) + } + const want = "complete is not an unfinished large file: upload" + if err := iter.Object().Cancel(context.Background()); err == nil || err.Error() != want { + t.Errorf("Cancel uploaded file error = %v, want %q", err, want) + } + if calls := srv.Calls("b2_cancel_large_file"); len(calls) != 0 { + t.Fatalf("b2_cancel_large_file calls = %d, want 0", len(calls)) + } + }) +} + +func liveOnlyClient(t *testing.T, srv *b2wiretest.Server) *Client { + t.Helper() + client, err := NewClient(context.Background(), "a", "k", APIBase(srv.URL())) + if err != nil { + t.Fatal(err) + } + return client +} + +func liveOnlyBucket(t *testing.T) (*b2wiretest.Server, *Bucket) { + t.Helper() + srv := b2wiretest.New(t) + srv.Reply("b2_list_buckets", http.StatusOK, `{"buckets":[]}`) + srv.Reply("b2_create_bucket", http.StatusOK, updateBucketJSON) + bucket, err := liveOnlyClient(t, srv).NewBucket(context.Background(), "n", nil) + if err != nil { + t.Fatal(err) + } + return srv, bucket +} + +func liveOnlyHead(id, name string) func(b2wiretest.Request) b2wiretest.Response { + return func(b2wiretest.Request) b2wiretest.Response { + return b2wiretest.Response{Status: http.StatusOK, Header: http.Header{ + "Content-Length": {"0"}, + "X-Bz-File-Id": {id}, + "X-Bz-File-Name": {name}, + }} + } +} diff --git a/base/live_only_calls_test.go b/base/live_only_calls_test.go new file mode 100644 index 00000000..490f6569 --- /dev/null +++ b/base/live_only_calls_test.go @@ -0,0 +1,216 @@ +package base + +import ( + "context" + "crypto/tls" + "crypto/x509" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" +) + +func TestUploadHangRequestsNewUploadURL(t *testing.T) { + t.Run("shared upload token refusal", func(t *testing.T) { + firstEntered := make(chan struct{}) + releaseFirst := make(chan struct{}) + firstDone := make(chan error, 1) + var releaseOnce sync.Once + release := func() { releaseOnce.Do(func() { close(releaseFirst) }) } + var uploadCount int + var uploadMu sync.Mutex + var srv *httptest.Server + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/b2api/v4/b2_authorize_account": + _, _ = io.WriteString(w, v4AuthJSON(srv.URL, nil, nil, "")) + case "/upload": + uploadMu.Lock() + uploadCount++ + call := uploadCount + uploadMu.Unlock() + if _, err := io.Copy(io.Discard, r.Body); err != nil { + t.Errorf("read upload body: %v", err) + return + } + switch call { + case 1: + close(firstEntered) + <-releaseFirst + _, _ = fmt.Fprint(w, `{"fileId":"first","fileName":"name","contentLength":7,"uploadTimestamp":0,"action":"upload"}`) + case 2: + w.WriteHeader(http.StatusBadRequest) + // B2 limits an upload URL/token to one thread, and its integration guide says to + // get a new pair after a broken connection. See https://github.com/restic/restic/issues/1207, + // https://www.backblaze.com/docs/cloud-storage-upload-files-with-the-native-api, and + // https://www.backblaze.com/docs/cloud-storage-integration-checklist. + _, _ = fmt.Fprint(w, `{"status":400,"code":"bad_request","message":"more than one upload using auth token: upload-token"}`) + default: + t.Errorf("unexpected upload call %d", call) + } + default: + t.Errorf("unexpected request: %s", r.URL.Path) + http.NotFound(w, r) + } + })) + t.Cleanup(srv.Close) + t.Cleanup(release) + root, err := AuthorizeAccount(context.Background(), "account", "key", SetAPIBase(srv.URL)) + if err != nil { + t.Fatal(err) + } + upload := &URL{uri: srv.URL + "/upload", token: "upload-token", b2: root} + go func() { + _, err := upload.UploadFile(context.Background(), strings.NewReader("payload"), len("payload"), "name", "text/plain", "sha1", nil) + firstDone <- err + }() + wait, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + select { + case <-firstEntered: + case <-wait.Done(): + t.Fatalf("first upload never reached handler: %v", wait.Err()) + } + if _, err := upload.UploadFile(context.Background(), strings.NewReader("payload"), len("payload"), "name", "text/plain", "sha1", nil); err == nil { + t.Fatal("second upload succeeded while the shared token was in use") + } else if got := Action(err); got != AttemptNewUpload { + t.Errorf("Action(%v) = %v, want AttemptNewUpload", err, got) + } + release() + select { + case err := <-firstDone: + if err != nil { + t.Errorf("first upload after release: %v", err) + } + case <-wait.Done(): + t.Fatalf("first upload did not complete after release: %v", wait.Err()) + } + }) + + t.Run("connection reset before a response", func(t *testing.T) { + var srv *httptest.Server + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/b2api/v4/b2_authorize_account": + _, _ = io.WriteString(w, v4AuthJSON(srv.URL, nil, nil, "")) + case "/upload": + conn, _, err := w.(http.Hijacker).Hijack() + if err != nil { + t.Errorf("hijack upload connection: %v", err) + return + } + _ = conn.Close() + default: + t.Errorf("unexpected request: %s", r.URL.Path) + http.NotFound(w, r) + } + })) + t.Cleanup(srv.Close) + root, err := AuthorizeAccount(context.Background(), "account", "key", SetAPIBase(srv.URL)) + if err != nil { + t.Fatal(err) + } + upload := &URL{uri: srv.URL + "/upload", token: "upload-token", b2: root} + if _, err := upload.UploadFile(context.Background(), strings.NewReader("payload"), len("payload"), "name", "text/plain", "sha1", nil); err == nil { + t.Fatal("upload succeeded after its connection reset") + } else if got := Action(err); got != AttemptNewUpload { + t.Errorf("Action(%v) = %v, want AttemptNewUpload", err, got) + } + }) +} + +func TestHTTPRequestCancellationAndDeadline(t *testing.T) { + for _, name := range []string{"cancelled", "deadline"} { + t.Run(name, func(t *testing.T) { + entered := make(chan struct{}) + observedCancel := make(chan struct{}) + release := make(chan struct{}) + var srv *httptest.Server + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/b2api/v4/b2_authorize_account": + _, _ = io.WriteString(w, v4AuthJSON(srv.URL, nil, nil, "")) + case "/wait": + _, _ = io.Copy(io.Discard, r.Body) + close(entered) + select { + case <-r.Context().Done(): + close(observedCancel) + case <-release: + } + default: + t.Errorf("unexpected request: %s", r.URL.Path) + http.NotFound(w, r) + } + })) + t.Cleanup(srv.Close) + // Release an ignored-context request even when the assertion fails. + t.Cleanup(func() { close(release) }) + root, err := AuthorizeAccount(context.Background(), "account", "key", SetAPIBase(srv.URL)) + if err != nil { + t.Fatal(err) + } + upload := &URL{uri: srv.URL + "/wait", token: "upload-token", b2: root} + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + want := context.DeadlineExceeded + if name == "cancelled" { + cancel() + want = context.Canceled + } + done := make(chan error, 1) + go func() { + _, err := upload.UploadFile(ctx, strings.NewReader("x"), 1, "name", "text/plain", "sha1", nil) + done <- err + }() + wait, cancelWait := context.WithTimeout(context.Background(), 5*time.Second) + defer cancelWait() + if name == "deadline" { + select { + case <-entered: + case <-wait.Done(): + t.Fatalf("deadline request never reached handler: %v", wait.Err()) + } + } + select { + case err := <-done: + if !errors.Is(err, want) { + t.Errorf("%s request error = %v, want %v", name, err, want) + } else if got := Action(err); got != Punt { + t.Errorf("Action(%v) = %v, want Punt", err, got) + } + case <-wait.Done(): + t.Fatalf("%s request did not return: %v", name, wait.Err()) + } + if name == "deadline" { + select { + case <-observedCancel: + case <-wait.Done(): + t.Fatal("handler did not observe request deadline") + } + } + }) + } +} + +func TestUnknownCertificateAuthority(t *testing.T) { + var srv *httptest.Server + srv = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, v4AuthJSON(srv.URL, nil, nil, "")) + })) + t.Cleanup(srv.Close) + _, err := AuthorizeAccount(context.Background(), "account", "key", SetAPIBase(srv.URL)) + if err == nil { + t.Fatal("authorization accepted an unknown certificate authority") + } + var verification *tls.CertificateVerificationError + var unknown x509.UnknownAuthorityError + if !errors.As(err, &verification) && !errors.As(err, &unknown) { + t.Errorf("authorization error = %T: %v, want certificate verification error", err, err) + } +} diff --git a/intent/credential-free-live-calls/intent.md b/intent/credential-free-live-calls/intent.md new file mode 100644 index 00000000..db9d7baf --- /dev/null +++ b/intent/credential-free-live-calls/intent.md @@ -0,0 +1,38 @@ +# Intent: credential-free tests for calls only covered by live tests +Author: Felipe Fumero. Issue: #203. Parent: #97. Status: ready for review. + +## Problem + +These calls run only in live tests that skip without credentials, so `go test ./...` and CI never execute +them: + +- hide and reveal (`b2/b2.go:881,891`, `base/base.go:1889`); +- download authorization, including `b2ContentDisposition` (`base/base.go:1739`, `b2/backend.go:729`, + `Bucket.AuthToken` and `Object.AuthURL` at `b2/b2.go:932,939`); +- list and delete keys (`b2/key.go:144`, `base/base.go:2095,2107`); +- cancel large file (`Object.Cancel`, `b2/b2.go:861`), upload after a connection hang, context cancellation + and deadline, and an unknown certificate authority (`base/integration_test.go:56,432,502,549,592`). + +`x/transport` has no test file. Its fault helpers are used by blazer's own tests and by downstream users, and +`go test ./...` reports `[no test files]`. + +## Proposed outcome + +Each call has a credential-free test of its request and response. `x/transport` has a test per helper. The +live tests stay as they are. Blazer's credential-free statement coverage is measured and recorded. + +## Affected users and APIs + +Test code only. + +## Constraints + +- Test-only. If a new test fails on current code, it found a product bug: stop and open a fix issue. +- Expected values from the B2 docs and blazer's doc comments, never from blazer's output. +- Credential-free and safe under `go test -race -count=5 -shuffle=on`. +- `transport.Trigger` has a known data race (#176). A `Trigger` test that fails under `-race` is #176's bug: + leave it out and say so, do not fix it here. + +## Open questions + +Key.Expires() maps a null expiration to 1970 (#244, fixed by #250); this change doesn't assert that mapping. diff --git a/intent/credential-free-live-calls/plan.md b/intent/credential-free-live-calls/plan.md new file mode 100644 index 00000000..b7aa0d8f --- /dev/null +++ b/intent/credential-free-live-calls/plan.md @@ -0,0 +1,92 @@ +# Plan: credential-free tests for live-only calls (from intent.md and spec.md) + +Branch: `test-issue-203-credential-free-live-calls`, from #191's branch (`test-issue-191-shared-recording-server`); +#191 merges first. One PR for #203 carries this folder and the tests. Follow `AGENTS.md`, +"Things agents get wrong here": new wire tests use `internal/b2wiretest`. Reuse the `httptest` patterns +in `base/*_test.go` and existing fixtures. + +## Files that change + +``` +b2/live_only_calls_wire_test.go (new) R1-R7 +base/live_only_calls_test.go (new) R8-R10 +x/transport/transport_test.go (new) R11 +``` + +No production file changes (R13). + +## Order of work + +0. Record coverage before: `go test -count=1 -coverprofile=/tmp/before.out ./... && go tool cover -func=/tmp/before.out | tail -1` (R12). +1. R11 `x/transport` tests (no dependencies). +2. R1–R7 wire tests. +3. R8–R10 base tests. +4. Record coverage after (R12). +5. `make ci`; then a fresh-context review of the diff per `AGENTS.md`, answering each finding. + +## Risks + +- A new test fails on current code: product bug; stop and report. +- `Trigger` under `-race`: drop the test, note #176. +- R8/R9 flake under `-count=5`: use channels and context deadlines, never fixed sleeps; if still flaky, stop + and report. + +## Proof + +| Req | Break | Must fail | +| --- | --- | --- | +| R1 | `base/base.go` `HideFile`: send `File: ""` | R1 test | +| R2 | `b2/b2.go` `Reveal`: `obj.f.status() == "hide"` → `!= "hide"` | R2 test | +| R3/R4 | `base/base.go` `GetDownloadAuthorization`: drop `ContentDisposition` from the request; then `b2/b2.go` `AuthURL`: drop the `b2ContentDisposition` query | R4 test (each) | +| R5 | `base/base.go` `ListKeys`: send `Next: ""` | R5 test | +| R6 | `base/base.go` `Key.Delete`: send `KeyID: ""` | R6 test | +| R7 | `b2/b2.go` `Cancel`: `status == "start"` → `status != "start"` | R7 test | +| R8 | `base/base.go` `Action`: remove the `more than one upload using auth token` case; then the `e.code == 0` upload case | R8 test (each) | +| R11 | `x/transport` `Response`: drop the `Body` message | the `Response`/`Body` test | + +Also record `go test -race -count=5 -shuffle=on ./b2 ./base ./x/transport` passing with the B2 credentials +unset, and `make ci`. + +## Review follow-up proof (2026-10-30) + +Each break below was applied alone. The command was run with the four B2 credential variables unset, +then the break was reverted and the identical command passed. Only test files and this intent folder +remain changed. The one-shot row concerns the callback installed by `AfterNBytes`, not `Trigger(ctx)`; +the latter's race is recorded separately below and in the spec. + +| Req | Break | Failing output | Passing output after revert | +| --- | --- | --- | --- | +| R2 (upload) | `Reveal`: delete for either `hide` or `upload` | `b2_delete_file_version calls = 1, want 0` | `ok github.com/Backblaze/blazer/b2 0.737s` | +| R7 (upload) | `Cancel`: permit either `start` or `upload` | `Cancel uploaded file error = , want "complete is not an unfinished large file: upload"`; `b2_cancel_large_file calls = 1, want 0` | `ok github.com/Backblaze/blazer/b2 0.317s` | +| R9 (cancelled) | `makeNetRequest`: attach `context.WithoutCancel(ctx)` | `cancelled request did not return: context deadline exceeded` | `ok github.com/Backblaze/blazer/base 0.279s` | +| R9 (deadline) | `makeNetRequest`: attach `context.WithoutCancel(ctx)` | `deadline request did not return: context deadline exceeded` | `ok github.com/Backblaze/blazer/base 1.292s` | +| R10 | Authorization test: supply a transport with `TLSClientConfig.InsecureSkipVerify: true` | `authorization accepted an unknown certificate authority` | `ok github.com/Backblaze/blazer/base 0.315s` | +| R11 (one-shot) | `doRequest`: reset `o.trg.triggered` to zero before each request | `effect calls after second upload and both responses = 2, want 1` | `ok github.com/Backblaze/blazer/x/transport 0.267s` | +| R11 (response bytes) | `doRequest`: remove response-body wrapping | `effect calls after short body and response = 0, want 1` | `ok github.com/Backblaze/blazer/x/transport 0.250s` | + +Commands (from the repo root, each prefixed by +`env -u B2_APPLICATION_KEY_ID -u B2_APPLICATION_KEY -u B2_ACCOUNT_ID -u B2_ACCOUNT_KEY`): + +```sh +go test -count=1 -run '^TestBucketRevealWire$/^leaves_a_current_upload_alone$' ./b2 +go test -count=1 -run '^TestObjectCancelWire$/^uploaded_file$' ./b2 +go test -count=1 -run '^TestHTTPRequestCancellationAndDeadline$/^cancelled$' ./base +go test -count=1 -run '^TestHTTPRequestCancellationAndDeadline$/^deadline$' ./base +go test -count=1 -run '^TestUnknownCertificateAuthority$' ./base +go test -count=1 -run '^TestAfterNBytesTriggersOnce$' ./x/transport +go test -count=1 -run '^TestAfterNBytesDoesNotTriggerForShortBody$' ./x/transport +``` + +The R9 tests use independent `cancelled` and `deadline` subtests with a five-second failure bound and a +cleanup release channel, so ignoring the context fails without hanging the test server. The R10 server +returns valid authorization JSON: accepting its untrusted certificate must fail the explicit assertion, +not panic later while parsing an incomplete fixture. +The Reveal listing request identifies `"gone"` with the B2 `prefix` field (`Reveal` uses `ListPrefix`); +`b2_list_file_versions` does not have a `fileName` request field. + +## Trigger race (#176) + +`x/transport.Trigger` has a data race when a request runs after its context is cancelled: the callback +writes `triggered` while `RoundTrip` reads it (`x/transport/transport.go:70` and `:159`). It reproduces +with `-race -count=20`. #176 tracks the fix, so this change adds no test that depends on it. The R11 tests +above cover `AfterNBytes`, not `Trigger(ctx)`, and are race-clean. diff --git a/intent/credential-free-live-calls/spec.md b/intent/credential-free-live-calls/spec.md new file mode 100644 index 00000000..fd5c5321 --- /dev/null +++ b/intent/credential-free-live-calls/spec.md @@ -0,0 +1,98 @@ +# Spec: credential-free tests for calls only covered by live tests +From: intent.md (Felipe Fumero). Issue: #203. Status: ready for review. +Skills applied: none beyond `AGENTS.md`, `REVIEW.md` and `docs/testing.md`. + +Sources of truth (each test links the page): B2 `b2_hide_file`, `b2_delete_file_version`, +`b2_get_download_authorization`, `b2_list_keys`, `b2_delete_key`, `b2_cancel_large_file`; B2 "Download +authorization" (the `Authorization` and `b2ContentDisposition` URL parameters); the `x/transport` doc comments. + +## Requirements + +### b2 (wire tests with `internal/b2wiretest`) + +- **R1. Hide.** `Object.Hide` sends one `b2_hide_file` with `bucketId` and `fileName`, and returns nil; a + scripted 400 error is returned to the caller. +- **R2. Reveal.** When the newest version of the name is a `hide` marker, `Bucket.Reveal` sends + `b2_delete_file_version` with that marker's `fileId` and `fileName`. When the newest version is an upload, + it sends no delete. +- **R3. Download authorization token.** `Bucket.AuthToken(ctx, "p/", time.Hour)` sends + `b2_get_download_authorization` with `bucketId`, `fileNamePrefix` `"p/"`, `validDurationInSeconds` `3600`, + no `b2ContentDisposition`, and returns the response `authorizationToken`. +- **R4. Download URL.** `Object.AuthURL(ctx, time.Hour, "attachment; filename=\"a b.txt\"")` sends + `fileNamePrefix` equal to the object name and `b2ContentDisposition` equal to that value, and returns a URL + whose query has `Authorization` equal to the token and `b2ContentDisposition` equal to the value (decoded). + With an empty disposition the URL has no `b2ContentDisposition`. +- **R5. List keys.** `Client.ListKeys(ctx, 10, "")` sends `b2_list_keys` with `accountId` and `maxKeyCount` + `10` and no `startApplicationKeyId`; a response with two keys and `nextApplicationKeyId` `"k-3"` returns two + keys (name, ID, non-null expiration from milliseconds) and cursor `"k-3"`; a second call with `"k-3"` + sends `startApplicationKeyId` `"k-3"`. An empty `keys` list returns `io.EOF`. + A key with no expiry uses `expirationTimestamp: null`; its name and ID are asserted, while its expiry + mapping is fixed by #250 (#244); this test does not assert it. Capabilities mapping is tracked separately in #220. +- **R6. Delete key.** `Key.Delete` sends `b2_delete_key` with `applicationKeyId`. +- **R7. Cancel large file.** `Object.Cancel` on an unfinished (`start`) file sends `b2_cancel_large_file` + with its `fileId`; on an uploaded file it returns an error and sends no cancel. + +### base (`httptest`, as the existing `base/*_test.go` do) + +- **R8. Upload after a connection hang.** This mirrors the live `TestUploadAuthAfterConnectionHang`: while one + upload on an upload URL is hung, a second upload with the same URL and token is refused by B2 with + `400 bad_request` and the message prefix `more than one upload using auth token` (restic/restic#1207). + Against an `httptest` server: the first `b2_upload_file` handler blocks (released at test end); the second + answers that 400. `Action(err)` for the second upload is `AttemptNewUpload`. Also: an upload whose + transport fails before any response (a connection reset, not a context error) yields `AttemptNewUpload`. + A context the caller cancelled or let expire is not an upload fault: `Action` is `Punt` (R9 covers the + error itself). +- **R9. Cancellation and deadline.** A request made with an already-cancelled context returns an error that + `errors.Is(err, context.Canceled)`; a request whose handler blocks past a short deadline returns an error + that `errors.Is(err, context.DeadlineExceeded)`, and the handler observes its request context done. No sleeps + beyond the deadline itself. +- **R10. Unknown CA.** Authorizing against `httptest.NewTLSServer` with the default transport (the test + certificate is not trusted) fails with a certificate verification error (`*tls.CertificateVerificationError` + or `x509.UnknownAuthorityError` via `errors.As`). + +### x/transport (new `x/transport/transport_test.go`) + +- **R11. One test per helper**, against an `httptest` server: `WithFailures(nil)` with no options passes + requests through; `FailureRate(0)` never fails and `FailureRate(1)` always fails; `MatchPathSubstring` + fails only matching paths; `Response(503)` with `Body("msg")` returns status 503 and body `msg`; `Stall` + returns no earlier than its duration or when the request context ends, whichever is first; `AfterNBytes` + calls its effect exactly once across requests after roughly n body bytes, counting request and response + bodies; a short request alone must not call the effect, but its response can cross the threshold. + `Trigger(ctx)` fails every request after `ctx` is + cancelled. If the `Trigger` test fails under `-race`, drop it and note #176 in the PR. + +### Coverage + +- **R12.** Record `go test -coverprofile` statement coverage for the module, credential-free, before and after + this change in the PR description. +- **R13. Test-only.** Only `*_test.go` files and `intent/credential-free-live-calls/` change. + +## Design + +- `b2/live_only_calls_wire_test.go`: R1–R7. +- `base/live_only_calls_test.go`: R8–R10. +- `x/transport/transport_test.go`: R11. + +## How each constraint is met + +| Constraint | Met by | +| --- | --- | +| Test-only | R13 | +| Expected values from docs | Field names from the B2 pages; literal values | +| Credential-free, race-safe | `b2wiretest`, `httptest`; `-race -count=5 -shuffle=on` | +| `Trigger` race is #176's | R11 rule | + +## Areas of concern + +1. **R2 Reveal listing.** `Reveal` lists with `ListHidden()`; the scripted `b2_list_file_versions` page must + put the newest version first, as B2 does. Owner: @ffumero2003. +2. **R10 error type.** Go wraps TLS failures in `*url.Error`; assert with `errors.As`, not string matching. + Owner: @ffumero2003. +3. **R11 Trigger is limited by #176.** A temporary `TestTriggerAfterCancellationRaceRepro` run with + `go test -race -count=20 -run '^TestTriggerAfterCancellationRaceRepro$' ./x/transport` reports a write in + `Trigger.func1.1` at `transport.go:159` racing with a read in `(*options).RoundTrip` at `transport.go:70`. + The cancellation-trigger assertion remains deferred to #176; the diagnostic test was removed so the + committed tests stay race-clean. This does not limit the separate `AfterNBytes` one-shot assertion. + Owner: @ffumero2003. +4. **R5 null expiry mapping.** #244's `expirationTimestamp: null` mapping is fixed by #250. The fixture + uses null and asserts the key's name and ID, but does not assert the expiry mapping. Owner: @ffumero2003. diff --git a/x/transport/transport_test.go b/x/transport/transport_test.go new file mode 100644 index 00000000..a3f06269 --- /dev/null +++ b/x/transport/transport_test.go @@ -0,0 +1,184 @@ +package transport + +import ( + "context" + "errors" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + "time" +) + +func TestWithFailuresPassesThrough(t *testing.T) { + srv := transportTestServer(t) + resp := transportRequest(t, &http.Client{Transport: WithFailures(nil)}, context.Background(), srv.URL+"/ok", nil) + if resp.StatusCode != http.StatusOK { + t.Errorf("status = %d, want %d", resp.StatusCode, http.StatusOK) + } +} + +func TestFailureRateAndPathMatch(t *testing.T) { + srv := transportTestServer(t) + tests := []struct { + name string + rt http.RoundTripper + path string + want int + }{ + {"rate zero passes through", WithFailures(nil, FailureRate(0), Response(http.StatusServiceUnavailable)), "/ok", http.StatusOK}, + {"rate one always fails", WithFailures(nil, FailureRate(1), Response(http.StatusServiceUnavailable)), "/ok", http.StatusServiceUnavailable}, + {"path mismatch passes through", WithFailures(nil, FailureRate(1), MatchPathSubstring("match"), Response(http.StatusServiceUnavailable)), "/other", http.StatusOK}, + {"path match fails", WithFailures(nil, FailureRate(1), MatchPathSubstring("match"), Response(http.StatusServiceUnavailable)), "/match", http.StatusServiceUnavailable}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + resp := transportRequest(t, &http.Client{Transport: tc.rt}, context.Background(), srv.URL+tc.path, nil) + if resp.StatusCode != tc.want { + t.Errorf("status = %d, want %d", resp.StatusCode, tc.want) + } + }) + } +} + +func TestResponseAndBody(t *testing.T) { + client := &http.Client{Transport: WithFailures(nil, FailureRate(1), Response(http.StatusServiceUnavailable), Body("retry later"))} + resp := transportRequest(t, client, context.Background(), "http://example.invalid", nil) + if resp.StatusCode != http.StatusServiceUnavailable { + t.Errorf("status = %d, want %d", resp.StatusCode, http.StatusServiceUnavailable) + } + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + if got, want := string(body), "retry later"; got != want { + t.Errorf("body = %q, want %q", got, want) + } +} + +func TestStallWaitsForDurationOrContext(t *testing.T) { + srv := transportTestServer(t) + const delay = 20 * time.Millisecond + t.Run("duration", func(t *testing.T) { + client := &http.Client{Transport: WithFailures(nil, FailureRate(1), Stall(delay))} + start := time.Now() + resp := transportRequest(t, client, context.Background(), srv.URL+"/ok", nil) + if elapsed := time.Since(start); elapsed < delay { + t.Errorf("returned after %v, want at least %v", elapsed, delay) + } + if resp.StatusCode != http.StatusOK { + t.Errorf("status = %d, want %d", resp.StatusCode, http.StatusOK) + } + }) + t.Run("context", func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), delay) + defer cancel() + const stall = time.Second + client := &http.Client{Transport: WithFailures(nil, FailureRate(1), Stall(stall))} + request, err := http.NewRequestWithContext(ctx, http.MethodGet, srv.URL+"/ok", nil) + if err != nil { + t.Fatal(err) + } + start := time.Now() + _, err = client.Do(request) + if !errors.Is(err, context.DeadlineExceeded) { + t.Errorf("error = %v, want context deadline exceeded", err) + } + if elapsed := time.Since(start); elapsed >= stall/2 { + t.Errorf("returned after %v, want before %v when the request context ends", elapsed, stall/2) + } + }) +} + +func TestAfterNBytesTriggersOnce(t *testing.T) { + srv := transportTestServer(t) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + var calls atomic.Int32 + triggered := make(chan struct{}) + client := &http.Client{Transport: WithFailures(nil, AfterNBytes(3, func() { + if calls.Add(1) == 1 { + close(triggered) + } + }))} + resp := transportRequest(t, client, ctx, srv.URL+"/upload", strings.NewReader("abcdef")) + if resp.StatusCode != http.StatusOK { + t.Errorf("status = %d, want %d", resp.StatusCode, http.StatusOK) + } + select { + case <-triggered: + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + if got := calls.Load(); got != 1 { + t.Errorf("effect calls = %d, want 1", got) + } + if _, err := io.Copy(io.Discard, resp.Body); err != nil { + t.Fatal(err) + } + resp = transportRequest(t, client, ctx, srv.URL+"/upload", strings.NewReader("abcdef")) + if resp.StatusCode != http.StatusOK { + t.Errorf("second status = %d, want %d", resp.StatusCode, http.StatusOK) + } + if _, err := io.Copy(io.Discard, resp.Body); err != nil { + t.Fatal(err) + } + if got := calls.Load(); got != 1 { + t.Errorf("effect calls after second upload and both responses = %d, want 1", got) + } +} + +func TestAfterNBytesDoesNotTriggerForShortBody(t *testing.T) { + srv := transportTestServer(t) + var calls atomic.Int32 + client := &http.Client{Transport: WithFailures(nil, AfterNBytes(3, func() { + calls.Add(1) + }))} + resp := transportRequest(t, client, context.Background(), srv.URL+"/upload", strings.NewReader("ab")) + if resp.StatusCode != http.StatusOK { + t.Errorf("status = %d, want %d", resp.StatusCode, http.StatusOK) + } + if got := calls.Load(); got != 0 { + t.Errorf("effect calls after short body = %d, want 0", got) + } + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + if string(body) != "ok" { + t.Errorf("response body = %q, want %q", body, "ok") + } + if got := calls.Load(); got != 1 { + t.Errorf("effect calls after short body and response = %d, want 1", got) + } +} + +func transportTestServer(t *testing.T) *httptest.Server { + t.Helper() + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.Copy(io.Discard, r.Body) + _, _ = io.WriteString(w, "ok") + })) + t.Cleanup(srv.Close) + return srv +} + +func transportRequest(t *testing.T, client *http.Client, ctx context.Context, target string, body io.Reader) *http.Response { + t.Helper() + req, err := http.NewRequestWithContext(ctx, http.MethodPost, target, body) + if err != nil { + t.Fatal(err) + } + resp, err := client.Do(req) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if err := resp.Body.Close(); err != nil { + t.Error(err) + } + }) + return resp +}