Skip to content

fix(go): correct buffer size for CreateUser and UpdatePermissions serialization#3097

Open
atharvalade wants to merge 1 commit intoapache:masterfrom
atharvalade:fix/go-sdk-createuser-permissions-len
Open

fix(go): correct buffer size for CreateUser and UpdatePermissions serialization#3097
atharvalade wants to merge 1 commit intoapache:masterfrom
atharvalade:fix/go-sdk-createuser-permissions-len

Conversation

@atharvalade
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #2981

Rationale

The Go SDK's CreateUser and UpdatePermissions commands had incorrect buffer size calculations for the has_permissions flag byte, causing a 1-byte over-allocation in CreateUser (non-nil path) and an index-out-of-bounds panic in UpdatePermissions (nil path).

What changed?

Both CreateUser.MarshalBinary() and UpdatePermissions.MarshalBinary() double-counted the 1-byte has_permissions flag in the non-nil branch, while UpdatePermissions omitted it entirely from the base allocation (causing a panic when permissions were nil).

The fix accounts for the has_permissions byte exactly once: in CreateUser it is part of the base capacity 4, and in UpdatePermissions it is now added to the base length. The +1 was removed from the non-nil branch in both methods. Unit tests were added for all four cases (nil/non-nil x CreateUser/UpdatePermissions).

Local Execution

  • Passed
  • Pre-commit hooks ran (trailing-whitespace, trailing-newline, license-headers all pass; go test ./... all pass; gofmt clean; go vet clean)

AI Usage

  1. Opus 4.6
  2. Minimal AI used
  3. Yes, all code can be explained

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.72%. Comparing base (27f0f11) to head (8ec4cfd).

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #3097   +/-   ##
=========================================
  Coverage     72.71%   72.72%           
  Complexity      943      943           
=========================================
  Files          1117     1117           
  Lines         96285    96285           
  Branches      73485    73485           
=========================================
+ Hits          70014    70020    +6     
+ Misses        23725    23719    -6     
  Partials       2546     2546           
Components Coverage Δ
Rust Core 73.45% <ø> (ø)
Java SDK 62.30% <ø> (ø)
C# SDK 69.40% <ø> (ø)
Python SDK 81.43% <ø> (ø)
Node SDK 91.53% <ø> (ø)
Go SDK 39.09% <100.00%> (+0.12%) ⬆️
Files with missing lines Coverage Δ
foreign/go/internal/command/user.go 91.30% <100.00%> (+6.52%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Go SDK: CreateUser omits mandatory permissions_len field when permissions are nil

1 participant