From 93fd0322e5a4d80daad02c0e92824b28d670e034 Mon Sep 17 00:00:00 2001 From: Matthias Hochgatterer Date: Mon, 28 Feb 2022 12:32:12 +0100 Subject: [PATCH] Allow the update of a characteristic value to fail brutella/hc#163 --- README.md | 7 ++- characteristic/bool.go | 7 ++- characteristic/bytes.go | 7 ++- characteristic/c.go | 79 ++++++++++++++++---------- characteristic/c_test.go | 26 ++++++++- characteristic/float.go | 7 ++- characteristic/int.go | 7 ++- characteristic/string.go | 7 ++- characteristics.go | 28 +++++++-- identify.go | 1 - json.go | 1 + server_test.go | 120 +++++++++++++++++++++++++++++++++++++++ 12 files changed, 251 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index af93ef1..1f917f7 100644 --- a/README.md +++ b/README.md @@ -36,9 +36,10 @@ If you want to support `hap`, please purchase Home from the [App Store][home-app This library is a rewrite of [hc](https://github.com/brutella/hc). If you want to migrate from `hc`, consider the following changes. -- Instead of `hc.NewIPTransport(...)` you now call `hap.NewServer(...)` to create a server. -- You can provide your own persistent store by implementing the [Store](store.go) interface. -- You can define your own http handlers. [hc#212](https://github.com/brutella/hc/issues/212) +- Instead of `hc.NewIPTransport(...)` you now call [hap.NewServer(...)](https://pkg.go.dev/github.com/brutella/hap#NewServer) to create a server. +- You can create your own persistent storage by implementing the [Store](store.go) interface. +- Setting the value of a characteristic can now fail. Fixes [hc#163](https://github.com/brutella/hc/issues/163) +- You can define custom http handlers. Fixes [hc#212](https://github.com/brutella/hc/issues/212) ```go server.ServeMux().HandleFunc("/ping", func(res http.ResponseWriter, req *http.Request) { res.Write([]byte("pong")) diff --git a/characteristic/bool.go b/characteristic/bool.go index c971532..df1e11d 100644 --- a/characteristic/bool.go +++ b/characteristic/bool.go @@ -23,7 +23,12 @@ func (c *Bool) SetValue(v bool) { // Value returns the value of c as bool. func (c *Bool) Value() bool { - return c.C.value(nil).(bool) + v, _ := c.C.valueRequest(nil) + if v == nil { + return false + } + + return v.(bool) } // OnValueRemoteUpdate calls fn when the value of the characteristic was updated. diff --git a/characteristic/bytes.go b/characteristic/bytes.go index 7093232..6520ab7 100644 --- a/characteristic/bytes.go +++ b/characteristic/bytes.go @@ -23,7 +23,12 @@ func (c *Bytes) SetValue(v []byte) { // Value returns the value of c as byte array. func (c *Bytes) Value() []byte { - str := c.C.value(nil).(string) + v, _ := c.C.valueRequest(nil) + if v == nil { + return []byte{} + } + + str := v.(string) if b, err := base64.StdEncoding.DecodeString(str); err != nil { return []byte{} } else { diff --git a/characteristic/c.go b/characteristic/c.go index 2d8421d..fd04308 100644 --- a/characteristic/c.go +++ b/characteristic/c.go @@ -79,10 +79,20 @@ type C struct { // Stores which connected client has events enabled for this characteristic. Events map[string]bool - // ValFunc returns the value of C. - // If no nil, the return value of this function is used instead of Val. - // req is non nil, if the value is requested from a request. - ValFunc func(req *http.Request) interface{} + // ValueRequestFunc is called when the value of C is requested. + // The http request is non-nil, if the value of C is requested. + // by an HTTP request coming from a paired controller. + // The first return value of this function is the value of C. + // If the second argument is non-zero, the server responds with the + // HTTP status code 500 Internal Server Error. + ValueRequestFunc func(request *http.Request) (interface{}, int) + + // SetValueRequestFunc is called when the value of C is updated. + // The first argument "value" is the new value of C. + // The second argument "request" is non-nil, if the value of C is + // updated from an HTTP request coming from a paired controller. + // An error is inidcated if the return value is non-zero. + SetValueRequestFunc func(value interface{}, request *http.Request) int // A list of update value functions. // There are called when the value of the characteristic is updated. @@ -110,14 +120,23 @@ func (c *C) OnCValueUpdate(fn ValueUpdateFunc) { // Sets the value of c to v. // The function is called if the value is updated from an http request. -func (c *C) SetValueRequest(v interface{}, req *http.Request) { +func (c *C) SetValueRequest(v interface{}, req *http.Request) int { // check write permission if !c.IsWritable() { log.Info.Printf("writing %v by %s not allowed\n", v, req.RemoteAddr) - return + return -70404 + } + + if c.SetValueRequestFunc != nil { + if s := c.SetValueRequestFunc(v, req); s != 0 { + return s + } } c.setValue(v, req) + + // no error + return 0 } func (c *C) setValue(v interface{}, req *http.Request) { @@ -148,25 +167,25 @@ func (c *C) setValue(v interface{}, req *http.Request) { } } -// ValueRequest returns the value of the characteristic for -// a http request. -func (c *C) ValueRequest(req *http.Request) interface{} { +// ValueRequest returns the value of C and a status code. +// if the characteristic is not readable, the status code -70405 is returned. +func (c *C) ValueRequest(req *http.Request) (interface{}, int) { // check write permission if !c.IsReadable() { log.Info.Printf("reading %d by %s not allowed\n", c.Id, req.RemoteAddr) - return nil + return nil, -70405 } - return c.value(req) + return c.valueRequest(req) } -// value returns the value of the characteristic. -func (c *C) value(req *http.Request) interface{} { - if c.ValFunc != nil { - return c.ValFunc(req) +// value returns the value of C and a status code. +func (c *C) valueRequest(req *http.Request) (interface{}, int) { + if c.ValueRequestFunc != nil { + return c.ValueRequestFunc(req) } - return c.Val + return c.Val, 0 } func (c *C) IsWritable() bool { @@ -203,7 +222,7 @@ func (c *C) IsWriteOnly() bool { return len(c.Permissions) == 1 && c.Permissions[0] == PermissionWrite } -func (ch *C) MarshalJSON() ([]byte, error) { +func (c *C) MarshalJSON() ([]byte, error) { d := struct { Id uint64 `json:"iid"` // managed by accessory Type string `json:"type"` @@ -219,20 +238,22 @@ func (ch *C) MarshalJSON() ([]byte, error) { MinValue interface{} `json:"minValue,omitempty"` StepValue interface{} `json:"minStep,omitempty"` }{ - Id: ch.Id, - Type: ch.Type, - Permissions: ch.Permissions, - Description: ch.Description, - Format: ch.Format, - Unit: ch.Unit, - MaxLen: ch.MaxLen, - MaxValue: ch.MaxVal, - MinValue: ch.MinVal, - StepValue: ch.StepVal, + Id: c.Id, + Type: c.Type, + Permissions: c.Permissions, + Description: c.Description, + Format: c.Format, + Unit: c.Unit, + MaxLen: c.MaxLen, + MaxValue: c.MaxVal, + MinValue: c.MinVal, + StepValue: c.StepVal, } - if ch.IsReadable() { - d.Value = ch.value(nil) + if c.IsReadable() { + if v, _ := c.valueRequest(nil); v != nil { + d.Value = v + } } return json.Marshal(&d) diff --git a/characteristic/c_test.go b/characteristic/c_test.go index 8f1318b..99299b2 100644 --- a/characteristic/c_test.go +++ b/characteristic/c_test.go @@ -11,9 +11,9 @@ func TestCharacteristicValue(t *testing.T) { c.Val = 0 n := 0 - c.ValFunc = func(*http.Request) interface{} { + c.ValueRequestFunc = func(*http.Request) (interface{}, int) { n++ - return n + return n, 0 } if is, want := c.Value(), 1; is != want { @@ -139,3 +139,25 @@ func TestReadOnly(t *testing.T) { t.Fatalf("is=%v want=%v", is, want) } } + +func TestSetValueRequestFunc(t *testing.T) { + c := NewBrightness() + + c.SetValue(100) + c.SetValueRequestFunc = func(v interface{}, r *http.Request) int { + if r != nil { + return -70408 + } + + return 0 + } + + s := c.SetValueRequest(50, &http.Request{}) + if is, want := s, -70408; is != want { + t.Fatalf("%v != %v", is, want) + } + + if is, want := c.Value(), 100; is != want { + t.Fatalf("is=%v want=%v", is, want) + } +} diff --git a/characteristic/float.go b/characteristic/float.go index cbd0673..e062007 100644 --- a/characteristic/float.go +++ b/characteristic/float.go @@ -33,7 +33,12 @@ func (c *Float) SetStepValue(v float64) { // Value returns the value of c as float64. func (c *Float) Value() float64 { - return c.C.value(nil).(float64) + v, _ := c.C.valueRequest(nil) + if v == nil { + return 0 + } + + return v.(float64) } func (c *Float) MinValue() float64 { diff --git a/characteristic/int.go b/characteristic/int.go index 8e30603..67a2641 100644 --- a/characteristic/int.go +++ b/characteristic/int.go @@ -33,7 +33,12 @@ func (c *Int) SetStepValue(v int) { // Value returns the value of c as integer. func (c *Int) Value() int { - return c.C.value(nil).(int) + v, _ := c.C.valueRequest(nil) + if v == nil { + return 0 + } + + return v.(int) } func (c *Int) MinValue() int { diff --git a/characteristic/string.go b/characteristic/string.go index 54b72c4..163b1ad 100644 --- a/characteristic/string.go +++ b/characteristic/string.go @@ -23,7 +23,12 @@ func (c *String) SetValue(v string) { // Value returns the value of c as string. func (c *String) Value() string { - return c.C.value(nil).(string) + v, _ := c.C.valueRequest(nil) + if v == nil { + return "" + } + + return v.(string) } // OnValueRemoteUpdate calls fn when the value of the characteristic was updated. diff --git a/characteristics.go b/characteristics.go index 800aa99..c9b12dd 100644 --- a/characteristics.go +++ b/characteristics.go @@ -72,7 +72,13 @@ func (srv *Server) getCharacteristics(res http.ResponseWriter, req *http.Request continue } - cdata.Value = c.ValueRequest(req) + v, s := c.ValueRequest(req) + if s != 0 { + err = true + cdata.Status = &s + } else { + cdata.Value = v + } if meta { cdata.Format = &c.Format @@ -156,13 +162,19 @@ func (srv *Server) putCharacteristics(res http.ResponseWriter, req *http.Request continue } - if d.Response != nil { - cdata.Value = c.ValueRequest(req) - arr = append(arr, cdata) + if d.Value != nil { + s := c.SetValueRequest(d.Value, req) + if s != 0 { + cdata.Status = &s + } } - if d.Value != nil { - c.SetValueRequest(d.Value, req) + if d.Response != nil { + if v, s := c.ValueRequest(req); s != 0 { + cdata.Status = &s + } else { + cdata.Value = v + } } if d.Events != nil { @@ -174,6 +186,10 @@ func (srv *Server) putCharacteristics(res http.ResponseWriter, req *http.Request c.Events[req.RemoteAddr] = *d.Events } } + + if cdata.Status != nil || cdata.Value != nil { + arr = append(arr, cdata) + } } if len(arr) == 0 { diff --git a/identify.go b/identify.go index a93f703..220f777 100644 --- a/identify.go +++ b/identify.go @@ -6,7 +6,6 @@ import ( func (srv *Server) identify(res http.ResponseWriter, req *http.Request) { if srv.isPaired() { - res.WriteHeader(http.StatusBadRequest) jsonError(res, JsonStatusInsufficientPrivileges) return } diff --git a/json.go b/json.go index a713071..a78f540 100644 --- a/json.go +++ b/json.go @@ -41,6 +41,7 @@ func jsonError(res http.ResponseWriter, status int) error { return err } + res.WriteHeader(http.StatusBadRequest) _, err = res.Write(b) return err } diff --git a/server_test.go b/server_test.go index 73cc4bd..08de33a 100644 --- a/server_test.go +++ b/server_test.go @@ -4,6 +4,9 @@ import ( "github.com/brutella/hap/accessory" "github.com/brutella/hap/service" + "bytes" + "fmt" + "io/ioutil" "net/http" "net/http/httptest" "testing" @@ -66,3 +69,120 @@ func TestIdentify(t *testing.T) { t.Fatalf("%v != %v", is, want) } } + +func TestSetValueRequestSuccess(t *testing.T) { + a := accessory.NewOutlet(accessory.Info{Name: "ABC"}) + + s, err := NewServer(NewMemStore(), a.A) + if err != nil { + t.Fatal(err) + } + + // fake pairing + p := Pairing{ + Name: "unit test", + } + if err := s.savePairing(p); err != nil { + t.Fatal(err) + } + + body := fmt.Sprintf("{\"characteristics\":[{\"aid\":%d,\"iid\":%d,\"value\":true}]}", a.Id, a.Outlet.On.Id) + req := httptest.NewRequest(http.MethodPut, "/characteristics", bytes.NewBuffer([]byte(body))) + w := httptest.NewRecorder() + + var setValueRequestFunc, onValueUpdateFunc bool + a.Outlet.On.SetValueRequestFunc = func(v interface{}, r *http.Request) int { + if is, want := v.(bool), true; is != want { + t.Fatalf("%v != %v", is, want) + } + + if r == nil { + t.Fatalf("request expected but got nil") + } + + setValueRequestFunc = true + + return 0 + } + + a.Outlet.On.OnValueUpdate(func(new bool, old bool, r *http.Request) { + if is, want := new, true; is != want { + t.Fatalf("%v != %v", is, want) + } + + if is, want := old, false; is != want { + t.Fatalf("%v != %v", is, want) + } + + if r == nil { + t.Fatalf("request expected but got nil") + } + + onValueUpdateFunc = true + }) + + s.ss.Handler.ServeHTTP(w, req) + + r := w.Result() + if is, want := r.StatusCode, http.StatusNoContent; is != want { + t.Fatalf("%v != %v", is, want) + } + + if is, want := setValueRequestFunc, true; is != want { + t.Fatalf("%v != %v", is, want) + } + + if is, want := onValueUpdateFunc, true; is != want { + t.Fatalf("%v != %v", is, want) + } + + if is, want := a.Outlet.On.Value(), true; is != want { + t.Fatalf("%v != %v", is, want) + } +} + +func TestSetValueRequestFailure(t *testing.T) { + a := accessory.NewOutlet(accessory.Info{Name: "ABC"}) + + s, err := NewServer(NewMemStore(), a.A) + if err != nil { + t.Fatal(err) + } + + // fake pairing + p := Pairing{ + Name: "unit test", + } + if err := s.savePairing(p); err != nil { + t.Fatal(err) + } + + body := fmt.Sprintf("{\"characteristics\":[{\"aid\":%d,\"iid\":%d,\"value\":true}]}", a.Id, a.Outlet.On.Id) + req := httptest.NewRequest(http.MethodPut, "/characteristics", bytes.NewBuffer([]byte(body))) + w := httptest.NewRecorder() + + a.Outlet.On.SetValueRequestFunc = func(v interface{}, r *http.Request) int { + return JsonStatusResourceBusy + } + + s.ss.Handler.ServeHTTP(w, req) + + r := w.Result() + if is, want := r.StatusCode, http.StatusMultiStatus; is != want { + t.Fatalf("%v != %v", is, want) + } + + b, err := ioutil.ReadAll(r.Body) + if err != nil { + t.Fatal(err) + } + + body = fmt.Sprintf("{\"characteristics\":[{\"aid\":%d,\"iid\":%d,\"status\":-70403}]}", a.Id, a.Outlet.On.Id) + if is, want := string(b), body; is != want { + t.Fatalf("%v != %v", is, want) + } + + if is, want := a.Outlet.On.Value(), false; is != want { + t.Fatalf("%v != %v", is, want) + } +}