From 2c04e4c8b295d9737b16d2538c89ebf9c45a9610 Mon Sep 17 00:00:00 2001 From: Christophe Varoqui Date: Wed, 28 Aug 2024 12:09:07 +0200 Subject: [PATCH] Fix daemon panic on object.New uses object.New should verify the type cast worked, and raise a new object.ErrWrongType error if not. The api handlers now emit a "Bad Request" response if they get a object.ErrWrongType, and "Internal Server Error" is still used for all other errors. --- core/object/factory.go | 18 ++++++++++++++---- .../daemonapi/delete_object_kvstore_entry.go | 6 +++++- daemon/daemonapi/get_object_kvstore.go | 7 ++++++- daemon/daemonapi/get_object_kvstore_entry.go | 7 ++++++- daemon/daemonapi/get_object_kvstore_keys.go | 7 ++++++- daemon/daemonapi/patch_object_kvstore.go | 7 ++++++- daemon/daemonapi/post_object_kvstore_entry.go | 7 ++++++- daemon/daemonapi/put_object_kvstore_entry.go | 7 ++++++- 8 files changed, 55 insertions(+), 11 deletions(-) diff --git a/core/object/factory.go b/core/object/factory.go index 0ccf4119d..696e16a34 100644 --- a/core/object/factory.go +++ b/core/object/factory.go @@ -9,6 +9,8 @@ import ( "github.com/opensvc/om3/util/plog" ) +var ErrWrongType = errors.New("wrong type provided for interface") + // WithConfigFile sets a non-standard configuration location. func WithConfigFile(s string) funcopt.O { return funcopt.F(func(t any) error { @@ -87,8 +89,10 @@ func New(p naming.Path, opts ...funcopt.O) (any, error) { func NewCore(p naming.Path, opts ...funcopt.O) (Core, error) { if o, err := New(p, opts...); err != nil { return nil, err + } else if i, ok := o.(Core); ok { + return i, nil } else { - return o.(Core), nil + return nil, ErrWrongType } } @@ -96,8 +100,10 @@ func NewCore(p naming.Path, opts ...funcopt.O) (Core, error) { func NewConfigurer(p naming.Path, opts ...funcopt.O) (Configurer, error) { if o, err := New(p, opts...); err != nil { return nil, err + } else if i, ok := o.(Configurer); ok { + return i, nil } else { - return o.(Configurer), nil + return nil, ErrWrongType } } @@ -105,8 +111,10 @@ func NewConfigurer(p naming.Path, opts ...funcopt.O) (Configurer, error) { func NewActor(p naming.Path, opts ...funcopt.O) (Actor, error) { if o, err := New(p, opts...); err != nil { return nil, err + } else if i, ok := o.(Actor); ok { + return i, nil } else { - return o.(Actor), nil + return nil, ErrWrongType } } @@ -114,7 +122,9 @@ func NewActor(p naming.Path, opts ...funcopt.O) (Actor, error) { func NewKeystore(p naming.Path, opts ...funcopt.O) (Keystore, error) { if o, err := New(p, opts...); err != nil { return nil, err + } else if i, ok := o.(Keystore); ok { + return i, nil } else { - return o.(Keystore), nil + return nil, ErrWrongType } } diff --git a/daemon/daemonapi/delete_object_kvstore_entry.go b/daemon/daemonapi/delete_object_kvstore_entry.go index ca593b24f..87e9c1b3a 100644 --- a/daemon/daemonapi/delete_object_kvstore_entry.go +++ b/daemon/daemonapi/delete_object_kvstore_entry.go @@ -30,7 +30,11 @@ func (a *DaemonAPI) DeleteObjectKVStoreEntry(ctx echo.Context, namespace string, if _, ok := instanceConfigData[a.localhost]; ok { ks, err := object.NewKeystore(p) - if err != nil { + + switch { + case errors.Is(err, object.ErrWrongType): + return JSONProblemf(ctx, http.StatusBadRequest, "NewKeystore", "%s", err) + case err != nil: return JSONProblemf(ctx, http.StatusInternalServerError, "NewKeystore", "%s", err) } diff --git a/daemon/daemonapi/get_object_kvstore.go b/daemon/daemonapi/get_object_kvstore.go index c580a3d9c..525672e68 100644 --- a/daemon/daemonapi/get_object_kvstore.go +++ b/daemon/daemonapi/get_object_kvstore.go @@ -1,6 +1,7 @@ package daemonapi import ( + "errors" "net/http" "github.com/labstack/echo/v4" @@ -31,7 +32,11 @@ func (a *DaemonAPI) GetObjectKVStore(ctx echo.Context, namespace string, kind na if _, ok := instanceConfigData[a.localhost]; ok { ks, err := object.NewKeystore(p) - if err != nil { + + switch { + case errors.Is(err, object.ErrWrongType): + return JSONProblemf(ctx, http.StatusBadRequest, "NewKeystore", "%s", err) + case err != nil: return JSONProblemf(ctx, http.StatusInternalServerError, "NewKeystore", "%s", err) } diff --git a/daemon/daemonapi/get_object_kvstore_entry.go b/daemon/daemonapi/get_object_kvstore_entry.go index 383132871..2129b7755 100644 --- a/daemon/daemonapi/get_object_kvstore_entry.go +++ b/daemon/daemonapi/get_object_kvstore_entry.go @@ -1,6 +1,7 @@ package daemonapi import ( + "errors" "net/http" "unicode/utf8" @@ -30,7 +31,11 @@ func (a *DaemonAPI) GetObjectKVStoreEntry(ctx echo.Context, namespace string, ki if _, ok := instanceConfigData[a.localhost]; ok { ks, err := object.NewKeystore(p) - if err != nil { + + switch { + case errors.Is(err, object.ErrWrongType): + return JSONProblemf(ctx, http.StatusBadRequest, "NewKeystore", "%s", err) + case err != nil: return JSONProblemf(ctx, http.StatusInternalServerError, "NewKeystore", "%s", err) } diff --git a/daemon/daemonapi/get_object_kvstore_keys.go b/daemon/daemonapi/get_object_kvstore_keys.go index 158ec5d42..9a0cf8a49 100644 --- a/daemon/daemonapi/get_object_kvstore_keys.go +++ b/daemon/daemonapi/get_object_kvstore_keys.go @@ -1,6 +1,7 @@ package daemonapi import ( + "errors" "net/http" "github.com/labstack/echo/v4" @@ -30,7 +31,11 @@ func (a *DaemonAPI) GetObjectKVStoreKeys(ctx echo.Context, namespace string, kin if _, ok := instanceConfigData[a.localhost]; ok { ks, err := object.NewKeystore(p) - if err != nil { + + switch { + case errors.Is(err, object.ErrWrongType): + return JSONProblemf(ctx, http.StatusBadRequest, "NewKeystore", "%s", err) + case err != nil: return JSONProblemf(ctx, http.StatusInternalServerError, "NewKeystore", "%s", err) } diff --git a/daemon/daemonapi/patch_object_kvstore.go b/daemon/daemonapi/patch_object_kvstore.go index 6e2559d59..479a701ee 100644 --- a/daemon/daemonapi/patch_object_kvstore.go +++ b/daemon/daemonapi/patch_object_kvstore.go @@ -1,6 +1,7 @@ package daemonapi import ( + "errors" "net/http" "github.com/labstack/echo/v4" @@ -57,7 +58,11 @@ func (a *DaemonAPI) PatchObjectKVStore(ctx echo.Context, namespace string, kind if _, ok := instanceConfigData[a.localhost]; ok { ks, err := object.NewKeystore(p) - if err != nil { + + switch { + case errors.Is(err, object.ErrWrongType): + return JSONProblemf(ctx, http.StatusBadRequest, "NewKeystore", "%s", err) + case err != nil: return JSONProblemf(ctx, http.StatusInternalServerError, "NewKeystore", "%s", err) } diff --git a/daemon/daemonapi/post_object_kvstore_entry.go b/daemon/daemonapi/post_object_kvstore_entry.go index 37b739980..30d5358fe 100644 --- a/daemon/daemonapi/post_object_kvstore_entry.go +++ b/daemon/daemonapi/post_object_kvstore_entry.go @@ -31,9 +31,14 @@ func (a *DaemonAPI) PostObjectKVStoreEntry(ctx echo.Context, namespace string, k if _, ok := instanceConfigData[a.localhost]; ok { ks, err := object.NewKeystore(p) - if err != nil { + + switch { + case errors.Is(err, object.ErrWrongType): + return JSONProblemf(ctx, http.StatusBadRequest, "NewKeystore", "%s", err) + case err != nil: return JSONProblemf(ctx, http.StatusInternalServerError, "NewKeystore", "%s", err) } + b, err := ioutil.ReadAll(ctx.Request().Body) if err != nil { return JSONProblemf(ctx, http.StatusInternalServerError, "ReadAll", "%s: %s", params.Key, err) diff --git a/daemon/daemonapi/put_object_kvstore_entry.go b/daemon/daemonapi/put_object_kvstore_entry.go index 3b4c8f29b..f7fe5e415 100644 --- a/daemon/daemonapi/put_object_kvstore_entry.go +++ b/daemon/daemonapi/put_object_kvstore_entry.go @@ -32,9 +32,14 @@ func (a *DaemonAPI) PutObjectKVStoreEntry(ctx echo.Context, namespace string, ki if _, ok := instanceConfigData[a.localhost]; ok { ks, err := object.NewKeystore(p) - if err != nil { + + switch { + case errors.Is(err, object.ErrWrongType): + return JSONProblemf(ctx, http.StatusBadRequest, "NewKeystore", "%s", err) + case err != nil: return JSONProblemf(ctx, http.StatusInternalServerError, "NewKeystore", "%s", err) } + keys, err := ks.AllKeys() if err != nil { return JSONProblemf(ctx, http.StatusInternalServerError, "AllKeys", "%s", err)