atharvalade commented on code in PR #2973: URL: https://github.com/apache/iggy/pull/2973#discussion_r2958694591
########## foreign/go/internal/command/access_token_test.go: ########## @@ -0,0 +1,225 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package command + +import ( + "bytes" + "testing" +) + +// TestSerialize_GetPersonalAccessTokens tests serialization of GetPersonalAccessTokens command +func TestSerialize_GetPersonalAccessTokens(t *testing.T) { + cmd := GetPersonalAccessTokens{} + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize GetPersonalAccessTokens: %v", err) + } + + expected := []byte{} // Empty byte array + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_DeletePersonalAccessToken tests serialization with normal token name +func TestSerialize_DeletePersonalAccessToken(t *testing.T) { + cmd := DeletePersonalAccessToken{ + Name: "test_token", + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize DeletePersonalAccessToken: %v", err) + } + + expected := []byte{ + 0x0A, // Name length = 10 + 0x74, 0x65, 0x73, 0x74, 0x5F, 0x74, 0x6F, 0x6B, 0x65, 0x6E, // "test_token" + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_DeletePersonalAccessToken_SingleChar tests edge case with single character name +func TestSerialize_DeletePersonalAccessToken_SingleChar(t *testing.T) { + cmd := DeletePersonalAccessToken{ + Name: "a", + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize DeletePersonalAccessToken with single char: %v", err) + } + + expected := []byte{ + 0x01, // Name length = 1 + 0x61, // "a" + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_DeletePersonalAccessToken_EmptyName tests edge case with empty name +func TestSerialize_DeletePersonalAccessToken_EmptyName(t *testing.T) { + cmd := DeletePersonalAccessToken{ + Name: "", + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize DeletePersonalAccessToken with empty name: %v", err) + } + + expected := []byte{ + 0x00, // Name length = 0 + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_CreatePersonalAccessToken tests serialization with normal values +func TestSerialize_CreatePersonalAccessToken(t *testing.T) { + cmd := CreatePersonalAccessToken{ + Name: "test", + Expiry: 3600, // 1 hour in seconds + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize CreatePersonalAccessToken: %v", err) + } + + expected := []byte{ + 0x04, // Name length = 4 + 0x74, 0x65, 0x73, 0x74, // "test" + 0x00, 0x00, 0x00, 0x00, // Reserved/padding (4 bytes) + 0x10, 0x0E, 0x00, 0x00, // Expiry = 3600 (little endian uint32) + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_CreatePersonalAccessToken_ZeroExpiry tests edge case with zero expiry +func TestSerialize_CreatePersonalAccessToken_ZeroExpiry(t *testing.T) { + cmd := CreatePersonalAccessToken{ + Name: "token", + Expiry: 0, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize CreatePersonalAccessToken with zero expiry: %v", err) + } + + expected := []byte{ Review Comment: These 4 zero bytes are not "Reserved/padding" -- the Rust wire format specifies `[expiry:u64_le]` starting at position 1 + `name_len` (see `core/binary_protocol/.../create_personal_access_token.rs:25`). The Go implementation uses `PutUint32` at `len(bytes)-4`, which writes the expiry in the high 32 bits of the `u64` field, causing the server to interpret the value as expiry << 32. The underlying bug is in `access_token.go:36` -- it should use `PutUint64(bytes[1+len(c.Name):]`, `uint64(c.Expiry))`, and the `Expiry` field type should be `uint64` to match the wire protocol. These tests currently codify the buggy serialization. ########## foreign/go/internal/command/user_test.go: ########## @@ -0,0 +1,439 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package command + +import ( + "bytes" + "testing" + + iggcon "github.com/apache/iggy/foreign/go/contracts" +) + +// Helper to create test permissions with only global permissions +func createTestGlobalPermissions(all bool) iggcon.GlobalPermissions { + return iggcon.GlobalPermissions{ + ManageServers: all, + ReadServers: all, + ManageUsers: all, + ReadUsers: all, + ManageStreams: all, + ReadStreams: all, + ManageTopics: all, + ReadTopics: all, + PollMessages: all, + SendMessages: all, + } +} + +func createTestPermissions(global iggcon.GlobalPermissions) *iggcon.Permissions { + return &iggcon.Permissions{ + Global: global, + Streams: nil, // Simple case: no stream-specific permissions + } +} + +// TestSerialize_CreateUser_WithPermissions_ActiveStatus tests CreateUser with permissions and Active status +func TestSerialize_CreateUser_WithPermissions_ActiveStatus(t *testing.T) { + globalPerms := createTestGlobalPermissions(true) + permissions := createTestPermissions(globalPerms) + + cmd := CreateUser{ + Username: "admin", + Password: "secret", + Status: iggcon.Active, + Permissions: permissions, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize CreateUser: %v", err) + } + + expected := []byte{ + 0x05, // Username length = 5 + 0x61, 0x64, 0x6D, 0x69, 0x6E, // "admin" + 0x06, // Password length = 6 + 0x73, 0x65, 0x63, 0x72, 0x65, 0x74, // "secret" + 0x01, // Status = Active (1) + 0x01, // Has permissions = 1 + 0x0B, 0x00, 0x00, 0x00, // Permissions length = 11 + // Global permissions (10 bytes) - all true + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x00, // No stream-specific permissions + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_CreateUser_WithoutPermissions tests CreateUser without permissions +func TestSerialize_CreateUser_WithoutPermissions(t *testing.T) { + cmd := CreateUser{ + Username: "user1", + Password: "pass123", + Status: iggcon.Active, + Permissions: nil, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize CreateUser without permissions: %v", err) + } + + expected := []byte{ + 0x05, // Username length = 5 + 0x75, 0x73, 0x65, 0x72, 0x31, // "user1" + 0x07, // Password length = 7 + 0x70, 0x61, 0x73, 0x73, 0x31, 0x32, 0x33, // "pass123" + 0x01, // Status = Active (1) + 0x00, // Has permissions = 0 + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_CreateUser_InactiveStatus tests CreateUser with Inactive status +func TestSerialize_CreateUser_InactiveStatus(t *testing.T) { + cmd := CreateUser{ + Username: "inactive_user", + Password: "pwd", + Status: iggcon.Inactive, + Permissions: nil, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize CreateUser with inactive status: %v", err) + } + + expected := []byte{ + 0x0D, // Username length = 13 + 0x69, 0x6E, 0x61, 0x63, 0x74, 0x69, 0x76, 0x65, 0x5F, 0x75, 0x73, 0x65, 0x72, // "inactive_user" + 0x03, // Password length = 3 + 0x70, 0x77, 0x64, // "pwd" + 0x02, // Status = Inactive (2) + 0x00, // Has permissions = 0 + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_CreateUser_EmptyCredentials tests edge case with empty username/password +func TestSerialize_CreateUser_EmptyCredentials(t *testing.T) { + cmd := CreateUser{ + Username: "", + Password: "", + Status: iggcon.Active, + Permissions: nil, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize CreateUser with empty credentials: %v", err) + } + + expected := []byte{ + 0x00, // Username length = 0 + 0x00, // Password length = 0 + 0x01, // Status = Active (1) + 0x00, // Has permissions = 0 + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_CreateUser_PartialPermissions tests with partial global permissions +func TestSerialize_CreateUser_PartialPermissions(t *testing.T) { + globalPerms := iggcon.GlobalPermissions{ + ManageServers: true, + ReadServers: true, + ManageUsers: false, + ReadUsers: true, + ManageStreams: false, + ReadStreams: true, + ManageTopics: false, + ReadTopics: true, + PollMessages: true, + SendMessages: false, + } + permissions := createTestPermissions(globalPerms) + + cmd := CreateUser{ + Username: "user", + Password: "pass", + Status: iggcon.Active, + Permissions: permissions, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize CreateUser with partial permissions: %v", err) + } + + expected := []byte{ + 0x04, // Username length = 4 + 0x75, 0x73, 0x65, 0x72, // "user" + 0x04, // Password length = 4 + 0x70, 0x61, 0x73, 0x73, // "pass" + 0x01, // Status = Active (1) + 0x01, // Has permissions = 1 + 0x0B, 0x00, 0x00, 0x00, // Permissions length = 11 + // Global permissions: 1,1,0,1,0,1,0,1,1,0 + 0x01, 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01, 0x01, 0x00, + 0x00, // No stream-specific permissions + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_GetUsers tests GetUsers serialization (empty) +func TestSerialize_GetUsers(t *testing.T) { + cmd := GetUsers{} + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize GetUsers: %v", err) + } + + expected := []byte{} // Empty byte array + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_GetUser_NumericId tests GetUser with numeric identifier +func TestSerialize_GetUser_NumericId(t *testing.T) { + userId, _ := iggcon.NewIdentifier(uint32(123)) + + cmd := GetUser{ + Id: userId, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize GetUser with numeric ID: %v", err) + } + + expected := []byte{ + 0x01, // Kind = NumericId + 0x04, // Length = 4 + 0x7B, 0x00, 0x00, 0x00, // Value = 123 + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_GetUser_StringId tests GetUser with string identifier +func TestSerialize_GetUser_StringId(t *testing.T) { + userId, _ := iggcon.NewIdentifier("admin") + + cmd := GetUser{ + Id: userId, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize GetUser with string ID: %v", err) + } + + expected := []byte{ + 0x02, // Kind = StringId + 0x05, // Length = 5 + 0x61, 0x64, 0x6D, 0x69, 0x6E, // "admin" + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_UpdatePermissions_WithPermissions tests UpdatePermissions with permissions +func TestSerialize_UpdatePermissions_WithPermissions(t *testing.T) { + userId, _ := iggcon.NewIdentifier(uint32(42)) + globalPerms := createTestGlobalPermissions(false) + permissions := createTestPermissions(globalPerms) + + cmd := UpdatePermissions{ + UserID: userId, + Permissions: permissions, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize UpdatePermissions: %v", err) + } + + expected := []byte{ + 0x01, // UserId Kind = NumericId + 0x04, // UserId Length = 4 + 0x2A, 0x00, 0x00, 0x00, // UserId Value = 42 + 0x01, // Has permissions = 1 + 0x0B, 0x00, 0x00, 0x00, // Permissions length = 11 + // Global permissions (all false = 0) + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, // No stream-specific permissions + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_UpdatePermissions_Nil tests UpdatePermissions without permissions +func TestSerialize_UpdatePermissions_Nil(t *testing.T) { + userId, _ := iggcon.NewIdentifier("user123") + + cmd := UpdatePermissions{ + UserID: userId, + Permissions: nil, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize UpdatePermissions without permissions: %v", err) + } + + expected := []byte{ + 0x02, // UserId Kind = StringId + 0x07, // UserId Length = 7 + 0x75, 0x73, 0x65, 0x72, 0x31, 0x32, 0x33, // "user123" + 0x00, // Has permissions = 0 + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_ChangePassword tests ChangePassword serialization +func TestSerialize_ChangePassword(t *testing.T) { + userId, _ := iggcon.NewIdentifier(uint32(1)) + + cmd := ChangePassword{ + UserID: userId, + CurrentPassword: "old_pass", + NewPassword: "new_pass", + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize ChangePassword: %v", err) + } + + expected := []byte{ + 0x01, // UserId Kind = NumericId + 0x04, // UserId Length = 4 + 0x01, 0x00, 0x00, 0x00, // UserId Value = 1 + 0x08, // CurrentPassword length = 8 + 0x6F, 0x6C, 0x64, 0x5F, 0x70, 0x61, 0x73, 0x73, // "old_pass" + 0x08, // NewPassword length = 8 + 0x6E, 0x65, 0x77, 0x5F, 0x70, 0x61, 0x73, 0x73, // "new_pass" + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_ChangePassword_EmptyPasswords tests edge case with empty passwords +func TestSerialize_ChangePassword_EmptyPasswords(t *testing.T) { + userId, _ := iggcon.NewIdentifier("admin") + + cmd := ChangePassword{ + UserID: userId, + CurrentPassword: "", + NewPassword: "", + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize ChangePassword with empty passwords: %v", err) + } + + expected := []byte{ + 0x02, // UserId Kind = StringId + 0x05, // UserId Length = 5 + 0x61, 0x64, 0x6D, 0x69, 0x6E, // "admin" + 0x00, // CurrentPassword length = 0 + 0x00, // NewPassword length = 0 + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_DeleteUser_NumericId tests DeleteUser with numeric identifier +func TestSerialize_DeleteUser_NumericId(t *testing.T) { + userId, _ := iggcon.NewIdentifier(uint32(999)) + + cmd := DeleteUser{ + Id: userId, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize DeleteUser with numeric ID: %v", err) + } + + expected := []byte{ + 0x01, // Kind = NumericId + 0x04, // Length = 4 + 0xE7, 0x03, 0x00, 0x00, // Value = 999 + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_DeleteUser_StringId tests DeleteUser with string identifier +func TestSerialize_DeleteUser_StringId(t *testing.T) { + userId, _ := iggcon.NewIdentifier("test_user") + + cmd := DeleteUser{ + Id: userId, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize DeleteUser with string ID: %v", err) + } + + expected := []byte{ + 0x02, // Kind = StringId Review Comment: This fix prevents the nil-permissions panic (good), but it's incomplete. The base length should be `len(userIdBytes)` + 1 + 4 because the Rust wire format always includes` permissions_len:u32_le` on the wire even when `has_permissions`=0 (see `update_permissions.rs:37-39`). As-is, the server will fail to decode the message because it reads 4 bytes for `permissions_len` that aren't present. The else branch at line 142 also needs to write `permissions_len`=0 (4 zero bytes) after the flag byte. ########## foreign/go/internal/command/user_test.go: ########## @@ -0,0 +1,439 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package command + +import ( + "bytes" + "testing" + + iggcon "github.com/apache/iggy/foreign/go/contracts" +) + +// Helper to create test permissions with only global permissions +func createTestGlobalPermissions(all bool) iggcon.GlobalPermissions { + return iggcon.GlobalPermissions{ + ManageServers: all, + ReadServers: all, + ManageUsers: all, + ReadUsers: all, + ManageStreams: all, + ReadStreams: all, + ManageTopics: all, + ReadTopics: all, + PollMessages: all, + SendMessages: all, + } +} + +func createTestPermissions(global iggcon.GlobalPermissions) *iggcon.Permissions { + return &iggcon.Permissions{ + Global: global, + Streams: nil, // Simple case: no stream-specific permissions + } +} + +// TestSerialize_CreateUser_WithPermissions_ActiveStatus tests CreateUser with permissions and Active status +func TestSerialize_CreateUser_WithPermissions_ActiveStatus(t *testing.T) { + globalPerms := createTestGlobalPermissions(true) + permissions := createTestPermissions(globalPerms) + + cmd := CreateUser{ + Username: "admin", + Password: "secret", + Status: iggcon.Active, + Permissions: permissions, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize CreateUser: %v", err) + } + + expected := []byte{ + 0x05, // Username length = 5 + 0x61, 0x64, 0x6D, 0x69, 0x6E, // "admin" + 0x06, // Password length = 6 + 0x73, 0x65, 0x63, 0x72, 0x65, 0x74, // "secret" + 0x01, // Status = Active (1) + 0x01, // Has permissions = 1 + 0x0B, 0x00, 0x00, 0x00, // Permissions length = 11 + // Global permissions (10 bytes) - all true + 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, 0x01, + 0x00, // No stream-specific permissions + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_CreateUser_WithoutPermissions tests CreateUser without permissions +func TestSerialize_CreateUser_WithoutPermissions(t *testing.T) { + cmd := CreateUser{ + Username: "user1", + Password: "pass123", + Status: iggcon.Active, + Permissions: nil, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize CreateUser without permissions: %v", err) + } + + expected := []byte{ + 0x05, // Username length = 5 + 0x75, 0x73, 0x65, 0x72, 0x31, // "user1" + 0x07, // Password length = 7 + 0x70, 0x61, 0x73, 0x73, 0x31, 0x32, 0x33, // "pass123" + 0x01, // Status = Active (1) + 0x00, // Has permissions = 0 + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_CreateUser_InactiveStatus tests CreateUser with Inactive status +func TestSerialize_CreateUser_InactiveStatus(t *testing.T) { + cmd := CreateUser{ + Username: "inactive_user", + Password: "pwd", + Status: iggcon.Inactive, + Permissions: nil, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize CreateUser with inactive status: %v", err) + } + + expected := []byte{ + 0x0D, // Username length = 13 + 0x69, 0x6E, 0x61, 0x63, 0x74, 0x69, 0x76, 0x65, 0x5F, 0x75, 0x73, 0x65, 0x72, // "inactive_user" + 0x03, // Password length = 3 + 0x70, 0x77, 0x64, // "pwd" + 0x02, // Status = Inactive (2) + 0x00, // Has permissions = 0 + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_CreateUser_EmptyCredentials tests edge case with empty username/password +func TestSerialize_CreateUser_EmptyCredentials(t *testing.T) { + cmd := CreateUser{ + Username: "", + Password: "", + Status: iggcon.Active, + Permissions: nil, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize CreateUser with empty credentials: %v", err) + } + + expected := []byte{ + 0x00, // Username length = 0 + 0x00, // Password length = 0 + 0x01, // Status = Active (1) + 0x00, // Has permissions = 0 + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) + } +} + +// TestSerialize_CreateUser_PartialPermissions tests with partial global permissions +func TestSerialize_CreateUser_PartialPermissions(t *testing.T) { + globalPerms := iggcon.GlobalPermissions{ + ManageServers: true, + ReadServers: true, + ManageUsers: false, + ReadUsers: true, + ManageStreams: false, + ReadStreams: true, + ManageTopics: false, + ReadTopics: true, + PollMessages: true, + SendMessages: false, + } + permissions := createTestPermissions(globalPerms) + + cmd := CreateUser{ + Username: "user", + Password: "pass", + Status: iggcon.Active, + Permissions: permissions, + } + + serialized, err := cmd.MarshalBinary() + if err != nil { + t.Fatalf("Failed to serialize CreateUser with partial permissions: %v", err) + } + + expected := []byte{ + 0x04, // Username length = 4 + 0x75, 0x73, 0x65, 0x72, // "user" + 0x04, // Password length = 4 + 0x70, 0x61, 0x73, 0x73, // "pass" + 0x01, // Status = Active (1) + 0x01, // Has permissions = 1 + 0x0B, 0x00, 0x00, 0x00, // Permissions length = 11 + // Global permissions: 1,1,0,1,0,1,0,1,1,0 + 0x01, 0x01, 0x00, 0x01, 0x00, 0x01, 0x00, 0x01, 0x01, 0x00, + 0x00, // No stream-specific permissions + } + + if !bytes.Equal(serialized, expected) { + t.Errorf("Serialized bytes are incorrect.\nExpected:\t%v\nGot:\t\t%v", expected, serialized) Review Comment: This expected output is missing the `permissions_len:u32_le` field. The Rust wire format (see `create_user.rs:44-45`) always includes `permissions_len` on the wire, even when `has_permissions`=0. The expected bytes should end with 0x01, 0x00, 0x00, 0x00, 0x00, 0x00 (status, `has_permissions`=0, `permissions_len`=0). The underlying bug is in `user.go:77` where the nil-permissions branch only writes the flag byte but not the length field. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
