TrueCloudLab/frostfs-node
20
1
Fork 27
Code Issues 96 Pull requests 4 Projects Releases 1 Activity Actions

object: Validate attribute EXPIRATION_EPOCH on put #1188

Merged
fyrchik merged 1 commit from acid-ant/frostfs-node:bugfix/1109-validate-exp-epoch into master 2024-06-19 17:19:32 +00:00
Conversation 0 Commits 1 Files changed 2 +28 -30
2 changed files with 28 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
pkg/core/object/fmt.go
View file

@ -124,15 +124,22 @@ func (v *FormatValidator) Validate(ctx context.Context, obj *objectSDK.Object, u
return fmt.Errorf("invalid attributes: %w", err)
}
exp, err := expirationEpochAttribute(obj)
dstepanov-yadro marked this conversation as resolved Outdated
dstepanov-yadro commented 2024-06-18 12:30:09 +00:00
Outdated
Review
Copy link

Also it could happen, that user has defined both of attributes: objectV2.SysAttributeExpEpoch and objectV2.SysAttributeExpEpochNeoFS. Maybe it has to be an error too?

Also it could happen, that user has defined both of attributes: `objectV2.SysAttributeExpEpoch` and `objectV2.SysAttributeExpEpochNeoFS`. Maybe it has to be an error too?
fyrchik commented 2024-06-18 12:48:31 +00:00
Outdated
Review
Copy link

It definitely should be, but we will eventually remove NeoFS attribute, and currently we have deterministic behaviour, so I would leave it as is.

It definitely should be, but we will eventually remove NeoFS attribute, and currently we have deterministic behaviour, so I would leave it as is.
if err != nil {
if !errors.Is(err, errNoExpirationEpoch) {
return fmt.Errorf("object did not pass expiration check: %w", err)
}
} else if !unprepared && exp < v.netState.CurrentEpoch() {
fyrchik marked this conversation as resolved Outdated
fyrchik commented 2024-06-18 12:23:55 +00:00
Outdated
Review
Copy link

else if !unprepared?

Also, checkExpiration now contains a single if, we could move it here and rename checkExpiration to checkIfExpired. And checking whether expiration epoch is less than the current epoch is valid for both types of objects, the only check that is specific is IsLocked (because not-yet-created object cannot be locked).

`else if !unprepared`? Also, `checkExpiration` now contains a single `if`, we could move it here and rename `checkExpiration` to `checkIfExpired`. And checking whether `expiration epoch` is less than the current epoch is valid for both types of objects, the only check that is specific is `IsLocked` (because not-yet-created object cannot be locked).
acid-ant commented 2024-06-18 14:35:43 +00:00
Outdated
Review
Copy link

OMG, updated. Thanks.

OMG, updated. Thanks.
if err := v.checkIfExpired(ctx, obj); err != nil {
return fmt.Errorf("object did not pass expiration check: %w", err)
}
}
if !unprepared {
if err := v.validateSignatureKey(obj); err != nil {
return fmt.Errorf("(%T) could not validate signature key: %w", v, err)
}
if err := v.checkExpiration(ctx, obj); err != nil {
return fmt.Errorf("object did not pass expiration check: %w", err)
}
if err := objectSDK.CheckHeaderVerificationFields(obj); err != nil {
return fmt.Errorf("(%T) could not validate header fields: %w", v, err)
}
@ -348,17 +355,7 @@ func (v *FormatValidator) fillAndValidateTombstoneMeta(o *objectSDK.Object, meta
var errExpired = errors.New("object has expired")
func (v *FormatValidator) checkExpiration(ctx context.Context, obj *objectSDK.Object) error {
exp, err := expirationEpochAttribute(obj)
if err != nil {
if errors.Is(err, errNoExpirationEpoch) {
return nil // objects without expiration attribute are valid
}
return err
}
if exp < v.netState.CurrentEpoch() {
func (v *FormatValidator) checkIfExpired(ctx context.Context, obj *objectSDK.Object) error {
// an object could be expired but locked;
// put such an object is a correct operation
@ -377,7 +374,6 @@ func (v *FormatValidator) checkExpiration(ctx context.Context, obj *objectSDK.Ob
if !locked {
return errExpired
}
}
return nil
}

2
pkg/core/object/fmt_test.go
View file

@ -195,6 +195,8 @@ func TestFormatValidator_Validate(t *testing.T) {
val := "text"
err := v.Validate(context.Background(), fn(val), false)
require.Error(t, err)
err = v.Validate(context.Background(), fn(val), true)
require.Error(t, err)
})
t.Run("expired object", func(t *testing.T) {