fix: validate cached property access
This commit is contained in:
@@ -498,7 +498,24 @@ type cacheMethod struct {
|
|||||||
|
|
||||||
type propertyCache struct {
|
type propertyCache struct {
|
||||||
PropertySpec
|
PropertySpec
|
||||||
Method cacheMethod `json:"method"`
|
Method cacheMethod `json:"method"`
|
||||||
|
rwPresent bool
|
||||||
|
}
|
||||||
|
|
||||||
|
func (cache *propertyCache) UnmarshalJSON(data []byte) error {
|
||||||
|
type propertyCacheAlias propertyCache
|
||||||
|
decoded := struct {
|
||||||
|
*propertyCacheAlias
|
||||||
|
RW *string `json:"rw"`
|
||||||
|
}{propertyCacheAlias: (*propertyCacheAlias)(cache)}
|
||||||
|
if err := json.Unmarshal(data, &decoded); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
cache.rwPresent = decoded.RW != nil
|
||||||
|
if decoded.RW != nil {
|
||||||
|
cache.RW = *decoded.RW
|
||||||
|
}
|
||||||
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
type actionCache struct {
|
type actionCache struct {
|
||||||
@@ -536,6 +553,9 @@ func decodeDeviceInfo(data []byte, model string) (DeviceInfo, error) {
|
|||||||
|
|
||||||
info := DeviceInfo{Name: cache.Name, Model: cache.Model}
|
info := DeviceInfo{Name: cache.Name, Model: cache.Model}
|
||||||
for _, cachedProperty := range cache.Properties {
|
for _, cachedProperty := range cache.Properties {
|
||||||
|
if !cachedProperty.rwPresent {
|
||||||
|
return DeviceInfo{}, fmt.Errorf("property %q is missing access metadata", cachedProperty.Name)
|
||||||
|
}
|
||||||
property := cachedProperty.PropertySpec
|
property := cachedProperty.PropertySpec
|
||||||
if property.SIID == 0 {
|
if property.SIID == 0 {
|
||||||
property.SIID = cachedProperty.Method.SIID
|
property.SIID = cachedProperty.Method.SIID
|
||||||
@@ -572,7 +592,7 @@ func validateDeviceInfo(info *DeviceInfo, model string) error {
|
|||||||
properties := make(map[propertyID]PropertySpec, len(info.Properties))
|
properties := make(map[propertyID]PropertySpec, len(info.Properties))
|
||||||
for index, property := range info.Properties {
|
for index, property := range info.Properties {
|
||||||
if strings.TrimSpace(property.Name) == "" || !validPropertyType(property.Type) ||
|
if strings.TrimSpace(property.Name) == "" || !validPropertyType(property.Type) ||
|
||||||
(property.RW != "r" && property.RW != "w" && property.RW != "rw") || property.SIID <= 0 || property.PIID <= 0 {
|
(property.RW != "" && property.RW != "r" && property.RW != "w" && property.RW != "rw") || property.SIID <= 0 || property.PIID <= 0 {
|
||||||
return fmt.Errorf("property %d is invalid", index)
|
return fmt.Errorf("property %d is invalid", index)
|
||||||
}
|
}
|
||||||
properties[propertyID{siid: property.SIID, piid: property.PIID}] = property
|
properties[propertyID{siid: property.SIID, piid: property.PIID}] = property
|
||||||
|
|||||||
+31
-1
@@ -282,6 +282,35 @@ func TestVersion2DeviceInfoCacheTrustsActionWithoutInputs(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestVersion2DeviceInfoCacheRequiresPropertyAccessField(t *testing.T) {
|
||||||
|
for _, test := range []struct {
|
||||||
|
name string
|
||||||
|
property string
|
||||||
|
wantCalls int
|
||||||
|
}{
|
||||||
|
{name: "missing", property: `{"name":"power","type":"bool","siid":2,"piid":1}`, wantCalls: 1},
|
||||||
|
{name: "explicit empty", property: `{"name":"power","type":"bool","rw":"","siid":2,"piid":1}`},
|
||||||
|
} {
|
||||||
|
t.Run(test.name, func(t *testing.T) {
|
||||||
|
fixture := loadSpecFixture(t)
|
||||||
|
if test.wantCalls == 0 {
|
||||||
|
fixture = nil
|
||||||
|
}
|
||||||
|
testServer := newDeviceTestServer(t, fixture, nil)
|
||||||
|
cacheDir := t.TempDir()
|
||||||
|
cache := fmt.Sprintf(`{"version":2,"model":"test.light.v1","properties":[%s],"actions":[]}`, test.property)
|
||||||
|
if err := os.WriteFile(filepath.Join(cacheDir, "test.light.v1.json"), []byte(cache), 0o600); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
info, err := GetDeviceInfo(context.Background(), testServer.server.Client(), "test.light.v1", cacheDir)
|
||||||
|
if err != nil || testServer.specCalls != test.wantCalls || len(info.Properties) == 0 {
|
||||||
|
t.Fatalf("GetDeviceInfo() = %#v, %v, calls=%d, want calls=%d", info, err, testServer.specCalls, test.wantCalls)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestStaleDeviceInfoCacheRefreshFailureIncludesBothErrors(t *testing.T) {
|
func TestStaleDeviceInfoCacheRefreshFailureIncludesBothErrors(t *testing.T) {
|
||||||
testServer := newDeviceTestServer(t, []byte("unavailable"), nil)
|
testServer := newDeviceTestServer(t, []byte("unavailable"), nil)
|
||||||
testServer.status = http.StatusServiceUnavailable
|
testServer.status = http.StatusServiceUnavailable
|
||||||
@@ -678,7 +707,8 @@ func TestDeviceGetManyReturnsTransportError(t *testing.T) {
|
|||||||
}
|
}
|
||||||
func TestDeviceGetManyValidatesBeforeNetwork(t *testing.T) {
|
func TestDeviceGetManyValidatesBeforeNetwork(t *testing.T) {
|
||||||
device, testServer := fixtureDeviceWithServer(t, nil)
|
device, testServer := fixtureDeviceWithServer(t, nil)
|
||||||
tests := [][]string{{"power", "power"}, {"power", "missing"}, {"power", "write-only"}}
|
device.properties["no-access"] = PropertySpec{Name: "no-access", Type: "bool", SIID: 2, PIID: 4}
|
||||||
|
tests := [][]string{{"power", "power"}, {"power", "missing"}, {"power", "write-only"}, {"power", "no-access"}}
|
||||||
for _, names := range tests {
|
for _, names := range tests {
|
||||||
if results, err := device.GetMany(context.Background(), names); err == nil || results != nil {
|
if results, err := device.GetMany(context.Background(), names); err == nil || results != nil {
|
||||||
t.Fatalf("GetMany(%v) = %#v, %v", names, results, err)
|
t.Fatalf("GetMany(%v) = %#v, %v", names, results, err)
|
||||||
|
|||||||
Reference in New Issue
Block a user