Simplify object store base64url encoder

[mtmk]

Replace the object store's hand-rolled base64url encoder. The old loop allocated a fresh char[] on every call; the new code encodes into a stack-allocated buffer (pooled for inputs over 512 chars), so only the result string is allocated. On net9.0+ it uses the built-in Base64Url encoder (url-safe in a single pass); on older target frameworks it keeps a lookup-table loop over the url-safe alphabet. The '=' padding is retained so digests and metadata subject names stay wire-compatible with the other clients. The read path compares the digest as spans rather than building an interpolated string. Verified against the Go client by round-tripping objects in both directions.

Benchmark below (Intel Xeon w5-2445, BenchmarkDotNet ShortRun; baseline = the old new char[] table encoder, current = this PR). Allocation halves across every size and runtime. Throughput improves substantially on net9.0+ where the built-in Base64Url path runs (up to ~4.5x on 4 KB input); on net8.0 it is roughly unchanged, and on net481 it is slightly slower at small inputs, so on pre-net9 frameworks the change is an allocation reduction rather than a speedup. Size 32 is the SHA-256 digest, the common case.

Runtime	Size	Mean (base -> current)	Alloc (base -> current)
net481	16	48.3 -> 68.8 ns	152 -> 80 B
net481	32	72.8 -> 92.0 ns	233 -> 120 B
net481	256	450 -> 458 ns	1436 -> 722 B
net481	4096	6686 -> 6500 ns	21960 -> 10984 B
net8.0	16	29.7 -> 29.7 ns	144 -> 72 B
net8.0	32	47.1 -> 51.5 ns	224 -> 112 B
net8.0	256	291 -> 273 ns	1424 -> 712 B
net8.0	4096	4443 -> 3998 ns	21904 -> 10952 B
net10.0	16	27.1 -> 18.7 ns	144 -> 72 B
net10.0	32	43.7 -> 26.0 ns	224 -> 112 B
net10.0	256	279 -> 89 ns	1424 -> 712 B
net10.0	4096	4258 -> 933 ns	21904 -> 10952 B

Supersedes #565; original work by @to11mtm.

Fixes #557

PS opened a proposal in the dotnet/runtime repo to cover this case so we can avoid the extra translation step dotnet/runtime#129847

[github-actions]

🚀 Deployed on https://6a43a18604cec78f9e1bd8ae--teal-klepon-7e92d0.netlify.app

[mtmk]

@claude please review again, there was a typo preventing build before.

[synadia-claude-reviewer]

Claude finished @mtmk's task in 1m 28s —— View job

---

PR Review: Simplify object store base64url encoder

• Gather context
• Read current state of Encoder.cs
• Read current state of NatsObjStore.cs
• Post review findings

---

src/NATS.Client.ObjectStore/NatsObjStore.cs

The previous compilation error is fixed — memoryOwner is correctly named at line 108. The rest of the changes in this file are clean:

• Digest null guard (line 131): necessary addition — the old code would throw a NullReferenceException on info.Digest being null rather than a descriptive NatsObjException.
• StartsWith with StringComparison.Ordinal (line 132): correct and avoids culture-sensitive comparison.
• AsSpan().SequenceEqual() (line 133): avoids an intermediate string allocation versus the old $"SHA-256={digest}" interpolation. ✓
• Comment grammar fixes (lines 99–100): minor typo fixes ("ot avoid" → "to avoid", "an request-reply" → "a request-reply"). ✓

---

src/NATS.Client.ObjectStore/Internal/Encoder.cs

The rewrite is correct and well-structured. Full walkthrough:

Encode method:

• base64Length = (length + 2) / 3 * 4 is the exact output size for Base64.EncodeToUtf8 with a final block — no over- or under-allocation possible.
• Discarding the OperationStatus return (line 71/86) is safe here: the output buffer is exactly sized and the input is always a complete span, so the result is always Done.
• netstandard2.0 path: rents from ArrayPool and correctly slices to written bytes before passing to MakeUrlSafe. The finally block guarantees return. ✓
• #else path: stackalloc for ≤512 bytes, pool for larger. SHA-256 digests encode to 44 bytes (32 input → 44 base64), well within the threshold. The try/finally null-checks rented before returning it. ✓

MakeUrlSafe method:

• In-place swap of +→- and /→_ is correct.
• raw=true short-circuits at the first =, returning i (the index of the padding character), which truncates the output before any padding — matching the old raw=true behaviour where j tracked the end of payload characters.
• raw=false processes the full span and returns utf8.Length — padding bytes produced by Base64.EncodeToUtf8 are left untouched, matching Go's base64.URLEncoding. ✓

Sha256 method: calls Encode(destination) with raw=false (default), keeping padding on digest strings — consistent with the wire format described in the PR. ✓

---

Verdict

No issues. The typo is fixed, logic is correct, and both the netstandard2.0 and modern paths handle memory correctly. LGTM. ✅

[MihaZupan]

It'd probably be more efficient to use EncodeToChars instead

You can also use the int-returning EncodeToUtf8/EncodeToChars helper if you're going to ignore the OperationStatus

[mtmk]

makes sense. using EncodeToChars now. thanks for the feedback.

[mtmk]

@claude please take another look. The design has changed substantially since your first review (the encoder now multi-targets a separate test project to pin each Base64UrlEncoder branch via NuGet asset selection, plus the apicompat workflow change). Please review the full current diff fresh.

[synadia-claude-reviewer]

Claude finished @mtmk's task in 4m 42s —— View job

---

PR Review: Simplify object store base64url encoder

• Read CLAUDE.md (not present)
• Get full diff vs origin/release/3.0
• Review Encoder.cs changes
• Review test project multi-targeting structure
• Review apicompat workflow changes
• Review NatsObjStore.cs changes
• Post comprehensive findings

---

src/NATS.Client.ObjectStore/Internal/Encoder.cs

NET9+ path (#if NET9_0_OR_GREATER, lines 78–98)

Base64Url.EncodeToChars(inArray, chars) returns the number of URL-safe chars written, without padding. The code then fills the remaining positions with = for raw=false — correctly implementing padded base64url to match Go's base64.URLEncoding. For raw=true it just slices to written chars. ✓

Stack/pool split at 512 chars is generous. SHA-256 digests → 44 chars, so the common path is always stackalloc. The try/finally null-checks rented before returning. ✓

Note: MihaZupan's first suggestion ("use EncodeToChars") is already addressed — the current code uses the int-returning Base64Url.EncodeToChars overload. His second suggestion (Span.Replace/EndsWith) targeted the previous MakeUrlSafe method which no longer exists. It would be worth replying to close those threads.

Pre-NET9 path (#else, lines 100–174) — applies to netstandard2.0, netstandard2.1, and net8.0

The lookup table uses the URL-safe alphabet directly (entries 62/63 are -/_), so no post-encode swap is needed. ✓

fixed (char* dst = chars, tbl = SBase64Table):

• SBase64Table is a char[] — pinneable. ✓
• chars is a Span<char> from either stackalloc or ArrayPool — both support GetPinnableReference() in C# 7.3+. ✓
• inArray stays a bounds-checked span (indices i and limit are derived from inArray.Length), so the unsafety is confined to dst/tbl. ✓

The new string(dst, 0, j) call is inside the fixed block — the constructor copies immediately, so the stack/pool memory is safe to release afterward. ✓

---

tests/NATS.Client.ObjectStore.Encoder.Tests/

Multi-targeting strategy (per src/Directory.Build.props → ObjectStore targets netstandard2.0;netstandard2.1;net8.0;net10.0):

Test TFM	ObjectStore TFM selected	Active branch
net6.0	netstandard2.1	#else path (table encoder)
net8.0	net8.0	#else path (table encoder)
net10.0	net10.0	#if NET9_0_OR_GREATER (Base64Url.EncodeToChars)
net481 (Windows)	netstandard2.0	#else + #if NETSTANDARD2_0 in Sha256

This is an elegant and correct approach — each TFM genuinely exercises a different compiled branch. CheckEolTargetFramework=false and SuppressTfmSupportBuildWarnings=true correctly silence EOL noise for net6.0/net481. ✓

Windows uses -p:TestTfmsInParallel=false which is necessary to avoid port conflicts in platform-test suites — correct here even though the encoder tests are unit tests (consistent with the platform tests' precedent). ✓

Test coverage gap: Base64UrlEncoderTest.cs has no test for raw=true mode and no test for Sha256() output format. Since raw=true is not used by NatsObjStore.cs today this is acceptable, but a Sha256 round-trip test (verifying the padded output matches the digest wire format) would catch any future regression.

---

src/NATS.Client.ObjectStore/NATS.Client.ObjectStore.csproj

Two new InternalsVisibleTo entries are added: Encoder.Tests and MicroBench2. The public key matches the existing Tests entry. ✓

Minor note: granting InternalsVisibleTo to a sandbox benchmark project from a production library is unusual. Since Base64UrlEncoder is internal, this is the only way to access it from the benchmark. Acceptable for this repo's conventions, but worth being aware that MicroBench2 must always keep the same signing key.

---

src/NATS.Client.ObjectStore/NatsObjStore.cs

Unchanged from the previous review: null guard, Ordinal comparison, and AsSpan().SequenceEqual() are all correct. ✓

---

.github/workflows/apicompat.yml and test workflows

apicompat.yml now installs net6, net8, and net10 SDKs — required because the ObjectStore project multi-targets to net10.0 and the apicompat script must build all TFMs. ✓

test_linux.yml and test_windows.yml both add the "Test Object Store Encoder" step and the net6 SDK. The Linux step runs without extra flags; the Windows step correctly adds -p:TestTfmsInParallel=false for the net481 TFM. ✓

---

Verdict

The design is sound and correct. Both encoding paths handle memory safely, the multi-TFM test project is a clean solution to pinning each compiler branch, and the CI changes follow naturally. The only open items are:

1. Close MihaZupan's review threads — both suggestions are now moot; a brief reply to each would keep the review history clean.
2. Optional: add a Sha256() test to Base64UrlEncoderTest.cs to guard the wire-format digest path.

LGTM ✅

[scottf]

LGTM