Skip to content

Commit

Permalink
*: Prohibit zero IDs
Browse files Browse the repository at this point in the history
This follows nspcc-dev/neofs-api#303. `IsZero`
methods and `ErrZero` errors are provided for callers to handle zeros.

Signed-off-by: Leonard Lyubich <[email protected]>
  • Loading branch information
cthulhu-rider committed Jul 25, 2024
1 parent 28b2676 commit 70e062d
Show file tree
Hide file tree
Showing 42 changed files with 459 additions and 363 deletions.
35 changes: 16 additions & 19 deletions bearer/bearer.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,9 @@ import (
//
// Instances can be created using built-in var declaration.
type Token struct {
targetUserSet bool
targetUser user.ID
targetUser user.ID

issuerSet bool
issuer user.ID
issuer user.ID

eaclTableSet bool
eaclTable eacl.Table
Expand Down Expand Up @@ -56,19 +54,23 @@ func (b *Token) readFromV2(m acl.BearerToken, checkFieldPresence bool) error {
}

targetUser := body.GetOwnerID()
if b.targetUserSet = targetUser != nil; b.targetUserSet {
if targetUser != nil {
err = b.targetUser.ReadFromV2(*targetUser)
if err != nil {
return fmt.Errorf("invalid target user: %w", err)
}
} else {
b.targetUser = user.ID{}
}

issuer := body.GetIssuer()
if b.issuerSet = issuer != nil; b.issuerSet {
if issuer != nil {
err = b.issuer.ReadFromV2(*issuer)
if err != nil {
return fmt.Errorf("invalid issuer: %w", err)
}
} else {
b.issuer = user.ID{}
}

lifetime := body.GetLifetime()
Expand Down Expand Up @@ -98,7 +100,7 @@ func (b *Token) ReadFromV2(m acl.BearerToken) error {
}

func (b Token) fillBody() *acl.BearerTokenBody {
if !b.eaclTableSet && !b.targetUserSet && !b.lifetimeSet && !b.issuerSet {
if !b.eaclTableSet && b.targetUser.IsZero() && !b.lifetimeSet && b.issuer.IsZero() {
return nil
}

Expand All @@ -108,14 +110,14 @@ func (b Token) fillBody() *acl.BearerTokenBody {
body.SetEACL(b.eaclTable.ToV2())
}

if b.targetUserSet {
if !b.targetUser.IsZero() {
var targetUser refs.OwnerID
b.targetUser.WriteToV2(&targetUser)

body.SetOwnerID(&targetUser)
}

if b.issuerSet {
if !b.issuer.IsZero() {
var issuer refs.OwnerID
b.issuer.WriteToV2(&issuer)

Expand Down Expand Up @@ -240,8 +242,8 @@ func (b Token) AssertContainer(cnr cid.ID) bool {
return true
}

cnrTable, set := b.eaclTable.CID()
return !set || cnrTable == cnr
cnrTable := b.eaclTable.LimitedContainer()
return cnrTable.IsZero() || cnrTable == cnr
}

// ForUser specifies ID of the user who can use the Token for the operations
Expand All @@ -252,7 +254,6 @@ func (b Token) AssertContainer(cnr cid.ID) bool {
// See also AssertUser.
func (b *Token) ForUser(id user.ID) {
b.targetUser = id
b.targetUserSet = true
}

// AssertUser checks if the Token is issued to the given user.
Expand All @@ -261,7 +262,7 @@ func (b *Token) ForUser(id user.ID) {
//
// See also ForUser.
func (b Token) AssertUser(id user.ID) bool {
return !b.targetUserSet || b.targetUser == id
return b.targetUser.IsZero() || b.targetUser == id
}

// Sign calculates and writes signature of the [Token] data along with issuer ID
Expand Down Expand Up @@ -403,7 +404,6 @@ func (b Token) SigningKeyBytes() []byte {
//
// See also [Token.Issuer], [Token.Sign].
func (b *Token) SetIssuer(usr user.ID) {
b.issuerSet = true
b.issuer = usr
}

Expand All @@ -413,10 +413,7 @@ func (b *Token) SetIssuer(usr user.ID) {
//
// See also [Token.SetIssuer], [Token.Sign].
func (b Token) Issuer() user.ID {
if b.issuerSet {
return b.issuer
}
return user.ID{}
return b.issuer
}

// ResolveIssuer works like [Token.Issuer] with fallback to the public key
Expand All @@ -425,7 +422,7 @@ func (b Token) Issuer() user.ID {
//
// See also [Token.SigningKeyBytes], [Token.Sign].
func (b Token) ResolveIssuer() user.ID {
if b.issuerSet {
if !b.issuer.IsZero() {
return b.issuer
}

Expand Down
4 changes: 2 additions & 2 deletions bearer/bearer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,11 @@ func TestToken_AssertContainer(t *testing.T) {

eaclTable := eacltest.Table()

eaclTable.SetCID(cidtest.ID())
eaclTable.LimitToContainer(cidtest.ID())
val.SetEACLTable(eaclTable)
require.False(t, val.AssertContainer(cnr))

eaclTable.SetCID(cnr)
eaclTable.LimitToContainer(cnr)
val.SetEACLTable(eaclTable)
require.True(t, val.AssertContainer(cnr))
}
Expand Down
6 changes: 2 additions & 4 deletions client/accounting.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,13 @@ var (
type PrmBalanceGet struct {
prmCommonMeta

accountSet bool
account user.ID
account user.ID
}

// SetAccount sets identifier of the NeoFS account for which the balance is requested.
// Required parameter.
func (x *PrmBalanceGet) SetAccount(id user.ID) {
x.account = id
x.accountSet = true
}

// BalanceGet requests current balance of the NeoFS account.
Expand All @@ -48,7 +46,7 @@ func (c *Client) BalanceGet(ctx context.Context, prm PrmBalanceGet) (accounting.
}()

switch {
case !prm.accountSet:
case prm.account.IsZero():
err = ErrMissingAccount
return accounting.Decimal{}, err
}
Expand Down
5 changes: 2 additions & 3 deletions client/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ type PrmContainerGet struct {
prmCommonMeta
}

// ContainerGet reads NeoFS container by ID.
// ContainerGet reads NeoFS container by ID. The ID must not be zero.
//
// Any errors (local or remote, including returned status codes) are returned as Go errors,
// see [apistatus] package for NeoFS-specific error types.
Expand Down Expand Up @@ -509,8 +509,7 @@ func (c *Client) ContainerSetEACL(ctx context.Context, table eacl.Table, signer
return ErrMissingSigner
}

_, isCIDSet := table.CID()
if !isCIDSet {
if table.LimitedContainer().IsZero() {
err = ErrMissingEACLContainer
return err
}
Expand Down
6 changes: 3 additions & 3 deletions client/container_statistic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func prepareContainer(accountID user.ID) container.Container {

func testEaclTable(containerID cid.ID) eacl.Table {
var table eacl.Table
table.SetCID(containerID)
table.LimitToContainer(containerID)

r := eacl.NewRecord()
r.SetOperation(eacl.OperationPut)
Expand Down Expand Up @@ -362,7 +362,7 @@ func TestClientStatistic_ContainerSetEacl(t *testing.T) {
c.prm.statisticCallback = collector.Collect

var prm PrmContainerSetEACL
table := testEaclTable(cid.ID{})
table := testEaclTable(cidtest.ID())
err := c.ContainerSetEACL(ctx, table, usr, prm)
require.NoError(t, err)

Expand Down Expand Up @@ -587,7 +587,7 @@ func TestClientStatistic_ObjectPut(t *testing.T) {

var hdr object.Object
hdr.SetOwnerID(&usr.ID)
hdr.SetContainerID(containerID)
hdr.SetContainer(containerID)

writer, err := c.ObjectPutInit(ctx, hdr, usr, prm)
require.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions client/object_replicate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ func (x *testReplicationServer) Replicate(_ context.Context, req *objectgrpc.Rep
return &resp, nil
}

id, ok := obj.ID()
if !ok {
id := obj.GetID()
if id.IsZero() {
st.Code = 1024 // internal error
st.Message = "missing object ID"
resp.Status = &st
Expand Down
23 changes: 21 additions & 2 deletions container/id/id.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cid

import (
"crypto/sha256"
"errors"
"fmt"

"github.com/mr-tron/base58"
Expand All @@ -11,14 +12,18 @@ import (
// Size is the size of an [ID] in bytes.
const Size = sha256.Size

// ID represents NeoFS container identifier.
// ID represents NeoFS container identifier. Zero ID is usually prohibited, see
// docs for details.
//
// ID implements built-in comparable interface.
//
// ID is mutually compatible with github.com/nspcc-dev/neofs-api-go/v2/refs.ContainerID
// message. See ReadFromV2 / WriteToV2 methods.
type ID [Size]byte

// ErrZero is an error returned on zero [ID] encounter.
var ErrZero = errors.New("zero container ID")

// NewFromMarshalledContainer returns new ID calculated from the given NeoFS
// container encoded into Protocol Buffers V3 with ascending order of fields by
// number. It's callers responsibility to ensure the format of b. See
Expand All @@ -43,7 +48,11 @@ func DecodeString(s string) (ID, error) {
//
// See also WriteToV2.
func (id *ID) ReadFromV2(m refs.ContainerID) error {
return id.Decode(m.GetValue())
err := id.Decode(m.GetValue())
if err == nil && id.IsZero() {
err = ErrZero
}
return err
}

// WriteToV2 writes ID to the refs.ContainerID message.
Expand Down Expand Up @@ -139,3 +148,13 @@ func (id ID) String() string {
// See also [container.Container.CalculateID], [container.Container.AssertID].
// Deprecated: use [NewFromContainerBinary].
func (id *ID) FromBinary(cnr []byte) { *id = NewFromMarshalledContainer(cnr) }

// IsZero checks whether ID is zero.
func (id ID) IsZero() bool {
for i := range id {
if id[i] != 0 {
return false
}
}
return true
}
20 changes: 17 additions & 3 deletions container/id/id_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ var validBytes = [cid.Size]byte{231, 189, 121, 7, 173, 134, 254, 165, 63, 186, 6
// corresponds to validBytes.
const validString = "GbckSBPEdM2P41Gkb9cVapFYb5HmRPDTZZp9JExGnsCF"

var invalidValueTestcases = []struct {
type invalidValueTestCase struct {
name string
err string
val []byte
}{
}

var invalidValueTestcases = []invalidValueTestCase{
{name: "nil value", err: "invalid length 0", val: nil},
{name: "empty value", err: "invalid length 0", val: []byte{}},
{name: "undersized value", err: "invalid length 31", val: make([]byte, 31)},
Expand All @@ -37,7 +39,9 @@ func TestID_ReadFromV2(t *testing.T) {
require.EqualValues(t, validBytes, id)

t.Run("invalid", func(t *testing.T) {
for _, tc := range invalidValueTestcases {
for _, tc := range append(invalidValueTestcases, invalidValueTestCase{
name: "zero value", err: "zero container ID", val: make([]byte, cid.Size),
}) {
t.Run(tc.name, func(t *testing.T) {
var m refs.ContainerID
m.SetValue(tc.val)
Expand Down Expand Up @@ -166,3 +170,13 @@ func TestID_String(t *testing.T) {
require.Equal(t, id.String(), id.String())
require.NotEqual(t, id.String(), cidtest.OtherID(id).String())
}

func TestID_IsZero(t *testing.T) {
var id cid.ID
require.True(t, id.IsZero())
for i := 0; i < cid.Size; i++ {
var id2 cid.ID
id2[i]++
require.False(t, id2.IsZero())
}
}
7 changes: 2 additions & 5 deletions container/size_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
v2container "github.com/nspcc-dev/neofs-api-go/v2/container"
"github.com/nspcc-dev/neofs-api-go/v2/refs"
"github.com/nspcc-dev/neofs-sdk-go/container"
cid "github.com/nspcc-dev/neofs-sdk-go/container/id"
cidtest "github.com/nspcc-dev/neofs-sdk-go/container/id/test"
"github.com/stretchr/testify/require"
)
Expand Down Expand Up @@ -77,10 +76,8 @@ func TestSizeEstimation_ReadFromV2(t *testing.T) {

require.Error(t, val.ReadFromV2(msg))

cnrMsg.SetValue(make([]byte, cid.Size))

var cnr cid.ID
require.NoError(t, cnr.ReadFromV2(cnrMsg))
cnr := cidtest.ID()
cnrMsg.SetValue(cnr[:])

msg.SetEpoch(epoch)
msg.SetUsedSpace(value)
Expand Down
Loading

0 comments on commit 70e062d

Please sign in to comment.