From a719a77892f28d6c3dfc1b0ee0f0bcdfc760ef6f Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Wed, 28 Oct 2026 13:17:58 -0600 Subject: [PATCH 01/26] docs(intent): add the intent for a shared recording B2 server for wire tests Several tests pass while blazer sends the wrong request to B2, because each wire test builds its own fake server and most accept any body. Capture the problem and the outcome for a shared test helper before writing a spec. Refs #191 --- intent/b2wiretest/intent.md | 68 +++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 intent/b2wiretest/intent.md diff --git a/intent/b2wiretest/intent.md b/intent/b2wiretest/intent.md new file mode 100644 index 00000000..1d6c4aed --- /dev/null +++ b/intent/b2wiretest/intent.md @@ -0,0 +1,68 @@ +# Intent: a shared recording B2 server for wire tests +Author: Felipe Fumero. Issue: #191. Parent: #97. Status: draft. + +## Problem + +Tests that exercise blazer against a fake B2 server each build their own server. The `b2` package has +at least eight of them (`newComposeWire`, `newLockWire`, `newSSECPublicWire`, `newSSEB2Wire`, +`newProgressWire`, `newReplicationCreateWire`, `newDownloadByIDWire`, `newAccountInfoWire`), and `base` +tests start their own `httptest` servers too. Each one re-implements authorization, routing and bookkeeping, and most of them +accept any request body. + +As a result, several tests pass while blazer sends the wrong request to B2. A mutation and test-claim +review at a5e0d13 recorded concrete cases, each filed as an issue under #97. In each case the production +code was changed and `go test ./...` stayed green: + +- `b2/key.go:175`: a bucket key created with an empty bucket id (#192). +- `b2/writer.go:369,417`: retention and legal hold dropped on upload (#193). +- `b2/notification.go:78-118`: the rule signing secret dropped (#194). +- `base/base.go:1534`: a constant upload-part SHA1 (#195). +- `base/base.go:1643,1692` and `b2/iterator.go:254,292,325`: the listing prefix or cursor lost (#196). +- `base/base.go:1192` and `b2/writer.go:355,414`: the SSE-B2 or Content-Type header wrong (#197). + +Each of those issues needs a test that asserts what is sent to B2 and what is parsed back. Writing a +new private fake for each of them would repeat the cause. + +## Proposed outcome + +One small test helper that every new wire test can use. It plays B2 over HTTP, records exactly what +blazer sent on each call (method, path, headers, JSON body), and serves the responses a test scripts. +A test can then say "the `b2_create_key` body had `bucketId` X" and get a "got X, want Y" failure when it +does not. + +Success looks like: + +- #192 to #198 are written against the helper, and each recorded break listed in those issues makes a + test fail. +- A contributor adding a new B2 call can write its wire test without building a server from scratch. +- No change to blazer's public API or its runtime behavior. + +## Affected users and APIs + +- Contributors and reviewers of blazer tests. Not end users: nothing is exported from the module. +- Test code in `base`, `b2` and `b2/sync`. Production code is not touched. + +## Constraints + +- **No new dependencies.** blazer has none beyond the standard library; `go.mod` and `go.sum` stay as they + are. +- **Go 1.26.10**, the module's declared minimum (see #190). +- **Not part of the public API.** It must not be importable by users of the module. +- **Credential-free and offline**: no B2 account, no network beyond loopback, safe under + `go test -race -count=5 -shuffle=on`. +- **Expected values in tests come from the B2 API docs**, not from blazer's own output. The helper + records and serves; it must not compute expected values for the tests that use it. +- **Existing tests keep working unchanged.** Migrating the existing per-feature fakes is not part of this + change. +- Reviewable in one small PR, per `AGENTS.md`. + +## Open questions + +1. Where it lives: a package under `internal/` (shared by `base`, `b2` and `b2/sync`) or a file per + package. A `_test.go` file cannot be shared across packages. (For the spec.) +2. Whether it should also cover the download and upload URLs that B2 hands out after authorization, or + only the API calls, with those added by the tests that need them. (For the spec.) +3. Whether the changelog needs an entry for a test-only internal package, or the PR takes the + `skip-changelog` label. (@goanpeca) +4. Whether `x/b2sim` should be reused instead. It answers a different question (does blazer agree with the + simulator), and its future is still open, so this intent assumes a separate helper. (@goanpeca) From d6970a9c57a1df83a9a796ce2da94416b702e8c2 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Wed, 28 Oct 2026 13:20:56 -0600 Subject: [PATCH 02/26] docs(intent): add the spec and plan for the shared recording B2 server Spec R1-R12 define the internal/b2wiretest helper: recording, scripted responses, upload URLs and assertions, with no new dependencies and no public API. The plan orders the work, names a test per requirement and the break each test must catch. Refs #191 --- intent/b2wiretest/plan.md | 91 ++++++++++++++++++++++++++++++++++++++ intent/b2wiretest/spec.md | 93 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 184 insertions(+) create mode 100644 intent/b2wiretest/plan.md create mode 100644 intent/b2wiretest/spec.md diff --git a/intent/b2wiretest/plan.md b/intent/b2wiretest/plan.md new file mode 100644 index 00000000..2add7bd1 --- /dev/null +++ b/intent/b2wiretest/plan.md @@ -0,0 +1,91 @@ +# Plan: a shared recording B2 server for wire tests (from intent.md and spec.md) + +Branch: `test-issue-191-shared-recording-server`. One PR for issue #191 carries this folder and the code. +Before writing code, apply the lean-code ladder: reuse what exists, prefer the standard library, add the +minimum (see `AGENTS.md`). + +## Files that change + +``` +internal/b2wiretest/server.go (new) R1-R8 +internal/b2wiretest/assert.go (new) R9 +internal/b2wiretest/server_test.go (new) R10 +b2/update_cors_name_test.go (edit) R11: TestUpdateBucketSendsCORSRuleName uses the helper +intent/b2wiretest/plan.md (edit only if the implementation departs from this plan) +``` + +Nothing else changes. `go.mod`, `go.sum`, `base/`, `b2/` (other than the one test), `x/` and `.changes/` +are untouched. + +## Order of work + +1. **Server core** (R1, R2, R4). `Server`, `New`, `URL`, and the recording handler: route + `/b2api/v/`, `/upload_file` and `/upload_part`; copy method, path, headers and body; decode a + JSON object body into `JSON`; append under the mutex. Use only `net/http`, `net/http/httptest`, + `encoding/json`, `io`, `strings`, `strconv`, `sync`, `testing` and `fmt`. +2. **Built-in responses** (R3, R7). Default `b2_authorize_account`, `b2_get_upload_url` and + `b2_get_upload_part_url` responses; default 200 `{}` for `upload_file` and `upload_part`. The v4 + authorization JSON follows the shape blazer already parses in `b2/update_cors_name_test.go` + (`updateAuthJSON`); copy the field set, with the server URL filled in. +3. **Scripting** (R5, R6). `Response`, `Reply`, `ReplySequence`, `Handle`; the unscripted path answers 400 + with the B2 error shape and calls `t.Errorf`. Handlers run outside the lock. +4. **Reading records** (R8). `Requests`, `Calls`, `Last` (with `t.Helper()` and `t.Fatalf`). +5. **Assertions** (R9) in `assert.go`, each with `t.Helper()`. Normalize `want` by a JSON round trip, walk + the dotted path (map key or decimal index), compare with `reflect.DeepEqual`, and report + ` body : got , want ` (or `missing`). Use `%s` with `json.Marshal` output so + values print as JSON. +6. **Helper tests** (R10) in `server_test.go`. Table-driven where it fits, using `net/http` directly: + - `TestRecordsMethodPathHeadersAndJSON`: a POST with a header and a JSON body is recorded with each + field, `Call` set, and `Seq` 1. + - `TestRecordsInArrivalOrderUnderConcurrency`: 50 concurrent requests; 50 records with distinct `Seq` + 1..50. Run with `-race`. + - `TestDefaultAuthorizeUsesServerURL`: `apiUrl` and `downloadUrl` equal `URL()` for both `/b2api/v3/` and + `/b2api/v4/`. + - `TestUploadURLsAreServedAndRecorded`: `b2_get_upload_url` returns `URL()+"/upload_file"`; a POST + there is recorded as `upload_file` with the raw body. + - `TestReplySequenceRepeatsLast`: responses 503, 200, then 200 again. + - `TestHandleSeesTheRequest`: the handler receives the decoded body. + - `TestUnscriptedCallFails`: run against a fake `testing.TB` (a small struct embedding `testing.TB` that + records `Errorf`), so the outer test can assert both the 400 response and the error report. + - `TestAssertJSONReportsGotAndWant`: the same fake TB; a mismatch reports call, path, got and want; a + missing path reports `missing`; `int64(5)` matches `5.0`; `[]string{"a"}` matches `[]any{"a"}`. + - `TestAssertNoJSONAndAssertHeader`. +7. **Port one test** (R11). Rewrite `TestUpdateBucketSendsCORSRuleName` on the helper: script + `b2_create_bucket` and `b2_update_bucket` with the existing `updateBucketJSON`, then use `AssertJSON` on + `Last(t, "b2_update_bucket")` for `corsRules.0.corsRuleName` = `"browser"` (B2 docs: + `b2_update_bucket`, `corsRules[].corsRuleName`). Keep `updateAuthJSON` and `updateBucketJSON`, since + other tests in the file use them. Keep the test name. +8. **Check and self-review.** `make ci`; then a fresh-context review of the diff (`/code-review`, or a + subagent that did not write it) per `AGENTS.md`, answering each finding. + +Steps 1 to 5 are one file pair and run in sequence. Step 6 follows 5. Step 7 depends only on steps 1 to +4. There is nothing to run in parallel inside this change. + +## Risks + +- **Import cycle** (spec R1). If `b2wiretest` imports `base` or `b2`, every `b2` test fails to compile. + Check with `go list -deps ./internal/b2wiretest | grep -c Backblaze/blazer` (expect 1: itself). +- **Flaky ordering.** `Seq` is assigned under the lock at record time, not at response time, so a + concurrent test can rely on it. +- **`t.Errorf` from a server goroutine after the test ends** panics ("Log in goroutine after Test..."). + Cleanup closes the server before the test returns (`t.Cleanup` order). Do not add work after `Close`. +- **Changelog check** (spec concern 1). Expect `make changelog-check` to fail without the + `skip-changelog` label. Note it in the PR rather than adding a fragment for an internal helper. + +## Proof + +| Req | Test | Fails when the change is reverted? | +| --- | --- | --- | +| R2, R3 | `TestDefaultAuthorizeUsesServerURL` | Yes: remove the default authorize response and the test gets 400 | +| R4 | `TestRecordsMethodPathHeadersAndJSON`, `TestRecordsInArrivalOrderUnderConcurrency` | Yes: drop the header copy or the lock and they fail (the second under `-race`) | +| R5 | `TestReplySequenceRepeatsLast`, `TestHandleSeesTheRequest` | Yes | +| R6 | `TestUnscriptedCallFails` | Yes: remove `t.Errorf` and the fake TB records nothing | +| R7 | `TestUploadURLsAreServedAndRecorded` | Yes | +| R8 | used by every test above; `Last` failure covered in `TestUnscriptedCallFails` style with the fake TB | Yes | +| R9 | `TestAssertJSONReportsGotAndWant`, `TestAssertNoJSONAndAssertHeader` | Yes: make `AssertJSON` always pass and the fake TB records no error | +| R11 | `TestUpdateBucketSendsCORSRuleName` | Yes: in `toBaseCORSRules` (`b2/baseline.go:287`) stop copying `Name`; the test fails with `got , want "browser"` | +| R12 | `make ci`; `git diff origin/main -- go.mod go.sum` empty; `go list -deps` check above | n/a | + +Record in the PR, for R4, R9 and R11: the command, the failing output with the break applied, and the +passing output after reverting it. Also record `go test -race -count=5 -shuffle=on ./internal/b2wiretest ./b2` +passing with `B2_APPLICATION_KEY_ID`, `B2_APPLICATION_KEY`, `B2_ACCOUNT_ID` and `B2_ACCOUNT_KEY` unset. diff --git a/intent/b2wiretest/spec.md b/intent/b2wiretest/spec.md new file mode 100644 index 00000000..4c5d448f --- /dev/null +++ b/intent/b2wiretest/spec.md @@ -0,0 +1,93 @@ +# Spec: a shared recording B2 server for wire tests +From: intent.md (Felipe Fumero). Issue: #191. Status: ready for review. +Skills applied: none beyond `AGENTS.md`, `REVIEW.md` and `docs/testing.md` (no policy skills apply to a test helper). + +## Requirements + +- **R1. Location and imports.** A new package `internal/b2wiretest`. It imports the standard library only. + It must not import `base`, `b2` or anything that imports them, because tests in those packages import it + (an import cycle otherwise). +- **R2. Lifecycle.** `New(t testing.TB) *Server` starts an `httptest.Server` and registers `t.Cleanup` to + close it. `(*Server).URL() string` returns its base URL, to pass to `base.SetAPIBase` or `b2.APIBase`. +- **R3. Authorization.** Without scripting, `b2_authorize_account` on any API version answers with a v4 + response whose `apiUrl`, `downloadUrl` and `s3ApiUrl` are the server URL, authorization token `"test-token"`, + no bucket or prefix restriction, and part sizes `absoluteMinimumPartSize` 5000000 and + `recommendedPartSize` 100000000. A test can replace it like any other call (R5). +- **R4. Recording.** Every request is recorded in arrival order as a `Request` with: `Seq` (1-based order), + `Method`, `Path`, `Call` (the last path segment for API calls, e.g. `b2_create_key`; `upload_file` and + `upload_part` for upload URLs), `Header` (a copy), `Body` (raw bytes) and `JSON` (`map[string]any` + decoded from the body when it is a JSON object, otherwise nil). Recording is safe for concurrent requests. +- **R5. Scripted responses.** Per call name: + - `Reply(call string, status int, body string)`: the same response every time; + - `ReplySequence(call string, rs ...Response)`: successive responses, the last one repeating; + - `Handle(call string, h func(Request) Response)`: computed per request. + `Response` has `Status int`, `Body string` and `Header http.Header`. A later registration for a call + replaces the earlier one. +- **R6. Unscripted calls fail loudly.** A request to a call with no response registered (other than the + R3 and R7 built-ins) is recorded, answered with status 400 and a B2-shaped error body + (`{"status":400,"code":"bad_request","message":"b2wiretest: no response for "}`), and reported + with `t.Errorf` naming the call. +- **R7. Upload URLs.** Without scripting, `b2_get_upload_url` and `b2_get_upload_part_url` return + `uploadUrl` = server URL + `/upload_file` or `/upload_part`, and authorization tokens `"upload-token"` + and `"upload-part-token"`. Requests to those paths are recorded as calls `upload_file` and `upload_part` + and answered with status 200 and body `{}` unless scripted. Tests that need a B2 file object in the + upload response script it. +- **R8. Reading records.** `Requests() []Request` (all, in order), `Calls(call string) []Request` + (filtered, in order) and `Last(t testing.TB, call string) Request` (fails the test with `t.Fatalf` if the + call was never made). +- **R9. Assertions.** Each reports the call, the field and "got X, want Y": + - `AssertJSON(t, r Request, path string, want any)`: `path` is dot-separated into the JSON body + (`fileRetention.mode`, `bucketIds.0`). Values are compared after a JSON round trip of `want`, so + `int64(5)` and `5.0` compare equal, as do a `[]string` and a `[]any` with the same elements. + - `AssertNoJSON(t, r Request, path string)`: the field is absent. + - `AssertHeader(t, r Request, name, want string)`. + + The helper never computes expected values. Tests supply them from the B2 API docs. +- **R10. Tests of the helper.** `internal/b2wiretest` has its own tests for R2 to R9, using `net/http` + as the client. Each test fails if the behavior it names is removed. +- **R11. One existing test ported.** `TestUpdateBucketSendsCORSRuleName` (`b2/update_cors_name_test.go`) + is rewritten to use the helper, with the same assertions or stronger. This shows the API fits a real + `base` plus `b2` path. No other existing test changes. +- **R12. Repository rules.** `go.mod` and `go.sum` unchanged; builds with Go 1.26.10; `make ci` passes; no + exported symbol is added to `base`, `b2` or `x/...`. + +## Design + +``` +internal/b2wiretest/ +├── server.go # Server, New, URL, Reply, ReplySequence, Handle, Requests, Calls, Last +├── assert.go # AssertJSON, AssertNoJSON, AssertHeader +└── server_test.go # R10 +``` + +- The server routes on the path. `/b2api/v/` gives `Call = ` for any ``, so the helper + works across the API versions blazer uses (v3 and v4 today) without listing them. +- One mutex guards the record slice and the response table. Handlers run outside the lock, so a `Handle` + function may call `Requests()`. +- The record holds copies (headers cloned, body read fully), so a test can inspect it after the client + has closed the request. +- `AssertJSON` walks maps by key and slices by decimal index. A missing step reports + ` body : missing`. +- There is no global state. Each test gets its own server, so tests stay safe under `-shuffle=on` and + `t.Parallel()`. + +## How each constraint in the intent is met + +| Constraint | Met by | +| --- | --- | +| No new dependencies | R1, R12 | +| Go 1.26.10 | R12 (standard library only; no newer API) | +| Not part of the public API | R1 (`internal/`), R12 | +| Credential-free, offline, race-safe | R2 (loopback `httptest`), R4 (mutex), R10 under `-race -count=5 -shuffle=on` | +| Expected values come from the B2 docs | R9: the helper only records and compares; values come from tests | +| Existing tests keep working | R11 limits changes to one test | +| One small PR | Three new files and one rewritten test | + +## Areas of concern + +1. **Changelog.** `internal/b2wiretest/*.go` are non-test `.go` files, so `make changelog-check` will ask + for an entry. The change is invisible to users. Proposal: the `skip-changelog` label. Owner: @goanpeca. +2. **Reuse of `x/b2sim`.** It is not reused: it models B2 behavior and answers "does blazer agree with + the simulator", while this helper only records requests and plays scripted responses. Owner: @goanpeca. +3. **Coverage tooling.** A non-test package that only tests import shows up in coverage reports as + uncovered code outside its own tests. Its own tests (R10) cover it. From 9984f361cc9e87b725abba65af5746de9cc26456 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Wed, 28 Oct 2026 13:35:52 -0600 Subject: [PATCH 03/26] test(internal): add recording B2 wire server --- internal/b2wiretest/assert.go | 76 +++++++ internal/b2wiretest/server.go | 189 +++++++++++++++++ internal/b2wiretest/server_test.go | 318 +++++++++++++++++++++++++++++ 3 files changed, 583 insertions(+) create mode 100644 internal/b2wiretest/assert.go create mode 100644 internal/b2wiretest/server.go create mode 100644 internal/b2wiretest/server_test.go diff --git a/internal/b2wiretest/assert.go b/internal/b2wiretest/assert.go new file mode 100644 index 00000000..f060aa9a --- /dev/null +++ b/internal/b2wiretest/assert.go @@ -0,0 +1,76 @@ +package b2wiretest + +import ( + "encoding/json" + "reflect" + "strconv" + "strings" + "testing" +) + +// AssertJSON checks a dot-separated path in a request's JSON object body. +func AssertJSON(t testing.TB, r Request, path string, want any) { + t.Helper() + wantJSON, err := json.Marshal(want) + if err != nil { + t.Fatalf("%s body %s: encode want: %v", r.Call, path, err) + return + } + var normalized any + if err := json.Unmarshal(wantJSON, &normalized); err != nil { + t.Fatalf("%s body %s: decode want: %v", r.Call, path, err) + return + } + got, ok := jsonPath(r.JSON, path) + if !ok { + t.Errorf("%s body %s: missing, want %s", r.Call, path, wantJSON) + return + } + if !reflect.DeepEqual(got, normalized) { + gotJSON, _ := json.Marshal(got) + t.Errorf("%s body %s: got %s, want %s", r.Call, path, gotJSON, wantJSON) + } +} + +// AssertNoJSON checks that a dot-separated path is absent from a request body. +func AssertNoJSON(t testing.TB, r Request, path string) { + t.Helper() + if got, ok := jsonPath(r.JSON, path); ok { + gotJSON, _ := json.Marshal(got) + t.Errorf("%s body %s: got %s, want missing", r.Call, path, gotJSON) + } +} + +// AssertHeader checks one request header's value. +func AssertHeader(t testing.TB, r Request, name, want string) { + t.Helper() + if got := r.Header.Get(name); got != want { + t.Errorf("%s header %s: got %q, want %q", r.Call, name, got, want) + } +} + +func jsonPath(root map[string]any, path string) (any, bool) { + if path == "" { + return nil, false + } + var current any = root + for _, part := range strings.Split(path, ".") { + switch node := current.(type) { + case map[string]any: + value, ok := node[part] + if !ok { + return nil, false + } + current = value + case []any: + index, err := strconv.Atoi(part) + if err != nil || index < 0 || index >= len(node) { + return nil, false + } + current = node[index] + default: + return nil, false + } + } + return current, true +} diff --git a/internal/b2wiretest/server.go b/internal/b2wiretest/server.go new file mode 100644 index 00000000..92594966 --- /dev/null +++ b/internal/b2wiretest/server.go @@ -0,0 +1,189 @@ +// Package b2wiretest provides a recording HTTP server for B2 wire tests. +package b2wiretest + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" +) + +// Request is a snapshot of one HTTP request received by a Server. +type Request struct { + Seq int + Method string + Path string + Call string + Header http.Header + Body []byte + JSON map[string]any +} + +// Response is the status, headers, and body returned for a scripted call. +type Response struct { + Status int + Body string + Header http.Header +} + +type script struct { + responses []Response + handler func(Request) Response + used int +} + +// Server records B2 requests and answers with built-in or scripted responses. +type Server struct { + t testing.TB + http *httptest.Server + mu sync.Mutex + requests []Request + scripts map[string]*script +} + +// New starts a recording server and closes it when t finishes. +func New(t testing.TB) *Server { + t.Helper() + s := &Server{t: t, scripts: make(map[string]*script)} + s.http = httptest.NewServer(http.HandlerFunc(s.serve)) + t.Cleanup(s.http.Close) + return s +} + +// URL returns the server's base URL. +func (s *Server) URL() string { return s.http.URL } + +// Reply registers a response repeated for every request to call. +func (s *Server) Reply(call string, status int, body string) { + s.ReplySequence(call, Response{Status: status, Body: body}) +} + +// ReplySequence registers successive responses; the last response repeats. +func (s *Server) ReplySequence(call string, rs ...Response) { + s.mu.Lock() + defer s.mu.Unlock() + s.scripts[call] = &script{responses: append([]Response(nil), rs...)} +} + +// Handle registers a response computed from each request. It replaces any earlier script. +func (s *Server) Handle(call string, h func(Request) Response) { + s.mu.Lock() + defer s.mu.Unlock() + s.scripts[call] = &script{handler: h} +} + +// Requests returns all requests in arrival order. +func (s *Server) Requests() []Request { + s.mu.Lock() + defer s.mu.Unlock() + return copyRequests(s.requests) +} + +// Calls returns requests for call in arrival order. +func (s *Server) Calls(call string) []Request { + s.mu.Lock() + defer s.mu.Unlock() + var matches []Request + for _, r := range s.requests { + if r.Call == call { + matches = append(matches, r) + } + } + return copyRequests(matches) +} + +// Last returns the last request for call, or fails the test if none exists. +func (s *Server) Last(t testing.TB, call string) Request { + t.Helper() + requests := s.Calls(call) + if len(requests) == 0 { + t.Fatalf("b2wiretest: no request for %s", call) + return Request{} + } + return requests[len(requests)-1] +} + +func copyRequests(requests []Request) []Request { + result := make([]Request, len(requests)) + for i, r := range requests { + result[i] = r + result[i].Header = r.Header.Clone() + result[i].Body = append([]byte(nil), r.Body...) + if r.JSON != nil { + _ = json.Unmarshal(result[i].Body, &result[i].JSON) + } + } + return result +} + +func (s *Server) serve(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + s.t.Errorf("b2wiretest: read %s body: %v", r.URL.Path, err) + http.Error(w, "b2wiretest: cannot read request body", http.StatusBadRequest) + return + } + call := r.URL.Path[strings.LastIndexByte(r.URL.Path, '/')+1:] + req := Request{Method: r.Method, Path: r.URL.Path, Call: call, Header: r.Header.Clone(), Body: body} + if len(body) > 0 { + _ = json.Unmarshal(body, &req.JSON) + } + + s.mu.Lock() + req.Seq = len(s.requests) + 1 + s.requests = append(s.requests, req) + registered, ok := s.scripts[call] + var response Response + var handler func(Request) Response + if ok { + handler = registered.handler + if handler == nil && len(registered.responses) > 0 { + index := registered.used + if index >= len(registered.responses) { + index = len(registered.responses) - 1 + } + response = registered.responses[index] + registered.used++ + } + } + s.mu.Unlock() + + if handler != nil { + response = handler(copyRequests([]Request{req})[0]) + } else if !ok { + response, ok = s.builtin(call) + } + if !ok || response.Status == 0 { + s.t.Errorf("b2wiretest: no response for %s", call) + response = Response{Status: http.StatusBadRequest, + Body: fmt.Sprintf(`{"status":400,"code":"bad_request","message":"b2wiretest: no response for %s"}`, call)} + } + for name, values := range response.Header { + for _, value := range values { + w.Header().Add(name, value) + } + } + w.WriteHeader(response.Status) + if _, err := io.WriteString(w, response.Body); err != nil { + s.t.Errorf("b2wiretest: write %s response: %v", call, err) + } +} + +func (s *Server) builtin(call string) (Response, bool) { + switch call { + case "b2_authorize_account": + return Response{Status: http.StatusOK, Body: fmt.Sprintf(`{"accountId":"a","authorizationToken":"test-token","applicationKeyExpirationTimestamp":0,"apiInfo":{"storageApi":{"absoluteMinimumPartSize":5000000,"apiUrl":"%[1]s","capabilities":[],"downloadUrl":"%[1]s","storageApi":"storage","recommendedPartSize":100000000,"s3ApiUrl":"%[1]s","allowed":{"buckets":null,"capabilities":[],"namePrefix":null}}}}`, s.URL())}, true + case "b2_get_upload_url": + return Response{Status: http.StatusOK, Body: fmt.Sprintf(`{"uploadUrl":%q,"authorizationToken":"upload-token"}`, s.URL()+"/upload_file")}, true + case "b2_get_upload_part_url": + return Response{Status: http.StatusOK, Body: fmt.Sprintf(`{"uploadUrl":%q,"authorizationToken":"upload-part-token"}`, s.URL()+"/upload_part")}, true + case "upload_file", "upload_part": + return Response{Status: http.StatusOK, Body: `{}`}, true + default: + return Response{}, false + } +} diff --git a/internal/b2wiretest/server_test.go b/internal/b2wiretest/server_test.go new file mode 100644 index 00000000..3ac4e79e --- /dev/null +++ b/internal/b2wiretest/server_test.go @@ -0,0 +1,318 @@ +package b2wiretest + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" +) + +type recordingTB struct { + testing.TB + mu sync.Mutex + messages []string +} + +func (t *recordingTB) Errorf(format string, args ...any) { + t.mu.Lock() + defer t.mu.Unlock() + t.messages = append(t.messages, fmt.Sprintf(format, args...)) +} + +func (t *recordingTB) Fatalf(format string, args ...any) { t.Errorf(format, args...) } + +func (t *recordingTB) errors() []string { + t.mu.Lock() + defer t.mu.Unlock() + return append([]string(nil), t.messages...) +} + +func get(t *testing.T, url string) *http.Response { + t.Helper() + response, err := http.Get(url) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { response.Body.Close() }) + return response +} + +func TestRecordsMethodPathHeadersAndJSON(t *testing.T) { + s := New(t) + s.Reply("b2_create_key", http.StatusOK, `{}`) + request, err := http.NewRequest(http.MethodPost, s.URL()+"/b2api/v4/b2_create_key", strings.NewReader(`{"bucketIds":["bid"],"count":5}`)) + if err != nil { + t.Fatal(err) + } + request.Header.Set("X-Test", "value") + response, err := http.DefaultClient.Do(request) + if err != nil { + t.Fatal(err) + } + response.Body.Close() + if response.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want 200", response.StatusCode) + } + records := s.Requests() + if len(records) != 1 { + t.Fatalf("records = %d, want 1", len(records)) + } + r := records[0] + if r.Seq != 1 || r.Method != http.MethodPost || r.Path != "/b2api/v4/b2_create_key" || r.Call != "b2_create_key" { + t.Errorf("record = %+v, want seq=1 POST /b2api/v4/b2_create_key", r) + } + AssertHeader(t, r, "X-Test", "value") + if string(r.Body) != `{"bucketIds":["bid"],"count":5}` { + t.Errorf("body = %q", r.Body) + } + AssertJSON(t, r, "bucketIds.0", "bid") + AssertJSON(t, r, "count", int64(5)) + if len(s.Calls("b2_create_key")) != 1 || s.Last(t, "b2_create_key").Seq != 1 { + t.Error("Calls or Last did not return the recorded request") + } + // A caller must not be able to mutate the server's record through the returned slice. + r.Header.Set("X-Test", "changed") + r.Body[0] = 'x' + r.JSON["bucketIds"].([]any)[0] = "changed" + if got := s.Last(t, "b2_create_key"); got.Header.Get("X-Test") != "value" || got.Body[0] != '{' || got.JSON["bucketIds"].([]any)[0] != "bid" { + t.Errorf("record changed through returned snapshot: %+v", got) + } + // The server must copy the incoming header, not retain the request's map. + serverRequest := httptest.NewRequest(http.MethodPost, "/b2api/v4/b2_create_key", nil) + serverRequest.Header.Set("X-Test", "before") + s.serve(httptest.NewRecorder(), serverRequest) + serverRequest.Header.Set("X-Test", "after") + if got := s.Last(t, "b2_create_key").Header.Get("X-Test"); got != "before" { + t.Errorf("recorded header = %q, want before", got) + } +} + +func TestRecordsInArrivalOrderUnderConcurrency(t *testing.T) { + s := New(t) + s.Reply("b2_list_buckets", http.StatusOK, `{}`) + const count = 50 + var wg sync.WaitGroup + errors := make(chan error, count) + for i := 0; i < count; i++ { + wg.Add(1) + go func() { + defer wg.Done() + response, err := http.Get(s.URL() + "/b2api/v3/b2_list_buckets") + if err != nil { + errors <- err + return + } + response.Body.Close() + if response.StatusCode != http.StatusOK { + errors <- fmt.Errorf("status = %d", response.StatusCode) + } + }() + } + wg.Wait() + close(errors) + for err := range errors { + t.Error(err) + } + records := s.Requests() + if len(records) != count { + t.Fatalf("records = %d, want %d", len(records), count) + } + for i, r := range records { + if r.Seq != i+1 || r.Call != "b2_list_buckets" { + t.Errorf("record %d = seq %d call %q", i, r.Seq, r.Call) + } + } +} + +func TestDefaultAuthorizeUsesServerURL(t *testing.T) { + s := New(t) + for _, version := range []string{"v3", "v4"} { + t.Run(version, func(t *testing.T) { + response := get(t, s.URL()+"/b2api/"+version+"/b2_authorize_account") + if response.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want 200", response.StatusCode) + } + var body struct { + AccountID string `json:"accountId"` + AuthorizationToken string `json:"authorizationToken"` + APIInfo struct { + StorageAPI struct { + APIURL string `json:"apiUrl"` + DownloadURL string `json:"downloadUrl"` + S3APIURL string `json:"s3ApiUrl"` + AbsoluteMinimumPartSize int64 `json:"absoluteMinimumPartSize"` + RecommendedPartSize int64 `json:"recommendedPartSize"` + Allowed struct { + Buckets []string `json:"buckets"` + NamePrefix *string `json:"namePrefix"` + } `json:"allowed"` + } `json:"storageApi"` + } `json:"apiInfo"` + } + if err := json.NewDecoder(response.Body).Decode(&body); err != nil { + t.Fatal(err) + } + api := body.APIInfo.StorageAPI + if body.AccountID != "a" || body.AuthorizationToken != "test-token" || api.APIURL != s.URL() || api.DownloadURL != s.URL() || api.S3APIURL != s.URL() || api.AbsoluteMinimumPartSize != 5000000 || api.RecommendedPartSize != 100000000 || api.Allowed.Buckets != nil || api.Allowed.NamePrefix != nil { + t.Errorf("authorization response does not match built-in v4 shape: %+v", body) + } + }) + } + s.Reply("b2_authorize_account", http.StatusAccepted, `{"replacement":true}`) + if response := get(t, s.URL()+"/b2api/v4/b2_authorize_account"); response.StatusCode != http.StatusAccepted { + t.Errorf("scripted authorize status = %d, want 202", response.StatusCode) + } +} + +func TestUploadURLsAreServedAndRecorded(t *testing.T) { + s := New(t) + for _, tc := range []struct{ api, path, token string }{ + {"b2_get_upload_url", "/upload_file", "upload-token"}, + {"b2_get_upload_part_url", "/upload_part", "upload-part-token"}, + } { + t.Run(tc.api, func(t *testing.T) { + response := get(t, s.URL()+"/b2api/v4/"+tc.api) + var upload struct { + URL string `json:"uploadUrl"` + Token string `json:"authorizationToken"` + } + if err := json.NewDecoder(response.Body).Decode(&upload); err != nil { + t.Fatal(err) + } + if upload.URL != s.URL()+tc.path || upload.Token != tc.token { + t.Errorf("upload credentials = %+v", upload) + } + posted, err := http.Post(upload.URL, "application/octet-stream", strings.NewReader("raw-body")) + if err != nil { + t.Fatal(err) + } + data, err := io.ReadAll(posted.Body) + posted.Body.Close() + if err != nil || posted.StatusCode != http.StatusOK || string(data) != `{}` { + t.Errorf("upload response = %d %q, err %v", posted.StatusCode, data, err) + } + record := s.Last(t, strings.TrimPrefix(tc.path, "/")) + if record.Path != tc.path || string(record.Body) != "raw-body" || record.JSON != nil { + t.Errorf("upload record = %+v", record) + } + }) + } +} + +func TestReplySequenceRepeatsLast(t *testing.T) { + s := New(t) + s.ReplySequence("b2_copy_file", Response{Status: 503, Body: "retry"}, Response{Status: 200, Body: "ok", Header: http.Header{"X-Test": {"response"}}}) + for i, want := range []struct { + status int + body string + }{{503, "retry"}, {200, "ok"}, {200, "ok"}} { + response := get(t, s.URL()+"/b2api/v4/b2_copy_file") + data, err := io.ReadAll(response.Body) + if err != nil { + t.Fatal(err) + } + if response.StatusCode != want.status || string(data) != want.body { + t.Errorf("reply %d = %d %q, want %d %q", i, response.StatusCode, data, want.status, want.body) + } + if i > 0 && response.Header.Get("X-Test") != "response" { + t.Errorf("reply %d header = %q", i, response.Header.Get("X-Test")) + } + } + s.Reply("b2_copy_file", 201, "replacement") + if response := get(t, s.URL()+"/b2api/v4/b2_copy_file"); response.StatusCode != 201 { + t.Errorf("replacement status = %d, want 201", response.StatusCode) + } +} + +func TestHandleSeesTheRequest(t *testing.T) { + s := New(t) + s.Handle("b2_create_bucket", func(r Request) Response { + // Calling Requests here would deadlock if the handler ran under the server mutex. + if len(s.Requests()) != 1 { + return Response{Status: 500, Body: "request was not recorded before Handle"} + } + return Response{Status: 201, Body: fmt.Sprintf(`{"name":%q}`, r.JSON["bucketName"])} + }) + response, err := http.Post(s.URL()+"/b2api/v4/b2_create_bucket", "application/json", strings.NewReader(`{"bucketName":"test"}`)) + if err != nil { + t.Fatal(err) + } + defer response.Body.Close() + if response.StatusCode != 201 { + t.Fatalf("status = %d, want 201", response.StatusCode) + } + var body map[string]any + if err := json.NewDecoder(response.Body).Decode(&body); err != nil { + t.Fatal(err) + } + if body["name"] != "test" { + t.Errorf("handler response = %v, want name test", body) + } +} + +func TestUnscriptedCallFails(t *testing.T) { + fake := &recordingTB{TB: t} + s := New(fake) + response := get(t, s.URL()+"/b2api/v4/b2_unknown") + if response.StatusCode != 400 { + t.Errorf("status = %d, want 400", response.StatusCode) + } + var body struct { + Status int `json:"status"` + Code string `json:"code"` + Message string `json:"message"` + } + if err := json.NewDecoder(response.Body).Decode(&body); err != nil { + t.Fatal(err) + } + if body.Status != 400 || body.Code != "bad_request" || body.Message != "b2wiretest: no response for b2_unknown" { + t.Errorf("error body = %+v", body) + } + if messages := fake.errors(); len(messages) != 1 || !strings.Contains(messages[0], "b2_unknown") { + t.Errorf("error reports = %v, want b2_unknown", messages) + } + if len(s.Calls("b2_unknown")) != 1 { + t.Error("unscripted call was not recorded") + } + missing := &recordingTB{TB: t} + s.Last(missing, "never_called") + if messages := missing.errors(); len(messages) != 1 || !strings.Contains(messages[0], "never_called") { + t.Errorf("Last failure = %v, want never_called", messages) + } +} + +func TestAssertJSONReportsGotAndWant(t *testing.T) { + r := Request{Call: "b2_update_bucket", JSON: map[string]any{"count": float64(5), "bucketIds": []any{"a"}}} + fake := &recordingTB{TB: t} + AssertJSON(fake, r, "count", int64(5)) + AssertJSON(fake, r, "bucketIds", []string{"a"}) + if messages := fake.errors(); len(messages) != 0 { + t.Errorf("equivalent JSON values reported errors: %v", messages) + } + AssertJSON(fake, r, "count", 6) + AssertJSON(fake, r, "bucketIds.1", "b") + messages := fake.errors() + if len(messages) != 2 || !strings.Contains(messages[0], "b2_update_bucket body count: got 5, want 6") || !strings.Contains(messages[1], "b2_update_bucket body bucketIds.1: missing") { + t.Errorf("assertion reports = %v", messages) + } +} + +func TestAssertNoJSONAndAssertHeader(t *testing.T) { + r := Request{Call: "b2_create_bucket", JSON: map[string]any{"nested": map[string]any{"present": true}}, Header: http.Header{"X-Test": {"value"}}} + fake := &recordingTB{TB: t} + AssertNoJSON(fake, r, "nested.absent") + AssertHeader(fake, r, "X-Test", "value") + if messages := fake.errors(); len(messages) != 0 { + t.Errorf("matching assertions reported errors: %v", messages) + } + AssertNoJSON(fake, r, "nested.present") + AssertHeader(fake, r, "X-Test", "other") + messages := fake.errors() + if len(messages) != 2 || !strings.Contains(messages[0], "b2_create_bucket body nested.present: got true, want missing") || !strings.Contains(messages[1], `b2_create_bucket header X-Test: got "value", want "other"`) { + t.Errorf("assertion reports = %v", messages) + } +} From 7a6cfc98255935a8c6a08defcf4631634690097b Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Wed, 28 Oct 2026 13:36:16 -0600 Subject: [PATCH 04/26] test(b2): use shared recorder for CORS rule name --- b2/update_cors_name_test.go | 34 ++++++---------------------------- 1 file changed, 6 insertions(+), 28 deletions(-) diff --git a/b2/update_cors_name_test.go b/b2/update_cors_name_test.go index aceb9ce5..2c490f76 100644 --- a/b2/update_cors_name_test.go +++ b/b2/update_cors_name_test.go @@ -11,6 +11,7 @@ import ( "testing" "github.com/Backblaze/blazer/base" + "github.com/Backblaze/blazer/internal/b2wiretest" ) const updateAuthJSON = `{"accountId":"a","authorizationToken":"t","applicationKeyExpirationTimestamp":0,"apiInfo":{"storageApi":{"absoluteMinimumPartSize":5000000,"apiUrl":"%[1]s","capabilities":[],"downloadUrl":"%[1]s","storageApi":"storage","recommendedPartSize":100000000,"s3ApiUrl":"%[1]s","allowed":{"buckets":null,"capabilities":[],"namePrefix":null}}}}` @@ -20,29 +21,12 @@ const updateBucketJSON = `{"bucketId":"bid","bucketName":"n","bucketType":"allPr `"replicationConfiguration":{"isClientAuthorizedToRead":true,"value":{"asReplicationSource":null,"asReplicationDestination":null}}}` func TestUpdateBucketSendsCORSRuleName(t *testing.T) { - var srv *httptest.Server - var sent map[string]any - srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch { - case strings.HasSuffix(r.URL.Path, "/b2_authorize_account"): - fmt.Fprintf(w, updateAuthJSON, srv.URL) - case strings.HasSuffix(r.URL.Path, "/b2_create_bucket"): - fmt.Fprint(w, updateBucketJSON) - case strings.HasSuffix(r.URL.Path, "/b2_update_bucket"): - body, _ := io.ReadAll(r.Body) - if err := json.Unmarshal(body, &sent); err != nil { - t.Errorf("decode update body: %v", err) - } - fmt.Fprint(w, updateBucketJSON) - default: - t.Errorf("unexpected request path: %s", r.URL.Path) - http.NotFound(w, r) - } - })) - defer srv.Close() + srv := b2wiretest.New(t) + srv.Reply("b2_create_bucket", http.StatusOK, updateBucketJSON) + srv.Reply("b2_update_bucket", http.StatusOK, updateBucketJSON) ctx := context.Background() - root, err := base.AuthorizeAccount(ctx, "a", "k", base.SetAPIBase(srv.URL)) + root, err := base.AuthorizeAccount(ctx, "a", "k", base.SetAPIBase(srv.URL())) if err != nil { t.Fatal(err) } @@ -59,13 +43,7 @@ func TestUpdateBucketSendsCORSRuleName(t *testing.T) { if err != nil { t.Fatalf("updateBucket: %v", err) } - rules, ok := sent["corsRules"].([]any) - if !ok || len(rules) != 1 { - t.Fatalf("update request corsRules = %v, want one rule", sent["corsRules"]) - } - if name := rules[0].(map[string]any)["corsRuleName"]; name != "browser" { - t.Errorf("update request corsRuleName = %v, want browser", name) - } + b2wiretest.AssertJSON(t, srv.Last(t, "b2_update_bucket"), "corsRules.0.corsRuleName", "browser") } func TestReadModifyWriteKeepsCORSRuleName(t *testing.T) { From 99f64a07fad0e469e1c01fe5816d773c003f16ed Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Wed, 28 Oct 2026 13:37:29 -0600 Subject: [PATCH 05/26] test(internal): check recording server response closes --- internal/b2wiretest/server_test.go | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/internal/b2wiretest/server_test.go b/internal/b2wiretest/server_test.go index 3ac4e79e..d1b03b4a 100644 --- a/internal/b2wiretest/server_test.go +++ b/internal/b2wiretest/server_test.go @@ -37,7 +37,11 @@ func get(t *testing.T, url string) *http.Response { if err != nil { t.Fatal(err) } - t.Cleanup(func() { response.Body.Close() }) + t.Cleanup(func() { + if err := response.Body.Close(); err != nil { + t.Errorf("close response body: %v", err) + } + }) return response } @@ -53,7 +57,9 @@ func TestRecordsMethodPathHeadersAndJSON(t *testing.T) { if err != nil { t.Fatal(err) } - response.Body.Close() + if err := response.Body.Close(); err != nil { + t.Fatal(err) + } if response.StatusCode != http.StatusOK { t.Fatalf("status = %d, want 200", response.StatusCode) } @@ -106,7 +112,10 @@ func TestRecordsInArrivalOrderUnderConcurrency(t *testing.T) { errors <- err return } - response.Body.Close() + if err := response.Body.Close(); err != nil { + errors <- err + return + } if response.StatusCode != http.StatusOK { errors <- fmt.Errorf("status = %d", response.StatusCode) } @@ -191,7 +200,9 @@ func TestUploadURLsAreServedAndRecorded(t *testing.T) { t.Fatal(err) } data, err := io.ReadAll(posted.Body) - posted.Body.Close() + if closeErr := posted.Body.Close(); closeErr != nil { + t.Error(closeErr) + } if err != nil || posted.StatusCode != http.StatusOK || string(data) != `{}` { t.Errorf("upload response = %d %q, err %v", posted.StatusCode, data, err) } @@ -241,7 +252,11 @@ func TestHandleSeesTheRequest(t *testing.T) { if err != nil { t.Fatal(err) } - defer response.Body.Close() + t.Cleanup(func() { + if err := response.Body.Close(); err != nil { + t.Errorf("close response body: %v", err) + } + }) if response.StatusCode != 201 { t.Fatalf("status = %d, want 201", response.StatusCode) } From ccc6d5537c437e9bbcd2b37471a037d1b32f4710 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Wed, 28 Oct 2026 13:40:58 -0600 Subject: [PATCH 06/26] test(b2): retain CORS rule count assertion --- b2/update_cors_name_test.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/b2/update_cors_name_test.go b/b2/update_cors_name_test.go index 2c490f76..35cf6c55 100644 --- a/b2/update_cors_name_test.go +++ b/b2/update_cors_name_test.go @@ -43,7 +43,12 @@ func TestUpdateBucketSendsCORSRuleName(t *testing.T) { if err != nil { t.Fatalf("updateBucket: %v", err) } - b2wiretest.AssertJSON(t, srv.Last(t, "b2_update_bucket"), "corsRules.0.corsRuleName", "browser") + request := srv.Last(t, "b2_update_bucket") + rules, ok := request.JSON["corsRules"].([]any) + if !ok || len(rules) != 1 { + t.Fatalf("update request corsRules = %v, want one rule", request.JSON["corsRules"]) + } + b2wiretest.AssertJSON(t, request, "corsRules.0.corsRuleName", "browser") } func TestReadModifyWriteKeepsCORSRuleName(t *testing.T) { From 17d361bdf00375385997228b8af78818c7af62d2 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Wed, 28 Oct 2026 13:56:56 -0600 Subject: [PATCH 07/26] test(internal): reproduce shared JSON map race --- internal/b2wiretest/server_test.go | 60 ++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/internal/b2wiretest/server_test.go b/internal/b2wiretest/server_test.go index d1b03b4a..2a81a4ab 100644 --- a/internal/b2wiretest/server_test.go +++ b/internal/b2wiretest/server_test.go @@ -97,6 +97,66 @@ func TestRecordsMethodPathHeadersAndJSON(t *testing.T) { } } +func TestCallsReturnsIndependentJSON(t *testing.T) { + s := New(t) + s.Reply("b2_create_key", http.StatusOK, `{}`) + response, err := http.Post(s.URL()+"/b2api/v4/b2_create_key", "application/json", strings.NewReader(`{"keyName":"test"}`)) + if err != nil { + t.Fatal(err) + } + if err := response.Body.Close(); err != nil { + t.Fatal(err) + } + + call := "b2_create_key" + s.Calls(call)[0].JSON["added"] = true + if _, ok := s.Calls(call)[0].JSON["added"]; ok { + t.Error("returned JSON changed the recorded request") + } +} + +func TestHandleAndCallsCanReadJSONConcurrently(t *testing.T) { + s := New(t) + handlerStarted := make(chan struct{}) + readJSON := make(chan struct{}) + s.Handle("b2_create_bucket", func(r Request) Response { + close(handlerStarted) + <-readJSON + for range 10000 { + if r.JSON["bucketName"] != "test" { + return Response{Status: http.StatusInternalServerError, Body: "wrong request JSON"} + } + } + return Response{Status: http.StatusOK, Body: `{}`} + }) + + responseDone := make(chan error, 1) + go func() { + response, err := http.Post(s.URL()+"/b2api/v4/b2_create_bucket", "application/json", strings.NewReader(`{"bucketName":"test"}`)) + if err == nil { + err = response.Body.Close() + } + responseDone <- err + }() + <-handlerStarted + + callsStarted := make(chan struct{}) + callsDone := make(chan struct{}) + go func() { + defer close(callsDone) + close(callsStarted) + for range 10000 { + _ = s.Calls("b2_create_bucket") + } + }() + <-callsStarted + close(readJSON) + <-callsDone + if err := <-responseDone; err != nil { + t.Fatal(err) + } +} + func TestRecordsInArrivalOrderUnderConcurrency(t *testing.T) { s := New(t) s.Reply("b2_list_buckets", http.StatusOK, `{}`) From 3f6f07786a5de7917bbc8fc7587de3f242d5e72f Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Wed, 28 Oct 2026 13:57:34 -0600 Subject: [PATCH 08/26] fix(internal): isolate recorded JSON maps --- internal/b2wiretest/server.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/b2wiretest/server.go b/internal/b2wiretest/server.go index 92594966..e71c5574 100644 --- a/internal/b2wiretest/server.go +++ b/internal/b2wiretest/server.go @@ -114,6 +114,7 @@ func copyRequests(requests []Request) []Request { result[i].Header = r.Header.Clone() result[i].Body = append([]byte(nil), r.Body...) if r.JSON != nil { + result[i].JSON = nil _ = json.Unmarshal(result[i].Body, &result[i].JSON) } } From a73326144ecbaa9be156598d95dbb825210fc3be Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Wed, 28 Oct 2026 13:59:00 -0600 Subject: [PATCH 09/26] docs: direct wire tests to shared recorder --- AGENTS.md | 2 ++ docs/testing.md | 2 +- intent/b2wiretest/plan.md | 1 + 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index ef5aaa6a..05d1a629 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,6 +24,7 @@ coding agents and new contributors; `CLAUDE.md` points here. Keep it under about | `b2/` | Public, high-level API (`Client`, `Bucket`, `Object`, `Writer`, `Reader`). Users import this. | | `base/` | Low-level wire layer: one function per B2 call, request/response JSON, retry and reauth. | | `internal/b2types/` | JSON request/response structs shared by `base`. Wire-shape changes start here. | +| `internal/b2wiretest/` | recording `httptest` B2 server for wire tests; imported only from `_test.go`. | | `internal/retry/`, `internal/blog/` | Backoff helpers and debug logging. | | `x/` | Experimental packages (`consistent`, `transport`, `window`); weaker compatibility promise. | | `bin/b2keys/` | Key-management CLI, its own module. | @@ -59,6 +60,7 @@ Each line was raised by a reviewer on at least two PRs. 2. **Tests that do not pin the fix.** Revert the change and the new test must fail. Prove it in the PR. Assert the request body sent on the wire (`httptest` server or `RoundTripper` stub through `NewClient`), not only the fake backend or a decoded response. + New wire tests use `internal/b2wiretest`; existing per-feature fakes stay until a test is rewritten. 3. **Read-modify-write on `b2_update_bucket`.** Omit fields the caller did not change (`lifecycleRules`, `defaultRetention`, `defaultServerSideEncryption`, `replicationConfiguration`). Sending cached values needs extra capabilities and can regenerate rule IDs. Test two updates in a row on the same handle. diff --git a/docs/testing.md b/docs/testing.md index 26f54b7f..30f4ec63 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -8,7 +8,7 @@ how to run it, and how to show that a test pins the behavior a PR claims. | Level | Where | Run it with | Proves | Does not prove | | --- | --- | --- | --- | --- | | Unit, fake backend | `b2/*_test.go` using the fake in `b2/b2_test.go` | `go test ./...` | Logic inside the `b2` package | What is sent on the wire | -| Wire | `httptest` server or `RoundTripper` stub through `NewClient` (`*_wire_test.go`) | `go test ./...` | The exact request body, headers and query, and how a response is decoded | That B2 answers that way | +| Wire | New tests use `internal/b2wiretest`; existing per-feature fakes stay until rewritten (`*_wire_test.go`) | `go test ./...` | The exact request body, headers and query, and how a response is decoded | That B2 answers that way | | Repo-owned SDK harness contract | `.sdkharness/` ([README](../.sdkharness/README.md)) | See below | Behavior against the B2 simulator ([`backblaze-labs/b2-simulator`](https://github.com/backblaze-labs/b2-simulator)), one scenario per capability | Behavior the simulator does not model (encryption keys, retention, replication effects) | | Live integration | `b2/integration_test.go` | `B2_APPLICATION_KEY_ID=... B2_APPLICATION_KEY=... go test ./b2/` | Real B2 behavior for that account on that day | Other accounts, regions or later B2 releases | diff --git a/intent/b2wiretest/plan.md b/intent/b2wiretest/plan.md index 2add7bd1..18cb3980 100644 --- a/intent/b2wiretest/plan.md +++ b/intent/b2wiretest/plan.md @@ -11,6 +11,7 @@ internal/b2wiretest/server.go (new) R1-R8 internal/b2wiretest/assert.go (new) R9 internal/b2wiretest/server_test.go (new) R10 b2/update_cors_name_test.go (edit) R11: TestUpdateBucketSendsCORSRuleName uses the helper +AGENTS.md, docs/testing.md (edit) Review finding: direct new wire tests to `internal/b2wiretest`; retain per-feature fakes until rewritten intent/b2wiretest/plan.md (edit only if the implementation departs from this plan) ``` From a69a11a2797c94f840c4a6b05f45c195b7ca32e3 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Wed, 28 Oct 2026 14:46:17 -0600 Subject: [PATCH 10/26] docs(intent): record wire-test query parameters --- intent/b2wiretest/plan.md | 9 +++++---- intent/b2wiretest/spec.md | 7 +++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/intent/b2wiretest/plan.md b/intent/b2wiretest/plan.md index 18cb3980..3ff5b517 100644 --- a/intent/b2wiretest/plan.md +++ b/intent/b2wiretest/plan.md @@ -35,7 +35,8 @@ are untouched. the dotted path (map key or decimal index), compare with `reflect.DeepEqual`, and report ` body : got , want ` (or `missing`). Use `%s` with `json.Marshal` output so values print as JSON. -6. **Helper tests** (R10) in `server_test.go`. Table-driven where it fits, using `net/http` directly: +6. **Query recording** (R4, R9). Full-depth review finding: copy decoded query parameters and assert them. +7. **Helper tests** (R10) in `server_test.go`. Table-driven where it fits, using `net/http` directly: - `TestRecordsMethodPathHeadersAndJSON`: a POST with a header and a JSON body is recorded with each field, `Call` set, and `Seq` 1. - `TestRecordsInArrivalOrderUnderConcurrency`: 50 concurrent requests; 50 records with distinct `Seq` @@ -51,15 +52,15 @@ are untouched. - `TestAssertJSONReportsGotAndWant`: the same fake TB; a mismatch reports call, path, got and want; a missing path reports `missing`; `int64(5)` matches `5.0`; `[]string{"a"}` matches `[]any{"a"}`. - `TestAssertNoJSONAndAssertHeader`. -7. **Port one test** (R11). Rewrite `TestUpdateBucketSendsCORSRuleName` on the helper: script +8. **Port one test** (R11). Rewrite `TestUpdateBucketSendsCORSRuleName` on the helper: script `b2_create_bucket` and `b2_update_bucket` with the existing `updateBucketJSON`, then use `AssertJSON` on `Last(t, "b2_update_bucket")` for `corsRules.0.corsRuleName` = `"browser"` (B2 docs: `b2_update_bucket`, `corsRules[].corsRuleName`). Keep `updateAuthJSON` and `updateBucketJSON`, since other tests in the file use them. Keep the test name. -8. **Check and self-review.** `make ci`; then a fresh-context review of the diff (`/code-review`, or a +9. **Check and self-review.** `make ci`; then a fresh-context review of the diff (`/code-review`, or a subagent that did not write it) per `AGENTS.md`, answering each finding. -Steps 1 to 5 are one file pair and run in sequence. Step 6 follows 5. Step 7 depends only on steps 1 to +Steps 1 to 6 are one file pair and run in sequence. Step 7 follows 6. Step 8 depends only on steps 1 to 4. There is nothing to run in parallel inside this change. ## Risks diff --git a/intent/b2wiretest/spec.md b/intent/b2wiretest/spec.md index 4c5d448f..2a8b8275 100644 --- a/intent/b2wiretest/spec.md +++ b/intent/b2wiretest/spec.md @@ -15,8 +15,9 @@ Skills applied: none beyond `AGENTS.md`, `REVIEW.md` and `docs/testing.md` (no p `recommendedPartSize` 100000000. A test can replace it like any other call (R5). - **R4. Recording.** Every request is recorded in arrival order as a `Request` with: `Seq` (1-based order), `Method`, `Path`, `Call` (the last path segment for API calls, e.g. `b2_create_key`; `upload_file` and - `upload_part` for upload URLs), `Header` (a copy), `Body` (raw bytes) and `JSON` (`map[string]any` - decoded from the body when it is a JSON object, otherwise nil). Recording is safe for concurrent requests. + `upload_part` for upload URLs), `Query` (`url.Values` decoded from `r.URL.RawQuery`), `Header` (a copy), + `Body` (raw bytes) and `JSON` (`map[string]any` decoded from the body when it is a JSON object, otherwise + nil). Recording is safe for concurrent requests. - **R5. Scripted responses.** Per call name: - `Reply(call string, status int, body string)`: the same response every time; - `ReplySequence(call string, rs ...Response)`: successive responses, the last one repeating; @@ -41,6 +42,8 @@ Skills applied: none beyond `AGENTS.md`, `REVIEW.md` and `docs/testing.md` (no p `int64(5)` and `5.0` compare equal, as do a `[]string` and a `[]any` with the same elements. - `AssertNoJSON(t, r Request, path string)`: the field is absent. - `AssertHeader(t, r Request, name, want string)`. + - `AssertQuery(t, r Request, name, want string)`: reports ` query : got X, want Y` (or + `missing` when absent). The helper never computes expected values. Tests supply them from the B2 API docs. - **R10. Tests of the helper.** `internal/b2wiretest` has its own tests for R2 to R9, using `net/http` From 8fc8cf967caec280da87438c8c9ddad6726ba386 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Wed, 28 Oct 2026 14:48:27 -0600 Subject: [PATCH 11/26] test(internal): record wire-test query parameters --- internal/b2wiretest/assert.go | 12 +++++++++++ internal/b2wiretest/server.go | 10 ++++++++- internal/b2wiretest/server_test.go | 34 ++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/internal/b2wiretest/assert.go b/internal/b2wiretest/assert.go index f060aa9a..b9970397 100644 --- a/internal/b2wiretest/assert.go +++ b/internal/b2wiretest/assert.go @@ -49,6 +49,18 @@ func AssertHeader(t testing.TB, r Request, name, want string) { } } +// AssertQuery checks one request query parameter's value. +func AssertQuery(t testing.TB, r Request, name, want string) { + t.Helper() + if _, ok := r.Query[name]; !ok { + t.Errorf("%s query %s: missing, want %q", r.Call, name, want) + return + } + if got := r.Query.Get(name); got != want { + t.Errorf("%s query %s: got %q, want %q", r.Call, name, got, want) + } +} + func jsonPath(root map[string]any, path string) (any, bool) { if path == "" { return nil, false diff --git a/internal/b2wiretest/server.go b/internal/b2wiretest/server.go index e71c5574..12b90e3d 100644 --- a/internal/b2wiretest/server.go +++ b/internal/b2wiretest/server.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" "strings" "sync" "testing" @@ -18,6 +19,7 @@ type Request struct { Method string Path string Call string + Query url.Values Header http.Header Body []byte JSON map[string]any @@ -111,6 +113,12 @@ func copyRequests(requests []Request) []Request { result := make([]Request, len(requests)) for i, r := range requests { result[i] = r + if r.Query != nil { + result[i].Query = make(url.Values, len(r.Query)) + for name, values := range r.Query { + result[i].Query[name] = append([]string(nil), values...) + } + } result[i].Header = r.Header.Clone() result[i].Body = append([]byte(nil), r.Body...) if r.JSON != nil { @@ -129,7 +137,7 @@ func (s *Server) serve(w http.ResponseWriter, r *http.Request) { return } call := r.URL.Path[strings.LastIndexByte(r.URL.Path, '/')+1:] - req := Request{Method: r.Method, Path: r.URL.Path, Call: call, Header: r.Header.Clone(), Body: body} + req := Request{Method: r.Method, Path: r.URL.Path, Call: call, Query: r.URL.Query(), Header: r.Header.Clone(), Body: body} if len(body) > 0 { _ = json.Unmarshal(body, &req.JSON) } diff --git a/internal/b2wiretest/server_test.go b/internal/b2wiretest/server_test.go index 2a81a4ab..f819b9cb 100644 --- a/internal/b2wiretest/server_test.go +++ b/internal/b2wiretest/server_test.go @@ -6,6 +6,7 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" "strings" "sync" "testing" @@ -115,6 +116,28 @@ func TestCallsReturnsIndependentJSON(t *testing.T) { } } +func TestRecordsQuery(t *testing.T) { + s := New(t) + call := "b2_get_bucket_notification_rules" + s.Reply(call, http.StatusOK, `{}`) + response := get(t, s.URL()+"/b2api/v4/"+call+"?bucketId=b-1") + if response.StatusCode != http.StatusOK { + t.Fatalf("status = %d, want 200", response.StatusCode) + } + + record := s.Calls(call)[0] + if record.Path != "/b2api/v4/b2_get_bucket_notification_rules" || record.Call != call { + t.Errorf("record = %+v, want path and call for b2_get_bucket_notification_rules", record) + } + if got := record.Query.Get("bucketId"); got != "b-1" { + t.Fatalf("bucketId = %q, want b-1", got) + } + record.Query.Set("bucketId", "changed") + if got := s.Calls(call)[0].Query.Get("bucketId"); got != "b-1" { + t.Errorf("returned Query changed the recorded request: got %q, want b-1", got) + } +} + func TestHandleAndCallsCanReadJSONConcurrently(t *testing.T) { s := New(t) handlerStarted := make(chan struct{}) @@ -391,3 +414,14 @@ func TestAssertNoJSONAndAssertHeader(t *testing.T) { t.Errorf("assertion reports = %v", messages) } } + +func TestAssertQueryReportsGotAndWant(t *testing.T) { + r := Request{Call: "b2_get_bucket_notification_rules", Query: url.Values{"bucketId": {"b-1"}}} + fake := &recordingTB{TB: t} + AssertQuery(fake, r, "bucketId", "other") + AssertQuery(fake, r, "missing", "value") + messages := fake.errors() + if len(messages) != 2 || !strings.Contains(messages[0], `b2_get_bucket_notification_rules query bucketId: got "b-1", want "other"`) || !strings.Contains(messages[1], `b2_get_bucket_notification_rules query missing: missing, want "value"`) { + t.Errorf("assertion reports = %v", messages) + } +} From b61fcd8ccc068fd9d6b7761131ce99105ace5d78 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Wed, 28 Oct 2026 15:05:31 -0600 Subject: [PATCH 12/26] fix(internal): keep request data out of the unscripted response --- intent/b2wiretest/spec.md | 2 +- internal/b2wiretest/server.go | 5 +++-- internal/b2wiretest/server_test.go | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/intent/b2wiretest/spec.md b/intent/b2wiretest/spec.md index 2a8b8275..f33ecd50 100644 --- a/intent/b2wiretest/spec.md +++ b/intent/b2wiretest/spec.md @@ -26,7 +26,7 @@ Skills applied: none beyond `AGENTS.md`, `REVIEW.md` and `docs/testing.md` (no p replaces the earlier one. - **R6. Unscripted calls fail loudly.** A request to a call with no response registered (other than the R3 and R7 built-ins) is recorded, answered with status 400 and a B2-shaped error body - (`{"status":400,"code":"bad_request","message":"b2wiretest: no response for "}`), and reported + (`{"status":400,"code":"bad_request","message":"b2wiretest: no response scripted for this call"}`), and reported with `t.Errorf` naming the call. - **R7. Upload URLs.** Without scripting, `b2_get_upload_url` and `b2_get_upload_part_url` return `uploadUrl` = server URL + `/upload_file` or `/upload_part`, and authorization tokens `"upload-token"` diff --git a/internal/b2wiretest/server.go b/internal/b2wiretest/server.go index 12b90e3d..3d3fcee5 100644 --- a/internal/b2wiretest/server.go +++ b/internal/b2wiretest/server.go @@ -32,6 +32,8 @@ type Response struct { Header http.Header } +const unscriptedResponseBody = `{"status":400,"code":"bad_request","message":"b2wiretest: no response scripted for this call"}` + type script struct { responses []Response handler func(Request) Response @@ -168,8 +170,7 @@ func (s *Server) serve(w http.ResponseWriter, r *http.Request) { } if !ok || response.Status == 0 { s.t.Errorf("b2wiretest: no response for %s", call) - response = Response{Status: http.StatusBadRequest, - Body: fmt.Sprintf(`{"status":400,"code":"bad_request","message":"b2wiretest: no response for %s"}`, call)} + response = Response{Status: http.StatusBadRequest, Body: unscriptedResponseBody} } for name, values := range response.Header { for _, value := range values { diff --git a/internal/b2wiretest/server_test.go b/internal/b2wiretest/server_test.go index f819b9cb..08f26632 100644 --- a/internal/b2wiretest/server_test.go +++ b/internal/b2wiretest/server_test.go @@ -367,7 +367,7 @@ func TestUnscriptedCallFails(t *testing.T) { if err := json.NewDecoder(response.Body).Decode(&body); err != nil { t.Fatal(err) } - if body.Status != 400 || body.Code != "bad_request" || body.Message != "b2wiretest: no response for b2_unknown" { + if body.Status != 400 || body.Code != "bad_request" || body.Message != "b2wiretest: no response scripted for this call" { t.Errorf("error body = %+v", body) } if messages := fake.errors(); len(messages) != 1 || !strings.Contains(messages[0], "b2_unknown") { From 57d42fa847587afa414eda5b5652b0f11857994a Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Thu, 29 Oct 2026 09:18:23 -0600 Subject: [PATCH 13/26] test(internal): verify recording server cleanup --- internal/b2wiretest/server_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/b2wiretest/server_test.go b/internal/b2wiretest/server_test.go index 08f26632..3041f7f4 100644 --- a/internal/b2wiretest/server_test.go +++ b/internal/b2wiretest/server_test.go @@ -46,6 +46,22 @@ func get(t *testing.T, url string) *http.Response { return response } +func TestNewClosesServerAtCleanup(t *testing.T) { + var url string + t.Run("server", func(t *testing.T) { + srv := New(t) + url = srv.URL() + }) + + response, err := http.Get(url + "/b2api/v4/b2_authorize_account") + if err == nil { + if closeErr := response.Body.Close(); closeErr != nil { + t.Error(closeErr) + } + t.Fatal("server still accepts requests after test cleanup") + } +} + func TestRecordsMethodPathHeadersAndJSON(t *testing.T) { s := New(t) s.Reply("b2_create_key", http.StatusOK, `{}`) From e2d57269e21eb34cf1a249e7b3207644c8f769fe Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Thu, 29 Oct 2026 09:47:52 -0600 Subject: [PATCH 14/26] fix(internal): set recording response security headers --- internal/b2wiretest/server.go | 4 ++++ internal/b2wiretest/server_test.go | 15 ++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/internal/b2wiretest/server.go b/internal/b2wiretest/server.go index 3d3fcee5..38d05ca8 100644 --- a/internal/b2wiretest/server.go +++ b/internal/b2wiretest/server.go @@ -177,6 +177,10 @@ func (s *Server) serve(w http.ResponseWriter, r *http.Request) { w.Header().Add(name, value) } } + if w.Header().Get("Content-Type") == "" { + w.Header().Set("Content-Type", "application/json") + } + w.Header().Set("X-Content-Type-Options", "nosniff") w.WriteHeader(response.Status) if _, err := io.WriteString(w, response.Body); err != nil { s.t.Errorf("b2wiretest: write %s response: %v", call, err) diff --git a/internal/b2wiretest/server_test.go b/internal/b2wiretest/server_test.go index 3041f7f4..62cdad1b 100644 --- a/internal/b2wiretest/server_test.go +++ b/internal/b2wiretest/server_test.go @@ -315,7 +315,7 @@ func TestUploadURLsAreServedAndRecorded(t *testing.T) { func TestReplySequenceRepeatsLast(t *testing.T) { s := New(t) - s.ReplySequence("b2_copy_file", Response{Status: 503, Body: "retry"}, Response{Status: 200, Body: "ok", Header: http.Header{"X-Test": {"response"}}}) + s.ReplySequence("b2_copy_file", Response{Status: 503, Body: "retry"}, Response{Status: 200, Body: "ok", Header: http.Header{"Content-Type": {"text/plain"}, "X-Test": {"response"}}}) for i, want := range []struct { status int body string @@ -331,6 +331,9 @@ func TestReplySequenceRepeatsLast(t *testing.T) { if i > 0 && response.Header.Get("X-Test") != "response" { t.Errorf("reply %d header = %q", i, response.Header.Get("X-Test")) } + if i > 0 && response.Header.Get("Content-Type") != "text/plain" { + t.Errorf("reply %d Content-Type = %q, want text/plain", i, response.Header.Get("Content-Type")) + } } s.Reply("b2_copy_file", 201, "replacement") if response := get(t, s.URL()+"/b2api/v4/b2_copy_file"); response.StatusCode != 201 { @@ -375,6 +378,12 @@ func TestUnscriptedCallFails(t *testing.T) { if response.StatusCode != 400 { t.Errorf("status = %d, want 400", response.StatusCode) } + if got := response.Header.Get("Content-Type"); got != "application/json" { + t.Errorf("Content-Type = %q, want application/json", got) + } + if got := response.Header.Get("X-Content-Type-Options"); got != "nosniff" { + t.Errorf("X-Content-Type-Options = %q, want nosniff", got) + } var body struct { Status int `json:"status"` Code string `json:"code"` @@ -434,6 +443,10 @@ func TestAssertNoJSONAndAssertHeader(t *testing.T) { func TestAssertQueryReportsGotAndWant(t *testing.T) { r := Request{Call: "b2_get_bucket_notification_rules", Query: url.Values{"bucketId": {"b-1"}}} fake := &recordingTB{TB: t} + AssertQuery(fake, r, "bucketId", "b-1") + if messages := fake.errors(); len(messages) != 0 { + t.Errorf("matching query reported errors: %v", messages) + } AssertQuery(fake, r, "bucketId", "other") AssertQuery(fake, r, "missing", "value") messages := fake.errors() From a9748f35c57cb178a702c9528d98a37600d72cf1 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Thu, 29 Oct 2026 10:24:51 -0600 Subject: [PATCH 15/26] docs(intent): plan credential-free tests for live-only calls (#203) --- intent/credential-free-live-calls/intent.md | 38 ++++++++++ intent/credential-free-live-calls/plan.md | 47 ++++++++++++ intent/credential-free-live-calls/spec.md | 81 +++++++++++++++++++++ 3 files changed, 166 insertions(+) create mode 100644 intent/credential-free-live-calls/intent.md create mode 100644 intent/credential-free-live-calls/plan.md create mode 100644 intent/credential-free-live-calls/spec.md diff --git a/intent/credential-free-live-calls/intent.md b/intent/credential-free-live-calls/intent.md new file mode 100644 index 00000000..f00267bf --- /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: draft. + +## 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 + +None. diff --git a/intent/credential-free-live-calls/plan.md b/intent/credential-free-live-calls/plan.md new file mode 100644 index 00000000..10729288 --- /dev/null +++ b/intent/credential-free-live-calls/plan.md @@ -0,0 +1,47 @@ +# 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. Apply the lean-code ladder from +`AGENTS.md`: reuse `internal/b2wiretest`, 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.Action`: return `Retry` (or the non-upload action) for the hang error | R8 test | +| 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`. diff --git a/intent/credential-free-live-calls/spec.md b/intent/credential-free-live-calls/spec.md new file mode 100644 index 00000000..b0eeca89 --- /dev/null +++ b/intent/credential-free-live-calls/spec.md @@ -0,0 +1,81 @@ +# 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, capabilities, expiration from milliseconds) and cursor `"k-3"`; a second call with `"k-3"` + sends `startApplicationKeyId` `"k-3"`. An empty `keys` list returns `io.EOF`. +- **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.** An upload whose transport stalls (`x/transport` `AfterNBytes` with a + blocking effect, or a handler that never responds) under a context deadline fails, and `Action(err)` is + `AttemptNewUpload`. +- **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 after roughly n body bytes; `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. From e287c78bd186a0c68b0a4f20df5298b96a38e505 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Thu, 29 Oct 2026 10:49:06 -0600 Subject: [PATCH 16/26] docs(intent): correct R8 to the shared-upload-token refusal the live test checks (#203) --- intent/credential-free-live-calls/plan.md | 2 +- intent/credential-free-live-calls/spec.md | 11 ++++++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/intent/credential-free-live-calls/plan.md b/intent/credential-free-live-calls/plan.md index 10729288..51456571 100644 --- a/intent/credential-free-live-calls/plan.md +++ b/intent/credential-free-live-calls/plan.md @@ -40,7 +40,7 @@ No production file changes (R13). | 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.Action`: return `Retry` (or the non-upload action) for the hang error | R8 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 diff --git a/intent/credential-free-live-calls/spec.md b/intent/credential-free-live-calls/spec.md index b0eeca89..08fb30c0 100644 --- a/intent/credential-free-live-calls/spec.md +++ b/intent/credential-free-live-calls/spec.md @@ -32,9 +32,14 @@ authorization" (the `Authorization` and `b2ContentDisposition` URL parameters); ### base (`httptest`, as the existing `base/*_test.go` do) -- **R8. Upload after a connection hang.** An upload whose transport stalls (`x/transport` `AfterNBytes` with a - blocking effect, or a handler that never responds) under a context deadline fails, and `Action(err)` is - `AttemptNewUpload`. +- **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 From 2a7cd81b75098f8818354c0fd5c88e6f0b0a1241 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Thu, 29 Oct 2026 11:17:27 -0600 Subject: [PATCH 17/26] test: add credential-free tests for calls only covered by live tests Add hermetic HTTP and transport coverage for calls previously exercised only by credentialed live tests. ListKeys capabilities mapping is tracked separately in #220. The transport Trigger test is omitted because its existing race is tracked in #176. Closes #203 --- b2/live_only_calls_wire_test.go | 214 ++++++++++++++++++++++++++++++++ base/live_only_calls_test.go | 199 +++++++++++++++++++++++++++++ x/transport/transport_test.go | 141 +++++++++++++++++++++ 3 files changed, 554 insertions(+) create mode 100644 b2/live_only_calls_wire_test.go create mode 100644 base/live_only_calls_test.go create mode 100644 x/transport/transport_test.go diff --git a/b2/live_only_calls_wire_test.go b/b2/live_only_calls_wire_test.go new file mode 100644 index 00000000..62caad27 --- /dev/null +++ b/b2/live_only_calls_wire_test.go @@ -0,0 +1,214 @@ +package b2 + +import ( + "context" + "errors" + "io" + "net/http" + "testing" + "time" + + "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"}`) + if err := bucket.Object("gone").Hide(context.Background()); err == nil { + t.Fatal("Hide succeeded after a scripted 400") + } + }) +} + +func TestBucketRevealWire(t *testing.T) { + t.Run("deletes newest hide marker", func(t *testing.T) { + srv, bucket := liveOnlyBucket(t) + srv.Reply("b2_list_file_versions", http.StatusOK, `{"files":[{"fileId":"hide-id","fileName":"gone","action":"hide","uploadTimestamp":0}],"nextFileName":"","nextFileId":""}`) + srv.Reply("b2_delete_file_version", http.StatusOK, `{}`) + if err := bucket.Reveal(context.Background(), "gone"); err != nil { + t.Fatal(err) + } + request := srv.Last(t, "b2_delete_file_version") + b2wiretest.AssertJSON(t, request, "fileId", "hide-id") + b2wiretest.AssertJSON(t, request, "fileName", "gone") + }) + t.Run("leaves a current upload alone", func(t *testing.T) { + srv, bucket := liveOnlyBucket(t) + srv.Reply("b2_list_file_versions", http.StatusOK, `{"files":[{"fileId":"upload-id","fileName":"gone","action":"upload","uploadTimestamp":0}],"nextFileName":"","nextFileId":""}`) + if err := bucket.Reveal(context.Background(), "gone"); err != nil { + t.Fatal(err) + } + 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"}`) + object := bucket.Object("folder/a b.txt") + const disposition = `attachment; filename="a b.txt"` + 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", object.Name()) + 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) + } + 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) + 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":0}],"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" || keys[1].Expires().UnixMilli() != 0 { + t.Errorf("second key = id=%q name=%q expires=%v", keys[1].ID(), keys[1].Name(), keys[1].Expires()) + } + 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":""}`) + iter := bucket.List(context.Background()) + if !iter.Next() { + t.Fatalf("current list returned no object: %v", iter.Err()) + } + if err := iter.Object().Cancel(context.Background()); err == nil { + t.Fatal("Cancel succeeded for an uploaded object") + } + 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..4b43c614 --- /dev/null +++ b/base/live_only_calls_test.go @@ -0,0 +1,199 @@ +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) + _, _ = 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) { + entered := make(chan struct{}) + observedCancel := 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) + <-r.Context().Done() + close(observedCancel) + 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 + "/wait", token: "upload-token", b2: root} + cancelled, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := upload.UploadFile(cancelled, strings.NewReader("x"), 1, "name", "text/plain", "sha1", nil); !errors.Is(err, context.Canceled) { + t.Errorf("already-cancelled request error = %v, want context.Canceled", err) + } else if got := Action(err); got != Punt { + t.Errorf("Action(%v) = %v, want Punt", err, got) + } + ctx, stop := context.WithTimeout(context.Background(), time.Second) + defer stop() + 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(), 3*time.Second) + defer cancelWait() + select { + case <-entered: + case <-wait.Done(): + t.Fatalf("deadline request never reached handler: %v", wait.Err()) + } + select { + case err := <-done: + if !errors.Is(err, context.DeadlineExceeded) { + t.Errorf("blocked request error = %v, want context.DeadlineExceeded", err) + } else if got := Action(err); got != Punt { + t.Errorf("Action(%v) = %v, want Punt", err, got) + } + case <-wait.Done(): + t.Fatalf("deadline request did not return: %v", wait.Err()) + } + select { + case <-observedCancel: + case <-wait.Done(): + t.Fatal("handler did not observe request deadline") + } +} + +func TestUnknownCertificateAuthority(t *testing.T) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = fmt.Fprint(w, `{}`) + })) + 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/x/transport/transport_test.go b/x/transport/transport_test.go new file mode 100644 index 00000000..e866cea6 --- /dev/null +++ b/x/transport/transport_test.go @@ -0,0 +1,141 @@ +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() + client := &http.Client{Transport: WithFailures(nil, FailureRate(1), Stall(time.Second))} + request, err := http.NewRequestWithContext(ctx, http.MethodGet, srv.URL+"/ok", nil) + if err != nil { + t.Fatal(err) + } + _, err = client.Do(request) + if !errors.Is(err, context.DeadlineExceeded) { + t.Errorf("error = %v, want context deadline exceeded", err) + } + }) +} + +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) + } +} + +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 +} From 0407c68eef7307d1ef674a71e0ce3611857cc715 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Thu, 29 Oct 2026 11:29:15 -0600 Subject: [PATCH 18/26] test(transport): pin Stall context cancellation Assert that Stall returns before its duration when the request context ends. Align R5 with the separate ListKeys capabilities work in #220. --- intent/credential-free-live-calls/spec.md | 3 ++- x/transport/transport_test.go | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/intent/credential-free-live-calls/spec.md b/intent/credential-free-live-calls/spec.md index 08fb30c0..dd6656a8 100644 --- a/intent/credential-free-live-calls/spec.md +++ b/intent/credential-free-live-calls/spec.md @@ -24,8 +24,9 @@ authorization" (the `Authorization` and `b2ContentDisposition` URL parameters); 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, capabilities, expiration from milliseconds) and cursor `"k-3"`; a second call with `"k-3"` + keys (name, ID, expiration from milliseconds) and cursor `"k-3"`; a second call with `"k-3"` sends `startApplicationKeyId` `"k-3"`. An empty `keys` list returns `io.EOF`. + 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. diff --git a/x/transport/transport_test.go b/x/transport/transport_test.go index e866cea6..5083131f 100644 --- a/x/transport/transport_test.go +++ b/x/transport/transport_test.go @@ -75,15 +75,20 @@ func TestStallWaitsForDurationOrContext(t *testing.T) { t.Run("context", func(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), delay) defer cancel() - client := &http.Client{Transport: WithFailures(nil, FailureRate(1), Stall(time.Second))} + 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) + } }) } From 0791767a99c4f2cca950e6319aef4e42bc0e2bd8 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Thu, 29 Oct 2026 11:37:09 -0600 Subject: [PATCH 19/26] test(internal): reproduce recording server review edge cases --- b2/update_cors_name_test.go | 10 +-- internal/b2wiretest/server_test.go | 104 +++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 5 deletions(-) diff --git a/b2/update_cors_name_test.go b/b2/update_cors_name_test.go index 35cf6c55..b6d042f0 100644 --- a/b2/update_cors_name_test.go +++ b/b2/update_cors_name_test.go @@ -44,11 +44,11 @@ func TestUpdateBucketSendsCORSRuleName(t *testing.T) { t.Fatalf("updateBucket: %v", err) } request := srv.Last(t, "b2_update_bucket") - rules, ok := request.JSON["corsRules"].([]any) - if !ok || len(rules) != 1 { - t.Fatalf("update request corsRules = %v, want one rule", request.JSON["corsRules"]) - } - b2wiretest.AssertJSON(t, request, "corsRules.0.corsRuleName", "browser") + b2wiretest.AssertJSON(t, request, "corsRules", []map[string]any{{ + "corsRuleName": "browser", + "allowedOrigins": []string{"https://example.com"}, + "allowedOperations": []string{"b2_download_file_by_name"}, + }}) } func TestReadModifyWriteKeepsCORSRuleName(t *testing.T) { diff --git a/internal/b2wiretest/server_test.go b/internal/b2wiretest/server_test.go index 62cdad1b..d2dd15a8 100644 --- a/internal/b2wiretest/server_test.go +++ b/internal/b2wiretest/server_test.go @@ -62,6 +62,42 @@ func TestNewClosesServerAtCleanup(t *testing.T) { } } +type cleanupTB struct { + testing.TB + cleanup func() + ended bool +} + +func (t *cleanupTB) Cleanup(f func()) { + t.cleanup = f + t.TB.Cleanup(f) +} + +func (t *cleanupTB) Errorf(format string, args ...any) { + if t.ended { + panic("Errorf called after test cleanup") + } + t.TB.Errorf(format, args...) +} + +func TestLateRequestAfterCleanupDoesNotReport(t *testing.T) { + fake := &cleanupTB{TB: t} + s := New(fake) + fake.cleanup() + fake.ended = true + defer func() { + if value := recover(); value != nil { + t.Fatalf("late request panicked: %v, want no test report after cleanup", value) + } + }() + response := httptest.NewRecorder() + // Invoke the saved handler directly: the closed listener already refuses HTTP requests. + s.http.Config.Handler.ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/b2api/v4/late_request", nil)) + if response.Code != http.StatusServiceUnavailable { + t.Errorf("late request status = %d, want 503", response.Code) + } +} + func TestRecordsMethodPathHeadersAndJSON(t *testing.T) { s := New(t) s.Reply("b2_create_key", http.StatusOK, `{}`) @@ -341,6 +377,36 @@ func TestReplySequenceRepeatsLast(t *testing.T) { } } +func TestScriptedZeroStatusDefaultsToOK(t *testing.T) { + for _, tc := range []struct { + name string + register func(*Server) + }{ + {"Reply", func(s *Server) { s.Reply("zero", 0, `{"scripted":true}`) }}, + {"ReplySequence", func(s *Server) { s.ReplySequence("zero", Response{Body: `{"scripted":true}`}) }}, + {"Handle", func(s *Server) { + s.Handle("zero", func(Request) Response { return Response{Body: `{"scripted":true}`} }) + }}, + } { + t.Run(tc.name, func(t *testing.T) { + fake := &recordingTB{TB: t} + s := New(fake) + tc.register(s) + response := get(t, s.URL()+"/b2api/v4/zero") + body, err := io.ReadAll(response.Body) + if err != nil { + t.Fatal(err) + } + if response.StatusCode != http.StatusOK || string(body) != `{"scripted":true}` { + t.Errorf("scripted response = %d %q, want 200 %q", response.StatusCode, body, `{"scripted":true}`) + } + if reports := fake.errors(); len(reports) != 0 { + t.Errorf("scripted response reports = %v, want none", reports) + } + }) + } +} + func TestHandleSeesTheRequest(t *testing.T) { s := New(t) s.Handle("b2_create_bucket", func(r Request) Response { @@ -408,6 +474,44 @@ func TestUnscriptedCallFails(t *testing.T) { } } +func TestUnscriptedCallDoesNotReflectHTML(t *testing.T) { + fake := &recordingTB{TB: t} + s := New(fake) + const attack = `` + response := get(t, s.URL()+"/b2api/v4/"+url.PathEscape(attack)) + body, err := io.ReadAll(response.Body) + if err != nil { + t.Fatal(err) + } + const want = `{"status":400,"code":"bad_request","message":"b2wiretest: no response scripted for this call"}` + if response.StatusCode != http.StatusBadRequest || string(body) != want { + t.Errorf("unscripted response = %d %q, want 400 %q", response.StatusCode, body, want) + } + if got := len(s.Calls(attack)); got != 1 { + t.Errorf("recorded attack requests = %d, want 1", got) + } +} + +func TestJSONAssertionsRejectEmptyPath(t *testing.T) { + r := Request{Call: "b2_update_bucket", JSON: map[string]any{"": "empty key"}} + for _, tc := range []struct { + name string + assert func(testing.TB) + }{ + {"AssertJSON", func(t testing.TB) { AssertJSON(t, r, "", "unused") }}, + {"AssertNoJSON", func(t testing.TB) { AssertNoJSON(t, r, "") }}, + } { + t.Run(tc.name, func(t *testing.T) { + fake := &recordingTB{TB: t} + tc.assert(fake) + reports := fake.errors() + if len(reports) != 1 || reports[0] != "b2_update_bucket body: empty JSON path" { + t.Errorf("empty-path reports = %v, want [b2_update_bucket body: empty JSON path]", reports) + } + }) + } +} + func TestAssertJSONReportsGotAndWant(t *testing.T) { r := Request{Call: "b2_update_bucket", JSON: map[string]any{"count": float64(5), "bucketIds": []any{"a"}}} fake := &recordingTB{TB: t} From 5fc1c1e3857f36aeab4e1c037490b16acdc54c28 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Thu, 29 Oct 2026 11:41:56 -0600 Subject: [PATCH 20/26] fix(internal): handle recorder defaults and late requests Treat scripted status zero as 200, reject empty JSON assertion paths consistently, and mark the server closed before draining HTTP handlers so late requests cannot report through a completed test. Document the path and header guarantees, the migrated test coverage, and the simulator contract NOT RUN rationale requested in PR #205. --- docs/testing.md | 4 ++++ intent/b2wiretest/spec.md | 26 ++++++++++++++++++-------- internal/b2wiretest/assert.go | 14 ++++++++++++-- internal/b2wiretest/server.go | 31 ++++++++++++++++++++++++++++--- 4 files changed, 62 insertions(+), 13 deletions(-) diff --git a/docs/testing.md b/docs/testing.md index 30f4ec63..9223636f 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -56,6 +56,10 @@ PRs that each add a row will conflict on those counts; the second to merge sets GitHub CI does not run the simulator contract, so a change can be green in CI and still fail a scenario. Run the scenarios for the area you changed before asking for review. +For the shared recording-server change (#205), the simulator contract is **NOT RUN**. The helper is +imported only by `_test.go` files; no SDK production code, harness scenarios or contract executables +change, so this test-infrastructure change cannot affect simulator contract behavior. + ## Live integration tests `b2/integration_test.go` skips unless `B2_APPLICATION_KEY_ID` and `B2_APPLICATION_KEY` are set. Use a diff --git a/intent/b2wiretest/spec.md b/intent/b2wiretest/spec.md index f33ecd50..521246f3 100644 --- a/intent/b2wiretest/spec.md +++ b/intent/b2wiretest/spec.md @@ -9,13 +9,15 @@ Skills applied: none beyond `AGENTS.md`, `REVIEW.md` and `docs/testing.md` (no p (an import cycle otherwise). - **R2. Lifecycle.** `New(t testing.TB) *Server` starts an `httptest.Server` and registers `t.Cleanup` to close it. `(*Server).URL() string` returns its base URL, to pass to `base.SetAPIBase` or `b2.APIBase`. + Cleanup marks the server closed before stopping and draining HTTP handlers. Late handler calls + return 503 without reporting through the completed test's `testing.TB`. - **R3. Authorization.** Without scripting, `b2_authorize_account` on any API version answers with a v4 response whose `apiUrl`, `downloadUrl` and `s3ApiUrl` are the server URL, authorization token `"test-token"`, no bucket or prefix restriction, and part sizes `absoluteMinimumPartSize` 5000000 and `recommendedPartSize` 100000000. A test can replace it like any other call (R5). - **R4. Recording.** Every request is recorded in arrival order as a `Request` with: `Seq` (1-based order), - `Method`, `Path`, `Call` (the last path segment for API calls, e.g. `b2_create_key`; `upload_file` and - `upload_part` for upload URLs), `Query` (`url.Values` decoded from `r.URL.RawQuery`), `Header` (a copy), + `Method`, `Path`, `Call` (the last path segment for every path, including non-API paths; a trailing + slash gives an empty call), `Query` (`url.Values` decoded from `r.URL.RawQuery`), `Header` (a copy), `Body` (raw bytes) and `JSON` (`map[string]any` decoded from the body when it is a JSON object, otherwise nil). Recording is safe for concurrent requests. - **R5. Scripted responses.** Per call name: @@ -23,11 +25,13 @@ Skills applied: none beyond `AGENTS.md`, `REVIEW.md` and `docs/testing.md` (no p - `ReplySequence(call string, rs ...Response)`: successive responses, the last one repeating; - `Handle(call string, h func(Request) Response)`: computed per request. `Response` has `Status int`, `Body string` and `Header http.Header`. A later registration for a call - replaces the earlier one. + replaces the earlier one. Status 0 means 200 for all three scripting methods; it is not an unscripted + call. Responses default to `Content-Type: application/json` unless a script sets it, and always set + `X-Content-Type-Options: nosniff` (overriding any scripted value for that header). - **R6. Unscripted calls fail loudly.** A request to a call with no response registered (other than the R3 and R7 built-ins) is recorded, answered with status 400 and a B2-shaped error body (`{"status":400,"code":"bad_request","message":"b2wiretest: no response scripted for this call"}`), and reported - with `t.Errorf` naming the call. + with `t.Errorf` naming the call. The response body is constant and never interpolates request values. - **R7. Upload URLs.** Without scripting, `b2_get_upload_url` and `b2_get_upload_part_url` return `uploadUrl` = server URL + `/upload_file` or `/upload_part`, and authorization tokens `"upload-token"` and `"upload-part-token"`. Requests to those paths are recorded as calls `upload_file` and `upload_part` @@ -45,12 +49,15 @@ Skills applied: none beyond `AGENTS.md`, `REVIEW.md` and `docs/testing.md` (no p - `AssertQuery(t, r Request, name, want string)`: reports ` query : got X, want Y` (or `missing` when absent). + Both JSON assertions report `empty JSON path` for an empty path. Dots always separate segments; + literal dots in object keys cannot be escaped or addressed. The helper never computes expected values. Tests supply them from the B2 API docs. - **R10. Tests of the helper.** `internal/b2wiretest` has its own tests for R2 to R9, using `net/http` as the client. Each test fails if the behavior it names is removed. - **R11. One existing test ported.** `TestUpdateBucketSendsCORSRuleName` (`b2/update_cors_name_test.go`) - is rewritten to use the helper, with the same assertions or stronger. This shows the API fits a real - `base` plus `b2` path. No other existing test changes. + is rewritten to use the helper, retaining its request-body assertions in `AssertJSON` form. R6's + recorded 400 response and test error replace the old `unexpected request path` check. This shows the + API fits a real `base` plus `b2` path. No other existing test changes. - **R12. Repository rules.** `go.mod` and `go.sum` unchanged; builds with Go 1.26.10; `make ci` passes; no exported symbol is added to `base`, `b2` or `x/...`. @@ -59,16 +66,19 @@ Skills applied: none beyond `AGENTS.md`, `REVIEW.md` and `docs/testing.md` (no p ``` internal/b2wiretest/ ├── server.go # Server, New, URL, Reply, ReplySequence, Handle, Requests, Calls, Last -├── assert.go # AssertJSON, AssertNoJSON, AssertHeader +├── assert.go # AssertJSON, AssertNoJSON, AssertHeader, AssertQuery └── server_test.go # R10 ``` - The server routes on the path. `/b2api/v/` gives `Call = ` for any ``, so the helper - works across the API versions blazer uses (v3 and v4 today) without listing them. + works across the API versions blazer uses (v3 and v4 today) without listing them. The same last-segment + rule applies to all other paths; the helper does not restrict path shapes. - One mutex guards the record slice and the response table. Handlers run outside the lock, so a `Handle` function may call `Requests()`. - The record holds copies (headers cloned, body read fully), so a test can inspect it after the client has closed the request. +- Scripted status 0 defaults to 200. Response headers retain a scripted `Content-Type`, default to + `application/json` when absent, and force `X-Content-Type-Options: nosniff`. - `AssertJSON` walks maps by key and slices by decimal index. A missing step reports ` body : missing`. - There is no global state. Each test gets its own server, so tests stay safe under `-shuffle=on` and diff --git a/internal/b2wiretest/assert.go b/internal/b2wiretest/assert.go index b9970397..55bf45fe 100644 --- a/internal/b2wiretest/assert.go +++ b/internal/b2wiretest/assert.go @@ -8,9 +8,14 @@ import ( "testing" ) -// AssertJSON checks a dot-separated path in a request's JSON object body. +// AssertJSON checks a non-empty, dot-separated path in a request's JSON object body. +// Dots always separate path segments; keys containing literal dots cannot be addressed. func AssertJSON(t testing.TB, r Request, path string, want any) { t.Helper() + if path == "" { + t.Errorf("%s body: empty JSON path", r.Call) + return + } wantJSON, err := json.Marshal(want) if err != nil { t.Fatalf("%s body %s: encode want: %v", r.Call, path, err) @@ -32,9 +37,14 @@ func AssertJSON(t testing.TB, r Request, path string, want any) { } } -// AssertNoJSON checks that a dot-separated path is absent from a request body. +// AssertNoJSON checks that a non-empty, dot-separated path is absent from a request body. +// It has the same path limitations as AssertJSON. func AssertNoJSON(t testing.TB, r Request, path string) { t.Helper() + if path == "" { + t.Errorf("%s body: empty JSON path", r.Call) + return + } if got, ok := jsonPath(r.JSON, path); ok { gotJSON, _ := json.Marshal(got) t.Errorf("%s body %s: got %s, want missing", r.Call, path, gotJSON) diff --git a/internal/b2wiretest/server.go b/internal/b2wiretest/server.go index 38d05ca8..4b970d74 100644 --- a/internal/b2wiretest/server.go +++ b/internal/b2wiretest/server.go @@ -18,6 +18,7 @@ type Request struct { Seq int Method string Path string + // Call is the last segment of Path, including for non-API paths; a trailing slash gives "". Call string Query url.Values Header http.Header @@ -26,6 +27,8 @@ type Request struct { } // Response is the status, headers, and body returned for a scripted call. +// Status 0 means 200. Content-Type defaults to application/json unless Header sets it; +// X-Content-Type-Options is always nosniff. type Response struct { Status int Body string @@ -45,28 +48,38 @@ type Server struct { t testing.TB http *httptest.Server mu sync.Mutex + closed bool requests []Request scripts map[string]*script } // New starts a recording server and closes it when t finishes. +// Cleanup rejects new requests and waits for active HTTP handlers before returning. func New(t testing.TB) *Server { t.Helper() s := &Server{t: t, scripts: make(map[string]*script)} s.http = httptest.NewServer(http.HandlerFunc(s.serve)) - t.Cleanup(s.http.Close) + t.Cleanup(s.close) return s } +func (s *Server) close() { + s.mu.Lock() + s.closed = true + s.mu.Unlock() + s.http.Close() +} + // URL returns the server's base URL. func (s *Server) URL() string { return s.http.URL } -// Reply registers a response repeated for every request to call. +// Reply registers a response repeated for every request to call. Status 0 means 200. func (s *Server) Reply(call string, status int, body string) { s.ReplySequence(call, Response{Status: status, Body: body}) } // ReplySequence registers successive responses; the last response repeats. +// See Response for status and header defaults. func (s *Server) ReplySequence(call string, rs ...Response) { s.mu.Lock() defer s.mu.Unlock() @@ -74,6 +87,7 @@ func (s *Server) ReplySequence(call string, rs ...Response) { } // Handle registers a response computed from each request. It replaces any earlier script. +// See Response for status and header defaults. func (s *Server) Handle(call string, h func(Request) Response) { s.mu.Lock() defer s.mu.Unlock() @@ -132,6 +146,14 @@ func copyRequests(requests []Request) []Request { } func (s *Server) serve(w http.ResponseWriter, r *http.Request) { + s.mu.Lock() + if s.closed { + s.mu.Unlock() + http.Error(w, "b2wiretest: server closed", http.StatusServiceUnavailable) + return + } + s.mu.Unlock() + body, err := io.ReadAll(r.Body) if err != nil { s.t.Errorf("b2wiretest: read %s body: %v", r.URL.Path, err) @@ -168,10 +190,13 @@ func (s *Server) serve(w http.ResponseWriter, r *http.Request) { } else if !ok { response, ok = s.builtin(call) } - if !ok || response.Status == 0 { + if !ok { s.t.Errorf("b2wiretest: no response for %s", call) response = Response{Status: http.StatusBadRequest, Body: unscriptedResponseBody} } + if response.Status == 0 { + response.Status = http.StatusOK + } for name, values := range response.Header { for _, value := range values { w.Header().Add(name, value) From 6c6e654f9d54410736365a3290c0bf1d2561afff Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Thu, 29 Oct 2026 11:49:27 -0600 Subject: [PATCH 21/26] refactor(internal): share the recorded JSON body decode serve and copyRequests both decoded a request body into a JSON map. Share one decodeJSON helper so the two paths cannot drift; a body that is not a JSON object still decodes to nil. The same change was reviewed on #211. --- internal/b2wiretest/server.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/internal/b2wiretest/server.go b/internal/b2wiretest/server.go index 4b970d74..7b320f20 100644 --- a/internal/b2wiretest/server.go +++ b/internal/b2wiretest/server.go @@ -138,8 +138,7 @@ func copyRequests(requests []Request) []Request { result[i].Header = r.Header.Clone() result[i].Body = append([]byte(nil), r.Body...) if r.JSON != nil { - result[i].JSON = nil - _ = json.Unmarshal(result[i].Body, &result[i].JSON) + result[i].JSON = decodeJSON(result[i].Body) } } return result @@ -163,7 +162,7 @@ func (s *Server) serve(w http.ResponseWriter, r *http.Request) { call := r.URL.Path[strings.LastIndexByte(r.URL.Path, '/')+1:] req := Request{Method: r.Method, Path: r.URL.Path, Call: call, Query: r.URL.Query(), Header: r.Header.Clone(), Body: body} if len(body) > 0 { - _ = json.Unmarshal(body, &req.JSON) + req.JSON = decodeJSON(body) } s.mu.Lock() @@ -226,3 +225,10 @@ func (s *Server) builtin(call string) (Response, bool) { return Response{}, false } } + +// decodeJSON returns body decoded as a JSON object, or nil when it is not one. +func decodeJSON(body []byte) map[string]any { + var decoded map[string]any + _ = json.Unmarshal(body, &decoded) + return decoded +} From a3268c2bc640a2b772df5ce595c924bb6a1e81e6 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Thu, 29 Oct 2026 16:55:05 -0600 Subject: [PATCH 22/26] test: contrast newest and older versions in Reveal and pin AfterNBytes on short bodies Script newest-first and older versions for Reveal, and ensure AfterNBytes does not trigger when a request body ends before its threshold. Part of #203 --- b2/live_only_calls_wire_test.go | 15 ++++++++++----- x/transport/transport_test.go | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/b2/live_only_calls_wire_test.go b/b2/live_only_calls_wire_test.go index 62caad27..df4b7ad2 100644 --- a/b2/live_only_calls_wire_test.go +++ b/b2/live_only_calls_wire_test.go @@ -41,7 +41,8 @@ func TestObjectHideWire(t *testing.T) { func TestBucketRevealWire(t *testing.T) { t.Run("deletes newest hide marker", func(t *testing.T) { srv, bucket := liveOnlyBucket(t) - srv.Reply("b2_list_file_versions", http.StatusOK, `{"files":[{"fileId":"hide-id","fileName":"gone","action":"hide","uploadTimestamp":0}],"nextFileName":"","nextFileId":""}`) + // 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) @@ -52,7 +53,8 @@ func TestBucketRevealWire(t *testing.T) { }) t.Run("leaves a current upload alone", func(t *testing.T) { srv, bucket := liveOnlyBucket(t) - srv.Reply("b2_list_file_versions", http.StatusOK, `{"files":[{"fileId":"upload-id","fileName":"gone","action":"upload","uploadTimestamp":0}],"nextFileName":"","nextFileId":""}`) + // 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":""}`) if err := bucket.Reveal(context.Background(), "gone"); err != nil { t.Fatal(err) } @@ -84,8 +86,11 @@ func TestBucketAuthTokenWire(t *testing.T) { func TestObjectAuthURLWire(t *testing.T) { srv, bucket := liveOnlyBucket(t) srv.Reply("b2_get_download_authorization", http.StatusOK, `{"authorizationToken":"download-token"}`) - object := bucket.Object("folder/a b.txt") - const disposition = `attachment; filename="a b.txt"` + 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) @@ -97,7 +102,7 @@ func TestObjectAuthURLWire(t *testing.T) { t.Errorf("b2ContentDisposition = %q, want %q", got, disposition) } request := srv.Last(t, "b2_get_download_authorization") - b2wiretest.AssertJSON(t, request, "fileNamePrefix", object.Name()) + b2wiretest.AssertJSON(t, request, "fileNamePrefix", objectName) b2wiretest.AssertJSON(t, request, "b2ContentDisposition", disposition) u, err = object.AuthURL(context.Background(), time.Hour, "") if err != nil { diff --git a/x/transport/transport_test.go b/x/transport/transport_test.go index 5083131f..28b24764 100644 --- a/x/transport/transport_test.go +++ b/x/transport/transport_test.go @@ -117,6 +117,21 @@ func TestAfterNBytesTriggersOnce(t *testing.T) { } } +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) + } +} + func transportTestServer(t *testing.T) *httptest.Server { t.Helper() srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { From fe2843e39667066eebd34cd3719c04f09c4c19c0 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Fri, 30 Oct 2026 10:51:41 -0600 Subject: [PATCH 23/26] test: tighten live-only wire and transport assertions Pin the scripted service error and refusal paths, use the documented null expiry fixture without asserting the existing epoch mapping, and cover the AfterNBytes callback across uploads and response bodies. Record isolated red-to-green proofs for upload refusal, cancellation, deadlines, certificate verification and transport thresholds. Document the Trigger race under #176 and the deferred null-expiry mapping. Part of #203 --- b2/live_only_calls_wire_test.go | 27 +- base/live_only_calls_test.go | 133 +++++---- intent/credential-free-live-calls/intent.md | 2 +- intent/credential-free-live-calls/plan.md | 297 +++++++++++++++++++- intent/credential-free-live-calls/spec.md | 19 +- x/transport/transport_test.go | 23 ++ 6 files changed, 428 insertions(+), 73 deletions(-) diff --git a/b2/live_only_calls_wire_test.go b/b2/live_only_calls_wire_test.go index df4b7ad2..796f412a 100644 --- a/b2/live_only_calls_wire_test.go +++ b/b2/live_only_calls_wire_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/Backblaze/blazer/base" "github.com/Backblaze/blazer/internal/b2wiretest" ) @@ -32,8 +33,12 @@ func TestObjectHideWire(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"}`) - if err := bucket.Object("gone").Hide(context.Background()); err == nil { - t.Fatal("Hide succeeded after a scripted 400") + 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)) } }) } @@ -55,9 +60,15 @@ func TestBucketRevealWire(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)) } @@ -121,7 +132,7 @@ func TestClientListKeysAndDeleteWire(t *testing.T) { srv := b2wiretest.New(t) client := liveOnlyClient(t, srv) 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":0}],"nextApplicationKeyId":"k-3"}`}, + 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, `{}`) @@ -136,8 +147,8 @@ func TestClientListKeysAndDeleteWire(t *testing.T) { 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" || keys[1].Expires().UnixMilli() != 0 { - t.Errorf("second key = id=%q name=%q expires=%v", keys[1].ID(), keys[1].Name(), keys[1].Expires()) + 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") @@ -174,12 +185,14 @@ func TestObjectCancelWire(t *testing.T) { 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()) } - if err := iter.Object().Cancel(context.Background()); err == nil { - t.Fatal("Cancel succeeded for an uploaded object") + 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)) diff --git a/base/live_only_calls_test.go b/base/live_only_calls_test.go index 4b43c614..b7a2eba4 100644 --- a/base/live_only_calls_test.go +++ b/base/live_only_calls_test.go @@ -121,70 +121,83 @@ func TestUploadHangRequestsNewUploadURL(t *testing.T) { } func TestHTTPRequestCancellationAndDeadline(t *testing.T) { - entered := make(chan struct{}) - observedCancel := 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) - <-r.Context().Done() - close(observedCancel) - 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 + "/wait", token: "upload-token", b2: root} - cancelled, cancel := context.WithCancel(context.Background()) - cancel() - if _, err := upload.UploadFile(cancelled, strings.NewReader("x"), 1, "name", "text/plain", "sha1", nil); !errors.Is(err, context.Canceled) { - t.Errorf("already-cancelled request error = %v, want context.Canceled", err) - } else if got := Action(err); got != Punt { - t.Errorf("Action(%v) = %v, want Punt", err, got) - } - ctx, stop := context.WithTimeout(context.Background(), time.Second) - defer stop() - 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(), 3*time.Second) - defer cancelWait() - select { - case <-entered: - case <-wait.Done(): - t.Fatalf("deadline request never reached handler: %v", wait.Err()) - } - select { - case err := <-done: - if !errors.Is(err, context.DeadlineExceeded) { - t.Errorf("blocked request error = %v, want context.DeadlineExceeded", err) - } else if got := Action(err); got != Punt { - t.Errorf("Action(%v) = %v, want Punt", err, got) - } - case <-wait.Done(): - t.Fatalf("deadline request did not return: %v", wait.Err()) - } - select { - case <-observedCancel: - case <-wait.Done(): - t.Fatal("handler did not observe request deadline") + 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) { - srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - _, _ = fmt.Fprint(w, `{}`) + 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)) diff --git a/intent/credential-free-live-calls/intent.md b/intent/credential-free-live-calls/intent.md index f00267bf..24abea5b 100644 --- a/intent/credential-free-live-calls/intent.md +++ b/intent/credential-free-live-calls/intent.md @@ -1,5 +1,5 @@ # Intent: credential-free tests for calls only covered by live tests -Author: Felipe Fumero. Issue: #203. Parent: #97. Status: draft. +Author: Felipe Fumero. Issue: #203. Parent: #97. Status: ready for review. ## Problem diff --git a/intent/credential-free-live-calls/plan.md b/intent/credential-free-live-calls/plan.md index 51456571..d5258601 100644 --- a/intent/credential-free-live-calls/plan.md +++ b/intent/credential-free-live-calls/plan.md @@ -1,8 +1,9 @@ # 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. Apply the lean-code ladder from -`AGENTS.md`: reuse `internal/b2wiretest`, the `httptest` patterns in `base/*_test.go`, and existing fixtures. +#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 @@ -45,3 +46,295 @@ No production file changes (R13). 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. + +
+Complete failing and restored output for each break + +### R2 (upload) + +```text +--- FAIL: TestBucketRevealWire (0.00s) + --- FAIL: TestBucketRevealWire/leaves_a_current_upload_alone (0.00s) + live_only_calls_wire_test.go:73: b2_delete_file_version calls = 1, want 0 +FAIL +FAIL github.com/Backblaze/blazer/b2 0.267s +FAIL +``` + +After revert: + +```text +ok github.com/Backblaze/blazer/b2 0.737s +``` + +### R7 (upload) + +```text +--- FAIL: TestObjectCancelWire (0.00s) + --- FAIL: TestObjectCancelWire/uploaded_file (0.00s) + live_only_calls_wire_test.go:195: Cancel uploaded file error = , want "complete is not an unfinished large file: upload" + live_only_calls_wire_test.go:198: b2_cancel_large_file calls = 1, want 0 +FAIL +FAIL github.com/Backblaze/blazer/b2 0.824s +FAIL +``` + +After revert: + +```text +ok github.com/Backblaze/blazer/b2 0.317s +``` + +### R9 (cancelled) + +```text +--- FAIL: TestHTTPRequestCancellationAndDeadline (5.00s) + --- FAIL: TestHTTPRequestCancellationAndDeadline/cancelled (5.00s) + live_only_calls_test.go:184: cancelled request did not return: context deadline exceeded +FAIL +FAIL github.com/Backblaze/blazer/base 5.694s +FAIL +``` + +After revert: + +```text +ok github.com/Backblaze/blazer/base 0.279s +``` + +### R9 (deadline) + +```text +--- FAIL: TestHTTPRequestCancellationAndDeadline (5.00s) + --- FAIL: TestHTTPRequestCancellationAndDeadline/deadline (5.00s) + live_only_calls_test.go:184: deadline request did not return: context deadline exceeded +FAIL +FAIL github.com/Backblaze/blazer/base 5.287s +FAIL +``` + +After revert: + +```text +ok github.com/Backblaze/blazer/base 1.292s +``` + +### R10 + +```text +--- FAIL: TestUnknownCertificateAuthority (0.01s) + live_only_calls_test.go:205: authorization accepted an unknown certificate authority +FAIL +FAIL github.com/Backblaze/blazer/base 0.686s +FAIL +``` + +After revert: + +```text +ok github.com/Backblaze/blazer/base 0.315s +``` + +### R11 (one-shot) + +```text +--- FAIL: TestAfterNBytesTriggersOnce (0.00s) + transport_test.go:129: effect calls after second upload and both responses = 2, want 1 +FAIL +FAIL github.com/Backblaze/blazer/x/transport 0.664s +FAIL +``` + +After revert: + +```text +ok github.com/Backblaze/blazer/x/transport 0.267s +``` + +### R11 (response bytes) + +```text +--- FAIL: TestAfterNBytesDoesNotTriggerForShortBody (0.00s) + transport_test.go:154: effect calls after short body and response = 0, want 1 +FAIL +FAIL github.com/Backblaze/blazer/x/transport 0.462s +FAIL +``` + +After revert: + +```text +ok github.com/Backblaze/blazer/x/transport 0.250s +``` + +
+ +## Trigger race evidence (#176) + +A temporary diagnostic used the existing `transportTestServer` and `transportRequest` helpers. It +created a client with `WithFailures(nil, Trigger(ctx), Response(http.StatusServiceUnavailable))`, +cancelled `ctx`, then sent and drained 100 requests on that client. It used no sleeps. Run: + +```sh +env -u B2_APPLICATION_KEY_ID -u B2_APPLICATION_KEY -u B2_ACCOUNT_ID -u B2_ACCOUNT_KEY \ + go test -race -count=20 -run '^TestTriggerAfterCancellationRaceRepro$' ./x/transport +``` + +Actual output (the diagnostic test was removed after recording this, per R11): + +```text +================== +WARNING: DATA RACE +Write at 0x00c0001902b8 by goroutine 10: + github.com/Backblaze/blazer/x/transport.Trigger.func1.1() + /private/tmp/blazer-203/x/transport/transport.go:159 +0x58 + +Previous read at 0x00c0001902b8 by goroutine 8: + github.com/Backblaze/blazer/x/transport.(*options).RoundTrip() + /private/tmp/blazer-203/x/transport/transport.go:70 +0x44 + net/http.send() + /opt/homebrew/Cellar/go/1.27.1/libexec/src/net/http/client.go:266 +0x618 + net/http.(*Client).send() + /opt/homebrew/Cellar/go/1.27.1/libexec/src/net/http/client.go:187 +0x2d8 + net/http.(*Client).do() + /opt/homebrew/Cellar/go/1.27.1/libexec/src/net/http/client.go:745 +0xd8c + net/http.(*Client).Do() + /opt/homebrew/Cellar/go/1.27.1/libexec/src/net/http/client.go:604 +0xdc + github.com/Backblaze/blazer/x/transport.transportRequest() + /private/tmp/blazer-203/x/transport/transport_test.go:174 +0xd0 + github.com/Backblaze/blazer/x/transport.TestTriggerAfterCancellationRaceRepro() + /private/tmp/blazer-203/x/transport/trigger_race_repro_test.go:18 +0x208 + testing.tRunner() + /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2193 +0x164 + testing.(*T).Run.gowrap1() + /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2258 +0x34 + +Goroutine 10 (running) created at: + github.com/Backblaze/blazer/x/transport.Trigger.func1() + /private/tmp/blazer-203/x/transport/transport.go:157 +0xc8 + github.com/Backblaze/blazer/x/transport.WithFailures() + /private/tmp/blazer-203/x/transport/transport.go:42 +0x118 + github.com/Backblaze/blazer/x/transport.TestTriggerAfterCancellationRaceRepro() + /private/tmp/blazer-203/x/transport/trigger_race_repro_test.go:15 +0xf4 + testing.tRunner() + /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2193 +0x164 + testing.(*T).Run.gowrap1() + /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2258 +0x34 + +Goroutine 8 (running) created at: + testing.(*T).Run() + /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2258 +0x7bc + testing.runTests.func1() + /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2742 +0x74 + testing.tRunner() + /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2193 +0x164 + testing.runTests() + /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2740 +0x7a0 + testing.(*M).Run() + /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2600 +0xb38 + main.main() + _testmain.go:58 +0x100 +================== +--- FAIL: TestTriggerAfterCancellationRaceRepro (0.00s) + testing.go:1865: race detected during execution of test +FAIL +FAIL github.com/Backblaze/blazer/x/transport 0.651s +FAIL +``` + +## Review follow-up verification + +All commands below ran with `B2_APPLICATION_KEY_ID`, `B2_APPLICATION_KEY`, `B2_ACCOUNT_ID`, and +`B2_ACCOUNT_KEY` unset. Live B2 and simulator scenarios were NOT RUN; this follow-up only changes tests +and their intent records, with no SDK or harness behavior changes. + +```text +$ go test -race -count=5 -shuffle=on ./b2/... ./base/... ./x/transport/... +ok github.com/Backblaze/blazer/b2 63.665s +ok github.com/Backblaze/blazer/b2/sync 15.302s +ok github.com/Backblaze/blazer/base 33.271s +ok github.com/Backblaze/blazer/x/transport 2.063s + +$ go vet ./... +``` + +`go vet` exited 0 with no output. `make ci` exited 0: + +```text +go build ./... +go vet ./... +go mod tidy -diff +go mod verify +all modules verified +go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.14.0 run --new-from-rev=origin/main ./... +0 issues. +go test -race -shuffle=on ./... +ok github.com/Backblaze/blazer/b2 14.251s +ok github.com/Backblaze/blazer/b2/sync 4.510s +ok github.com/Backblaze/blazer/base 7.643s +? github.com/Backblaze/blazer/examples/simple [no test files] +? github.com/Backblaze/blazer/internal/b2assets [no test files] +ok github.com/Backblaze/blazer/internal/b2types 2.112s +ok github.com/Backblaze/blazer/internal/b2wiretest 2.667s +? github.com/Backblaze/blazer/internal/bin/cleanup [no test files] +ok github.com/Backblaze/blazer/internal/blog 2.310s +? github.com/Backblaze/blazer/internal/retry [no test files] +ok github.com/Backblaze/blazer/x/b2sim 2.981s +ok github.com/Backblaze/blazer/x/consistent 2.946s +ok github.com/Backblaze/blazer/x/transport 3.375s +ok github.com/Backblaze/blazer/x/window 3.108s +cd bin/b2keys && go build ./... && go test ./... +? github.com/Backblaze/blazer/bin/b2keys [no test files] +CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build ./... +CGO_ENABLED=0 GOOS=linux GOARCH=386 go build ./... +CGO_ENABLED=0 GOOS=linux GOARCH=arm go build ./... +CGO_ENABLED=0 GOOS=darwin GOARCH=arm64 go build ./... +CGO_ENABLED=0 GOOS=windows GOARCH=amd64 go build ./... +for t in scripts/*_test.sh; do "$t" || exit 1; done +ok comments on the workflow's own issue +ok ignores an issue with the same title opened by someone else +ok ignores a longer title that contains the phrase +ok picks the oldest matching issue +ok opens an issue when none exists +``` + +The local changelog check passed. No changelog fragment was added. + +Fresh-context code/security review of this follow-up found no actionable issues. diff --git a/intent/credential-free-live-calls/spec.md b/intent/credential-free-live-calls/spec.md index dd6656a8..1d5252ab 100644 --- a/intent/credential-free-live-calls/spec.md +++ b/intent/credential-free-live-calls/spec.md @@ -24,9 +24,10 @@ authorization" (the `Authorization` and `b2ContentDisposition` URL parameters); 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, expiration from milliseconds) and cursor `"k-3"`; a second call with `"k-3"` + 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`. - Capabilities mapping is tracked separately in #220. + A key with no expiry uses `expirationTimestamp: null`; its name and ID are asserted, while its expiry + mapping is deferred (see Areas of concern). 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. @@ -55,7 +56,9 @@ authorization" (the `Authorization` and `b2ContentDisposition` URL parameters); 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 after roughly n body bytes; `Trigger(ctx)` fails every request after `ctx` is + 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 @@ -85,3 +88,13 @@ authorization" (the `Authorization` and `b2ContentDisposition` URL parameters); 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 needs a separate issue.** The documented `expirationTimestamp: null` currently + produces `1970-01-01T00:00:00Z` (`IsZero=false`) through `Key.Expires()`. Do not pin that value as correct. + The fixture uses null and asserts the key's name and ID; propose a separate production fix to preserve + the absence of an expiry. Owner: @ffumero2003. diff --git a/x/transport/transport_test.go b/x/transport/transport_test.go index 28b24764..a3f06269 100644 --- a/x/transport/transport_test.go +++ b/x/transport/transport_test.go @@ -115,6 +115,19 @@ func TestAfterNBytesTriggersOnce(t *testing.T) { 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) { @@ -130,6 +143,16 @@ func TestAfterNBytesDoesNotTriggerForShortBody(t *testing.T) { 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 { From 11be84df321b8b1d5027aaf98ac670af4b1b0ee8 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Fri, 30 Oct 2026 11:05:49 -0600 Subject: [PATCH 24/26] docs(intent): trim the #203 plan to its proof tables Move pasted test, race and make ci output out of the committed plan; keep the requirement-to-break tables and a short pointer to the Trigger race in #176. Part of #203 --- intent/credential-free-live-calls/plan.md | 258 +--------------------- 1 file changed, 5 insertions(+), 253 deletions(-) diff --git a/intent/credential-free-live-calls/plan.md b/intent/credential-free-live-calls/plan.md index d5258601..b7aa0d8f 100644 --- a/intent/credential-free-live-calls/plan.md +++ b/intent/credential-free-live-calls/plan.md @@ -84,257 +84,9 @@ 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. -
-Complete failing and restored output for each break +## Trigger race (#176) -### R2 (upload) - -```text ---- FAIL: TestBucketRevealWire (0.00s) - --- FAIL: TestBucketRevealWire/leaves_a_current_upload_alone (0.00s) - live_only_calls_wire_test.go:73: b2_delete_file_version calls = 1, want 0 -FAIL -FAIL github.com/Backblaze/blazer/b2 0.267s -FAIL -``` - -After revert: - -```text -ok github.com/Backblaze/blazer/b2 0.737s -``` - -### R7 (upload) - -```text ---- FAIL: TestObjectCancelWire (0.00s) - --- FAIL: TestObjectCancelWire/uploaded_file (0.00s) - live_only_calls_wire_test.go:195: Cancel uploaded file error = , want "complete is not an unfinished large file: upload" - live_only_calls_wire_test.go:198: b2_cancel_large_file calls = 1, want 0 -FAIL -FAIL github.com/Backblaze/blazer/b2 0.824s -FAIL -``` - -After revert: - -```text -ok github.com/Backblaze/blazer/b2 0.317s -``` - -### R9 (cancelled) - -```text ---- FAIL: TestHTTPRequestCancellationAndDeadline (5.00s) - --- FAIL: TestHTTPRequestCancellationAndDeadline/cancelled (5.00s) - live_only_calls_test.go:184: cancelled request did not return: context deadline exceeded -FAIL -FAIL github.com/Backblaze/blazer/base 5.694s -FAIL -``` - -After revert: - -```text -ok github.com/Backblaze/blazer/base 0.279s -``` - -### R9 (deadline) - -```text ---- FAIL: TestHTTPRequestCancellationAndDeadline (5.00s) - --- FAIL: TestHTTPRequestCancellationAndDeadline/deadline (5.00s) - live_only_calls_test.go:184: deadline request did not return: context deadline exceeded -FAIL -FAIL github.com/Backblaze/blazer/base 5.287s -FAIL -``` - -After revert: - -```text -ok github.com/Backblaze/blazer/base 1.292s -``` - -### R10 - -```text ---- FAIL: TestUnknownCertificateAuthority (0.01s) - live_only_calls_test.go:205: authorization accepted an unknown certificate authority -FAIL -FAIL github.com/Backblaze/blazer/base 0.686s -FAIL -``` - -After revert: - -```text -ok github.com/Backblaze/blazer/base 0.315s -``` - -### R11 (one-shot) - -```text ---- FAIL: TestAfterNBytesTriggersOnce (0.00s) - transport_test.go:129: effect calls after second upload and both responses = 2, want 1 -FAIL -FAIL github.com/Backblaze/blazer/x/transport 0.664s -FAIL -``` - -After revert: - -```text -ok github.com/Backblaze/blazer/x/transport 0.267s -``` - -### R11 (response bytes) - -```text ---- FAIL: TestAfterNBytesDoesNotTriggerForShortBody (0.00s) - transport_test.go:154: effect calls after short body and response = 0, want 1 -FAIL -FAIL github.com/Backblaze/blazer/x/transport 0.462s -FAIL -``` - -After revert: - -```text -ok github.com/Backblaze/blazer/x/transport 0.250s -``` - -
- -## Trigger race evidence (#176) - -A temporary diagnostic used the existing `transportTestServer` and `transportRequest` helpers. It -created a client with `WithFailures(nil, Trigger(ctx), Response(http.StatusServiceUnavailable))`, -cancelled `ctx`, then sent and drained 100 requests on that client. It used no sleeps. Run: - -```sh -env -u B2_APPLICATION_KEY_ID -u B2_APPLICATION_KEY -u B2_ACCOUNT_ID -u B2_ACCOUNT_KEY \ - go test -race -count=20 -run '^TestTriggerAfterCancellationRaceRepro$' ./x/transport -``` - -Actual output (the diagnostic test was removed after recording this, per R11): - -```text -================== -WARNING: DATA RACE -Write at 0x00c0001902b8 by goroutine 10: - github.com/Backblaze/blazer/x/transport.Trigger.func1.1() - /private/tmp/blazer-203/x/transport/transport.go:159 +0x58 - -Previous read at 0x00c0001902b8 by goroutine 8: - github.com/Backblaze/blazer/x/transport.(*options).RoundTrip() - /private/tmp/blazer-203/x/transport/transport.go:70 +0x44 - net/http.send() - /opt/homebrew/Cellar/go/1.27.1/libexec/src/net/http/client.go:266 +0x618 - net/http.(*Client).send() - /opt/homebrew/Cellar/go/1.27.1/libexec/src/net/http/client.go:187 +0x2d8 - net/http.(*Client).do() - /opt/homebrew/Cellar/go/1.27.1/libexec/src/net/http/client.go:745 +0xd8c - net/http.(*Client).Do() - /opt/homebrew/Cellar/go/1.27.1/libexec/src/net/http/client.go:604 +0xdc - github.com/Backblaze/blazer/x/transport.transportRequest() - /private/tmp/blazer-203/x/transport/transport_test.go:174 +0xd0 - github.com/Backblaze/blazer/x/transport.TestTriggerAfterCancellationRaceRepro() - /private/tmp/blazer-203/x/transport/trigger_race_repro_test.go:18 +0x208 - testing.tRunner() - /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2193 +0x164 - testing.(*T).Run.gowrap1() - /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2258 +0x34 - -Goroutine 10 (running) created at: - github.com/Backblaze/blazer/x/transport.Trigger.func1() - /private/tmp/blazer-203/x/transport/transport.go:157 +0xc8 - github.com/Backblaze/blazer/x/transport.WithFailures() - /private/tmp/blazer-203/x/transport/transport.go:42 +0x118 - github.com/Backblaze/blazer/x/transport.TestTriggerAfterCancellationRaceRepro() - /private/tmp/blazer-203/x/transport/trigger_race_repro_test.go:15 +0xf4 - testing.tRunner() - /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2193 +0x164 - testing.(*T).Run.gowrap1() - /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2258 +0x34 - -Goroutine 8 (running) created at: - testing.(*T).Run() - /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2258 +0x7bc - testing.runTests.func1() - /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2742 +0x74 - testing.tRunner() - /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2193 +0x164 - testing.runTests() - /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2740 +0x7a0 - testing.(*M).Run() - /opt/homebrew/Cellar/go/1.27.1/libexec/src/testing/testing.go:2600 +0xb38 - main.main() - _testmain.go:58 +0x100 -================== ---- FAIL: TestTriggerAfterCancellationRaceRepro (0.00s) - testing.go:1865: race detected during execution of test -FAIL -FAIL github.com/Backblaze/blazer/x/transport 0.651s -FAIL -``` - -## Review follow-up verification - -All commands below ran with `B2_APPLICATION_KEY_ID`, `B2_APPLICATION_KEY`, `B2_ACCOUNT_ID`, and -`B2_ACCOUNT_KEY` unset. Live B2 and simulator scenarios were NOT RUN; this follow-up only changes tests -and their intent records, with no SDK or harness behavior changes. - -```text -$ go test -race -count=5 -shuffle=on ./b2/... ./base/... ./x/transport/... -ok github.com/Backblaze/blazer/b2 63.665s -ok github.com/Backblaze/blazer/b2/sync 15.302s -ok github.com/Backblaze/blazer/base 33.271s -ok github.com/Backblaze/blazer/x/transport 2.063s - -$ go vet ./... -``` - -`go vet` exited 0 with no output. `make ci` exited 0: - -```text -go build ./... -go vet ./... -go mod tidy -diff -go mod verify -all modules verified -go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.14.0 run --new-from-rev=origin/main ./... -0 issues. -go test -race -shuffle=on ./... -ok github.com/Backblaze/blazer/b2 14.251s -ok github.com/Backblaze/blazer/b2/sync 4.510s -ok github.com/Backblaze/blazer/base 7.643s -? github.com/Backblaze/blazer/examples/simple [no test files] -? github.com/Backblaze/blazer/internal/b2assets [no test files] -ok github.com/Backblaze/blazer/internal/b2types 2.112s -ok github.com/Backblaze/blazer/internal/b2wiretest 2.667s -? github.com/Backblaze/blazer/internal/bin/cleanup [no test files] -ok github.com/Backblaze/blazer/internal/blog 2.310s -? github.com/Backblaze/blazer/internal/retry [no test files] -ok github.com/Backblaze/blazer/x/b2sim 2.981s -ok github.com/Backblaze/blazer/x/consistent 2.946s -ok github.com/Backblaze/blazer/x/transport 3.375s -ok github.com/Backblaze/blazer/x/window 3.108s -cd bin/b2keys && go build ./... && go test ./... -? github.com/Backblaze/blazer/bin/b2keys [no test files] -CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build ./... -CGO_ENABLED=0 GOOS=linux GOARCH=386 go build ./... -CGO_ENABLED=0 GOOS=linux GOARCH=arm go build ./... -CGO_ENABLED=0 GOOS=darwin GOARCH=arm64 go build ./... -CGO_ENABLED=0 GOOS=windows GOARCH=amd64 go build ./... -for t in scripts/*_test.sh; do "$t" || exit 1; done -ok comments on the workflow's own issue -ok ignores an issue with the same title opened by someone else -ok ignores a longer title that contains the phrase -ok picks the oldest matching issue -ok opens an issue when none exists -``` - -The local changelog check passed. No changelog fragment was added. - -Fresh-context code/security review of this follow-up found no actionable issues. +`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. From 690a8f50817b1af5058751364f2665a7ba1a6a70 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Fri, 30 Oct 2026 14:38:10 -0600 Subject: [PATCH 25/26] test: tighten live-only call assertions --- b2/live_only_calls_wire_test.go | 18 +++++++++++++++--- base/live_only_calls_test.go | 4 ++++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/b2/live_only_calls_wire_test.go b/b2/live_only_calls_wire_test.go index 796f412a..1c352190 100644 --- a/b2/live_only_calls_wire_test.go +++ b/b2/live_only_calls_wire_test.go @@ -52,9 +52,17 @@ func TestBucketRevealWire(t *testing.T) { if err := bucket.Reveal(context.Background(), "gone"); err != nil { t.Fatal(err) } - request := srv.Last(t, "b2_delete_file_version") - b2wiretest.AssertJSON(t, request, "fileId", "hide-id") - b2wiretest.AssertJSON(t, request, "fileName", "gone") + 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) @@ -122,6 +130,9 @@ func TestObjectAuthURLWire(t *testing.T) { 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") } @@ -131,6 +142,7 @@ func TestObjectAuthURLWire(t *testing.T) { 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":[]}`}, diff --git a/base/live_only_calls_test.go b/base/live_only_calls_test.go index b7a2eba4..490f6569 100644 --- a/base/live_only_calls_test.go +++ b/base/live_only_calls_test.go @@ -45,6 +45,10 @@ func TestUploadHangRequestsNewUploadURL(t *testing.T) { _, _ = 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) From 05d391102d9fc0c5e012ba0b773c4e98ff00d650 Mon Sep 17 00:00:00 2001 From: Felipe Fumero Date: Fri, 30 Oct 2026 14:38:20 -0600 Subject: [PATCH 26/26] docs(intent): cite key expiration fix --- intent/credential-free-live-calls/intent.md | 2 +- intent/credential-free-live-calls/spec.md | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/intent/credential-free-live-calls/intent.md b/intent/credential-free-live-calls/intent.md index 24abea5b..db9d7baf 100644 --- a/intent/credential-free-live-calls/intent.md +++ b/intent/credential-free-live-calls/intent.md @@ -35,4 +35,4 @@ Test code only. ## Open questions -None. +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/spec.md b/intent/credential-free-live-calls/spec.md index 1d5252ab..fd5c5321 100644 --- a/intent/credential-free-live-calls/spec.md +++ b/intent/credential-free-live-calls/spec.md @@ -27,7 +27,7 @@ authorization" (the `Authorization` and `b2ContentDisposition` URL parameters); 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 deferred (see Areas of concern). Capabilities mapping is tracked separately in #220. + 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. @@ -94,7 +94,5 @@ authorization" (the `Authorization` and `b2ContentDisposition` URL parameters); 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 needs a separate issue.** The documented `expirationTimestamp: null` currently - produces `1970-01-01T00:00:00Z` (`IsZero=false`) through `Key.Expires()`. Do not pin that value as correct. - The fixture uses null and asserts the key's name and ID; propose a separate production fix to preserve - the absence of an expiry. 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.