[ffumero2003]

Context

base/base.go converted a key or download-authorization lifetime to the wire validDurationInSeconds with int(valid.Seconds()), which truncates. A positive sub-second duration became 0:

• CreateKey / CreateKeyMultiBucket: validDurationInSeconds is omitempty, so a 0 was dropped and B2 created a never-expiring key.
• GetDownloadAuthorization: the field is required, so a 0 was sent and rejected by B2.

Change

Add a validDurationSeconds helper that returns an error for a positive duration below one second, applied before the request at all three call sites. Document the behavior on the exported methods and b2.Lifetime (valid is whole seconds; a positive value below 1s errors; 0 means no expiration).

Verification

New tests in base/base_v4_test.go: a 500ms lifetime is rejected before any request at each call site; 1s sends 1; 0 is unchanged. Removing the guard fails the reject tests. make ci passes.

Closes #282

[sophiecarreras]

This fixes the positive sub-second case cleanly and the tests are solid (the request is asserted not to be sent). One hole remains in the other direction: validDurationSeconds only rejects valid > 0 && secs == 0, so -500ms (and -1ns) still returns 0, which omitempty turns into a never-expiring key. It is reachable through b2.Deadline(t) when the deadline passed less than a second ago, because it computes t.Sub(time.Now()) (b2/key.go). Please reject valid < 0 too, with tests for -500ms, -1s and Deadline(past).

Related:

• int(valid.Seconds()) is unbounded, so a very large duration wraps on 32-bit builds (linux/386 and arm are in the CI matrix). Consider rejecting anything above math.MaxInt32 seconds, or above the API's maximum key lifetime (please confirm the documented limit).
• A sentinel error (for example ErrInvalidDuration) would let callers use errors.Is; the message prefix says "blazer/base:" while neighbouring errors use "b2:", worth checking against the package convention.
• A valid == 0 download authorization is still sent and left for the server to reject; rejecting it client-side would be friendlier, but optional.
• Tests worth adding at the b2 level: Lifetime(500ms) and AuthURL with a sub-second duration, negative values, the 1-second boundary on the multi-bucket path, and a huge duration.

The area-owner-approvals status is the only non-green item and is only waiting on an approval; the rest of CI is green.

[sophiecarreras]

Review from Sofia, with Claude's help. Read only, not run. No blocking findings.

The helper rejects positive sub-second lifetimes before any request, and the tests check both the error and that no request was sent, plus the one-hour and zero (non-expiring key) cases. Marking it Breaking in the changelog is right.

Small gaps:

1. validDurationSeconds only rejects valid > 0 && secs == 0. A negative duration is sent as negative seconds, and 1.5s is silently truncated to 1. Worth a test and a decision for both.
2. Overlap with our open Blazer PR #281: both edit b2/b2.go and base/base.go. Each merges cleanly on its own today; the second to merge may need a rebase.

Comment only, not an approval.

[ffumero2003]

Thanks @sophiecarreras — all addressed in 682dfe9 (CI green; only the area-owner approval gate remains):

• Fixed — validDurationSeconds now rejects valid < 0 (which previously returned 0 → a never-expiring key via omitempty, reachable through Deadline(past)). Tests cover -500ms, -1s and a just-passed Deadline(t), asserting no request is sent.
• Fixed — fractional seconds are rejected (1.5s no longer truncates to 1), with b2-level tests on Lifetime/AuthURL.
• Fixed — 32-bit overflow: durations >= 1000 days are rejected (the documented b2_create_key maximum), which also keeps the value within int32; switched to integer division (valid / time.Second).
• Fixed — errors now wrap a sentinel base.ErrInvalidDuration (so errors.Is works) and use the b2: message prefix to match the package convention.
• Noted (optional) — a valid == 0 download authorization is still sent for the server to reject (the server requires >= 1s); documented in the godoc. Can reject it client-side too if you'd prefer it symmetric.
• Acknowledged — overlap with Fix failed bucket-update rollback (#108), ReadFrom offset (#181), Close panic and URL escaping; strengthen tests #281 (both edit b2/b2.go and base/base.go); I'll rebase whichever merges second.

Break-proofs: each guard removed turns its test red; restored → green. Re-requesting your review.

[david-santacruz-backblaze]

Mate, let's check this before merge.

Deadline breaks for any future time. It works out the lifetime as t.Sub(time.Now()), which is never a whole number of seconds, so the new whole-second check in validDurationSeconds rejects it:

CreateKey(ctx, "key", Deadline(time.Now().Add(time.Hour)))
// invalid duration 59m59.999999834s: must be a whole number of seconds

That worked on main. #282 only asks us to reject durations under 1s, so I'd cut the check back to 0 < valid < time.Second, and add a test with a future Deadline.

[ffumero2003]

Fixed in e07c570. Cut validDurationSeconds back to reject only negative, sub-second (< time.Second), and ≥1000-day lifetimes; fractional durations ≥1s now truncate via int(valid / time.Second) as on main, so Deadline(future) works again.

Added a future-Deadline wire test (asserts it succeeds and sends time.Until(deadline)/time.Second) and a direct base unit test (1.5s → 1, 500ms → err, negative, ≥1000 days). Updated the godoc and the **Breaking:** changelog note to describe the truncation. go test -race -count=5 -shuffle=on ./base ./b2/..., gofmt, vet and make ci all clean. Thanks for the catch.

[ffumero2003]

Windows CI flake fixed in ad0eb9b. The future_deadline test compared the sent validDurationInSeconds against a second time.Until(deadline) call, so when the SDK's clock read and the test's read straddled a one-second truncation boundary it could see 3599 vs 3600 (more likely on Windows's coarser clock). It now accepts either value — both are correct truncations of a one-hour deadline — with a comment. No production change; make ci green with creds unset.