Skip to content

test(go): cover access token command serialization #4640

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
p4rtridge wants to merge 7 commits into
base: master
Choose a base branch
from
+72 −14
Open

test(go): cover access token command serialization #4640

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e1b8ad2
test(go): cover access token command serialization
p4rtridge Oct 25, 2026
c883415
test(go): change test failure messages to use hex format
p4rtridge Oct 26, 2026
0877902
Merge branch 'master' into go-access-token-serialization-2883
p4rtridge Oct 26, 2026
3322fda
Merge branch 'master' into go-access-token-serialization-2883
p4rtridge Oct 29, 2026
5784311
Merge branch 'master' into go-access-token-serialization-2883
p4rtridge Nov 1, 2026
82fa460
test(go): simplify access token tests
p4rtridge Nov 1, 2026
e72bd78
Merge branch 'master' into go-access-token-serialization-2883
p4rtridge Nov 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 72 additions & 14 deletions foreign/go/internal/command/access_token_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,84 @@ import (
iggcon "github.com/apache/iggy/foreign/go/contracts"
)

func TestCreatePersonalAccessTokenMarshalBinary(t *testing.T) {
tests := []struct {
name string
expiry iggcon.Duration
want []byte
func TestPersonalAccessTokenCommandsMarshalBinary(t *testing.T) {
for _, test := range []struct {
name string
request Command
code Code
want []byte
}{
{"server default", iggcon.IggyExpiryServerDefault, []byte{5, 't', 'o', 'k', 'e', 'n', 0, 0, 0, 0, 0, 0, 0, 0}},
{"little endian", 0x01020304, []byte{5, 't', 'o', 'k', 'e', 'n', 4, 3, 2, 1, 0, 0, 0, 0}},
{"past 32 bits", 0x0102030405060708, []byte{5, 't', 'o', 'k', 'e', 'n', 8, 7, 6, 5, 4, 3, 2, 1}},
{"never expire", iggcon.IggyExpiryNeverExpire, []byte{5, 't', 'o', 'k', 'e', 'n', 255, 255, 255, 255, 255, 255, 255, 255}},
}

for _, test := range tests {
{
name: "server default expiry",
request: &CreatePersonalAccessToken{Name: "token", Expiry: iggcon.IggyExpiryServerDefault},
code: 42,
want: []byte{5, 't', 'o', 'k', 'e', 'n', 0, 0, 0, 0, 0, 0, 0, 0},
},
{
name: "little endian expiry",
request: &CreatePersonalAccessToken{Name: "token", Expiry: 0x01020304},
code: 42,
want: []byte{5, 't', 'o', 'k', 'e', 'n', 4, 3, 2, 1, 0, 0, 0, 0},
},
{
name: "expiry past 32 bits",
request: &CreatePersonalAccessToken{Name: "token", Expiry: 0x0102030405060708},
code: 42,
want: []byte{5, 't', 'o', 'k', 'e', 'n', 8, 7, 6, 5, 4, 3, 2, 1},
},
{
name: "never expire",
request: &CreatePersonalAccessToken{Name: "token", Expiry: iggcon.IggyExpiryNeverExpire},
code: 42,
want: []byte{5, 't', 'o', 'k', 'e', 'n', 255, 255, 255, 255, 255, 255, 255, 255},
},
{
name: "empty name",
request: &CreatePersonalAccessToken{Name: "", Expiry: 0x01020304},

@github-actions github-actions Bot Oct 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: The empty name rows assert a body the server rejects, because WireName requires 1 to 255 bytes (core/binary_protocol/src/primitives/identifier.rs:105). Replace them with a one-character name, the shortest name WireName accepts. Also at line 130.

@p4rtridge p4rtridge Oct 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @justinmclean, after reviewing core/binary_protocol/src/primitives/identifier.rs, I noticed that CreatePersonalAccessToken does not validate the token name’s length. As a result, submitting an empty name causes the server to reject the request.
I suggest adding token name validation in a separate PR to keep this PR focused on its current scope. What do you think?

code: 42,
want: []byte{0, 4, 3, 2, 1, 0, 0, 0, 0},
},
{
name: "longer name",
request: &CreatePersonalAccessToken{Name: "my-access-token", Expiry: iggcon.IggyExpiryServerDefault},

@github-actions github-actions Bot Oct 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: The name rows stop at 15 bytes, so the 255-byte wire limit and the byte(len(name)) wrap above 255 stay untested (foreign/go/internal/command/access_token.go:38). Add a 255-byte case and a case above 255 that expects an error.

code: 42,
want: []byte{15, 'm', 'y', '-', 'a', 'c', 'c', 'e', 's', 's', '-', 't', 'o', 'k', 'e', 'n', 0, 0, 0, 0, 0, 0, 0, 0},
},
{
name: "get tokens with no request payload",
request: &GetPersonalAccessTokens{},
code: 41,
want: []byte{},
},
{
name: "short name",
request: &DeletePersonalAccessToken{Name: "token"},
code: 43,
want: []byte{5, 't', 'o', 'k', 'e', 'n'},
},
{
name: "empty name",
request: &DeletePersonalAccessToken{Name: ""},
code: 43,
want: []byte{0},
},
{
name: "longer name",
request: &DeletePersonalAccessToken{Name: "my-access-token"},
code: 43,
want: []byte{15, 'm', 'y', '-', 'a', 'c', 'c', 'e', 's', 's', '-', 't', 'o', 'k', 'e', 'n'},
},
} {
t.Run(test.name, func(t *testing.T) {
got, err := (&CreatePersonalAccessToken{Name: "token", Expiry: test.expiry}).MarshalBinary()
if got := test.request.Code(); got != test.code {
t.Fatalf("command code = %d, want %d", got, test.code)
}
got, err := test.request.MarshalBinary()
if err != nil {
t.Fatal(err)
}
if !bytes.Equal(got, test.want) {
t.Fatalf("CreatePersonalAccessToken body = % x, want % x", got, test.want)
t.Fatalf("command body = %x, want %x", got, test.want)
}
})
}
Expand Down