From cce818b802fcd1e320fb3aa5eddf306067ead207 Mon Sep 17 00:00:00 2001 From: Andreas Linde Date: Tue, 24 Feb 2026 18:51:30 +0100 Subject: [PATCH] Reject partial writes when feature does not announce partial write support SPINE spec Section 5.3.4.2 defines write cmdOptions combinations but provides no fallback for unsupported partial writes (unlike reads where Section 5.3.4.5 allows ignoring filters and returning full data). Validates incoming writes with partial/delete filters against the feature's announced writePartial capability and returns error code 8 (RestrictedFunctionExchangeCombinationNotSupported) when not supported. --- spine/device_local.go | 14 ++ spine/device_local_partial_write_test.go | 238 +++++++++++++++++++++++ 2 files changed, 252 insertions(+) create mode 100644 spine/device_local_partial_write_test.go diff --git a/spine/device_local.go b/spine/device_local.go index d401e3f..1a42e98 100644 --- a/spine/device_local.go +++ b/spine/device_local.go @@ -437,6 +437,20 @@ func (r *DeviceLocal) ProcessCmd(datagram model.DatagramType, remoteDevice api.D _ = remoteFeature.Device().Sender().ResultError(message.RequestHeader, localFeature.Address(), err) return errors.New(err.String()) } + + // If the write contains a partial or delete filter, the feature must have announced + // partial write support via possibleOperations. Unlike reads (where the spec allows + // ignoring unsupported filters and returning full data), writes have no such fallback. + if filterPartial != nil || filterDelete != nil { + operations := localFeature.Operations()[*cmdData.Function] + if !operations.WritePartial() { + err := model.NewErrorType( + model.ErrorNumberTypeRestrictedFunctionExchangeCombinationNotSupported, + "partial write not supported for this function") + _ = remoteFeature.Device().Sender().ResultError(message.RequestHeader, localFeature.Address(), err) + return errors.New(err.String()) + } + } } err := localFeature.HandleMessage(message) diff --git a/spine/device_local_partial_write_test.go b/spine/device_local_partial_write_test.go new file mode 100644 index 0000000..184034e --- /dev/null +++ b/spine/device_local_partial_write_test.go @@ -0,0 +1,238 @@ +package spine + +import ( + "testing" + "time" + + shipapi "github.com/enbility/ship-go/api" + "github.com/enbility/spine-go/model" + "github.com/enbility/spine-go/util" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" +) + +func TestDeviceLocalPartialWriteSuite(t *testing.T) { + suite.Run(t, new(DeviceLocalPartialWriteSuite)) +} + +type DeviceLocalPartialWriteSuite struct { + suite.Suite + + sut *DeviceLocal + localFeature *FeatureLocal + remoteDevice *DeviceRemote + remoteFeature *FeatureRemote + lastMessage string +} + +var _ shipapi.ShipConnectionDataWriterInterface = (*DeviceLocalPartialWriteSuite)(nil) + +func (s *DeviceLocalPartialWriteSuite) WriteShipMessageWithPayload(msg []byte) { + s.lastMessage = string(msg) +} + +func (s *DeviceLocalPartialWriteSuite) SetupTest() { + s.sut = NewDeviceLocal("brand", "model", "serial", "code", "address", + model.DeviceTypeTypeEnergyManagementSystem, model.NetworkManagementFeatureSetTypeSmart) + + localEntity := NewEntityLocal(s.sut, model.EntityTypeTypeCEM, + NewAddressEntityType([]uint{1}), time.Second*4) + s.sut.AddEntity(localEntity) + + // Create a server feature with write support but NO partial write support. + // DeviceClassificationManufacturerData does not implement Updater, + // so writePartial will be false. + s.localFeature = NewFeatureLocal(1, localEntity, + model.FeatureTypeTypeDeviceClassification, model.RoleTypeServer) + s.localFeature.AddFunctionType(model.FunctionTypeDeviceClassificationManufacturerData, true, true) + localEntity.AddFeature(s.localFeature) + + ski := "test" + _ = s.sut.SetupRemoteDevice(ski, s) + remote := s.sut.RemoteDeviceForSki(ski) + s.remoteDevice = remote.(*DeviceRemote) + s.remoteDevice.address = util.Ptr(model.AddressDeviceType("remoteDevice")) + + remoteEntity := NewEntityRemote(s.remoteDevice, model.EntityTypeTypeCEM, + []model.AddressEntityType{1}) + s.remoteFeature = NewFeatureRemote(1, remoteEntity, + model.FeatureTypeTypeDeviceClassification, model.RoleTypeClient) + remoteEntity.AddFeature(s.remoteFeature) + s.remoteDevice.AddEntity(remoteEntity) + + // Add a binding so write permission checks pass + binding := model.BindingManagementRequestCallType{ + ClientAddress: s.remoteFeature.Address(), + ServerAddress: s.localFeature.Address(), + ServerFeatureType: util.Ptr(model.FeatureTypeTypeDeviceClassification), + } + err := s.sut.BindingManager().AddBinding(s.remoteDevice, binding) + assert.Nil(s.T(), err) +} + +// Test that a partial write to a feature that does not announce partial write +// is rejected with error code 8 (RestrictedFunctionExchangeCombinationNotSupported) +func (s *DeviceLocalPartialWriteSuite) Test_PartialWriteDenied_WhenNotSupported() { + // Verify precondition: the function supports write but NOT writePartial + ops := s.localFeature.Operations() + fnOps, ok := ops[model.FunctionTypeDeviceClassificationManufacturerData] + assert.True(s.T(), ok) + assert.True(s.T(), fnOps.Write()) + assert.False(s.T(), fnOps.WritePartial(), "precondition: writePartial must be false") + + // Build a write datagram with a partial filter + datagram := model.DatagramType{ + Header: model.HeaderType{ + AddressSource: s.remoteFeature.Address(), + AddressDestination: s.localFeature.Address(), + MsgCounter: util.Ptr(model.MsgCounterType(1)), + CmdClassifier: util.Ptr(model.CmdClassifierTypeWrite), + }, + Payload: model.PayloadType{ + Cmd: []model.CmdType{ + { + Function: util.Ptr(model.FunctionTypeDeviceClassificationManufacturerData), + Filter: []model.FilterType{ + { + CmdControl: &model.CmdControlType{ + Partial: &model.ElementTagType{}, + }, + }, + }, + DeviceClassificationManufacturerData: &model.DeviceClassificationManufacturerDataType{ + BrandName: util.Ptr(model.DeviceClassificationStringType("TestBrand")), + }, + }, + }, + }, + } + + err := s.sut.ProcessCmd(datagram, s.remoteDevice) + if assert.Error(s.T(), err) { + assert.Contains(s.T(), err.Error(), "partial write not supported") + } +} + +// Test that a write with a delete filter to a feature that does not announce +// partial write is also rejected +func (s *DeviceLocalPartialWriteSuite) Test_DeleteFilterDenied_WhenPartialNotSupported() { + ops := s.localFeature.Operations() + fnOps, _ := ops[model.FunctionTypeDeviceClassificationManufacturerData] + assert.False(s.T(), fnOps.WritePartial(), "precondition: writePartial must be false") + + datagram := model.DatagramType{ + Header: model.HeaderType{ + AddressSource: s.remoteFeature.Address(), + AddressDestination: s.localFeature.Address(), + MsgCounter: util.Ptr(model.MsgCounterType(1)), + CmdClassifier: util.Ptr(model.CmdClassifierTypeWrite), + }, + Payload: model.PayloadType{ + Cmd: []model.CmdType{ + { + Function: util.Ptr(model.FunctionTypeDeviceClassificationManufacturerData), + Filter: []model.FilterType{ + { + CmdControl: &model.CmdControlType{ + Delete: &model.ElementTagType{}, + }, + }, + }, + DeviceClassificationManufacturerData: &model.DeviceClassificationManufacturerDataType{}, + }, + }, + }, + } + + err := s.sut.ProcessCmd(datagram, s.remoteDevice) + if assert.Error(s.T(), err) { + assert.Contains(s.T(), err.Error(), "partial write not supported") + } +} + +// Test that a full write (no filter) to a feature that does not support +// partial write is still accepted +func (s *DeviceLocalPartialWriteSuite) Test_FullWriteAllowed_WhenPartialNotSupported() { + datagram := model.DatagramType{ + Header: model.HeaderType{ + AddressSource: s.remoteFeature.Address(), + AddressDestination: s.localFeature.Address(), + MsgCounter: util.Ptr(model.MsgCounterType(1)), + CmdClassifier: util.Ptr(model.CmdClassifierTypeWrite), + }, + Payload: model.PayloadType{ + Cmd: []model.CmdType{ + { + DeviceClassificationManufacturerData: &model.DeviceClassificationManufacturerDataType{ + BrandName: util.Ptr(model.DeviceClassificationStringType("TestBrand")), + }, + }, + }, + }, + } + + err := s.sut.ProcessCmd(datagram, s.remoteDevice) + assert.NoError(s.T(), err) +} + +// Test that a partial write to a feature that DOES support partial write +// is accepted +func (s *DeviceLocalPartialWriteSuite) Test_PartialWriteAllowed_WhenSupported() { + // Set up a LoadControl server feature which supports partial write + // (LoadControlLimitListDataType implements Updater) + localEntity := s.sut.Entity([]model.AddressEntityType{1}) + + lcFeature := NewFeatureLocal(2, localEntity.(*EntityLocal), + model.FeatureTypeTypeLoadControl, model.RoleTypeServer) + lcFeature.AddFunctionType(model.FunctionTypeLoadControlLimitListData, true, true) + localEntity.AddFeature(lcFeature) + + // Verify precondition: writePartial is true for this function + ops := lcFeature.Operations() + fnOps, ok := ops[model.FunctionTypeLoadControlLimitListData] + assert.True(s.T(), ok) + assert.True(s.T(), fnOps.WritePartial(), "precondition: writePartial must be true") + + // Add a remote LoadControl client feature so binding type matches + remoteEntity := s.remoteDevice.Entity([]model.AddressEntityType{1}) + remoteLcFeature := NewFeatureRemote(2, remoteEntity.(*EntityRemote), + model.FeatureTypeTypeLoadControl, model.RoleTypeClient) + remoteEntity.AddFeature(remoteLcFeature) + + // Add binding for the new feature pair + binding := model.BindingManagementRequestCallType{ + ClientAddress: remoteLcFeature.Address(), + ServerAddress: lcFeature.Address(), + ServerFeatureType: util.Ptr(model.FeatureTypeTypeLoadControl), + } + err := s.sut.BindingManager().AddBinding(s.remoteDevice, binding) + assert.Nil(s.T(), err) + + datagram := model.DatagramType{ + Header: model.HeaderType{ + AddressSource: remoteLcFeature.Address(), + AddressDestination: lcFeature.Address(), + MsgCounter: util.Ptr(model.MsgCounterType(2)), + CmdClassifier: util.Ptr(model.CmdClassifierTypeWrite), + }, + Payload: model.PayloadType{ + Cmd: []model.CmdType{ + { + Function: util.Ptr(model.FunctionTypeLoadControlLimitListData), + Filter: filterEmptyPartial(), + LoadControlLimitListData: &model.LoadControlLimitListDataType{ + LoadControlLimitData: []model.LoadControlLimitDataType{ + { + LimitId: util.Ptr(model.LoadControlLimitIdType(1)), + IsLimitActive: util.Ptr(true), + }, + }, + }, + }, + }, + }, + } + + err = s.sut.ProcessCmd(datagram, s.remoteDevice) + assert.NoError(s.T(), err) +}