From 24afcb090342ee6645611b236a286b06e7d60cad Mon Sep 17 00:00:00 2001 From: Tom Cole Date: Fri, 20 Dec 2024 13:03:06 -0500 Subject: [PATCH] fix: better reporting when invalid integer specified Numerous cases where Atoi conversions were done were reported using the error returned from the strconv package, which is not localized and is not very user friendly. This detects those cases and reports an Ego loalizable error. Also this fix updates a few cases where REST parameters that must be valid integers where being silently ignored when bogus values were given. --- builtins/append.go | 2 +- builtins/cast.go | 4 ++-- bytecode/coerce.go | 4 ++-- bytecode/context.go | 6 +++--- bytecode/create.go | 4 ++-- bytecode/math.go | 6 +++--- bytecode/store.go | 18 +++++++++--------- bytecode/types.go | 2 +- data/coerce.go | 8 ++++---- debugger/breaks.go | 2 +- debugger/show.go | 2 +- debugger/source.go | 2 +- errors/messages.go | 2 +- i18n/languages/messages_en.txt | 2 +- server/admin/logging.go | 7 ++++++- server/assets/handler.go | 12 ++++++++++-- server/server/admin.go | 6 +++--- server/services/cache.go | 8 +++++++- util/time.go | 4 ++-- 19 files changed, 60 insertions(+), 41 deletions(-) diff --git a/builtins/append.go b/builtins/append.go index a4d867b9..92bc0711 100644 --- a/builtins/append.go +++ b/builtins/append.go @@ -57,7 +57,7 @@ func Append(s *symbols.SymbolTable, args data.List) (interface{}, error) { // appropriate type. if typeChecking > defs.StrictTypeEnforcement { if j, err = data.Coerce(j, data.InstanceOfType(kind)); err != nil { - return nil, err + return nil, errors.New(err).In("append") } } else { // Nope, we are in strict type checking mode, so complain and be done. diff --git a/builtins/cast.go b/builtins/cast.go index e60814b0..20c508d4 100644 --- a/builtins/cast.go +++ b/builtins/cast.go @@ -52,7 +52,7 @@ func Cast(s *symbols.SymbolTable, args data.List) (interface{}, error) { value, err := data.Coerce(source, data.InstanceOfType(t.BaseType())) if err != nil { - return nil, err + return nil, errors.New(err).In(t.BaseType().String()) } _ = r.Set(0, value) @@ -62,7 +62,7 @@ func Cast(s *symbols.SymbolTable, args data.List) (interface{}, error) { v, err := data.Coerce(source, data.InstanceOfType(t)) if err != nil { - return nil, err + return nil, errors.New(err).In(t.String()) } if v != nil { diff --git a/bytecode/coerce.go b/bytecode/coerce.go index 206da3ca..4dcc16f0 100644 --- a/bytecode/coerce.go +++ b/bytecode/coerce.go @@ -109,9 +109,9 @@ func coerceByteCode(c *Context, i interface{}) error { for i, element := range base { v, err := data.Coerce(element, model) if err != nil { - return err + return c.error(err) } - + _ = array.Set(i, v) } diff --git a/bytecode/context.go b/bytecode/context.go index 7bae4561..bf574e09 100644 --- a/bytecode/context.go +++ b/bytecode/context.go @@ -492,14 +492,14 @@ func (c *Context) checkType(name string, value interface{}) (interface{}, error) if newT.IsIntegerType() && oldT.IsIntegerType() { value, err = data.Coerce(value, existingValue) if err != nil { - return nil, err + return nil, c.error(err) } } if newT.IsFloatType() && oldT.IsFloatType() { value, err = data.Coerce(value, existingValue) if err != nil { - return nil, err + return nil, c.error(err) } } } else if c.typeStrictness == defs.StrictTypeEnforcement && canCoerce { @@ -511,7 +511,7 @@ func (c *Context) checkType(name string, value interface{}) (interface{}, error) if ok && (oldT.IsIntegerType() || oldT.IsFloatType()) { value, err = data.Coerce(value, existingValue) if err != nil { - return nil, err + return nil, c.error(err) } } diff --git a/bytecode/create.go b/bytecode/create.go index d2f4f0bc..e9014aac 100644 --- a/bytecode/create.go +++ b/bytecode/create.go @@ -336,7 +336,7 @@ func applyStructModel(c *Context, model interface{}, structMap map[string]interf // by chasing the model chain. err := addMissingFields(model, structMap, c) if err != nil { - return nil, err + return nil, c.error(err) } } else { return nil, c.error(errors.ErrUnknownType, typeInfo.String()) @@ -370,7 +370,7 @@ func addMissingFields(model *data.Struct, structMap map[string]interface{}, c *C if err == nil { return err } - + structMap[fieldName] = existingValue } else { typeString := data.TypeOf(existingValue).String() diff --git a/bytecode/math.go b/bytecode/math.go index 219e4b8b..214eeff2 100644 --- a/bytecode/math.go +++ b/bytecode/math.go @@ -59,7 +59,7 @@ func incrementByteCode(c *Context, i interface{}) error { if a.Type().BaseType().Kind() != data.InterfaceType.Kind() { increment, err = data.Coerce(increment, data.InstanceOfType(a.Type().BaseType())) if err != nil { - return err + return c.error(err) } } } @@ -282,7 +282,7 @@ func addByteCode(c *Context, i interface{}) error { if _, ok := v2.(*data.Array); !ok && c.typeStrictness != defs.StrictTypeEnforcement { v2, err = data.Coerce(v2, data.InstanceOfType(a.Type().BaseType())) if err != nil { - return err + return c.error(err) } } @@ -298,7 +298,7 @@ func addByteCode(c *Context, i interface{}) error { if _, ok := v1.(*data.Array); !ok && c.typeStrictness != defs.StrictTypeEnforcement { v1, err = data.Coerce(v1, data.InstanceOfType(a.Type().BaseType())) if err != nil { - return err + return c.error(err) } } diff --git a/bytecode/store.go b/bytecode/store.go index c1076718..c02c8614 100644 --- a/bytecode/store.go +++ b/bytecode/store.go @@ -320,7 +320,7 @@ func storeStringViaPointer(c *Context, name string, src interface{}, destination if c.typeStrictness > defs.RelaxedTypeEnforcement { d, err = data.Coerce(src, "") if err != nil { - return err + return c.error(err) } } else if _, ok := d.(string); !ok { return c.error(errors.ErrInvalidVarType).Context(name) @@ -338,7 +338,7 @@ func storeFloat32ViaPointer(c *Context, name string, src interface{}, destinatio if c.typeStrictness > defs.RelaxedTypeEnforcement { d, err = data.Coerce(src, float32(0)) if err != nil { - return err + return c.error(err) } } else if _, ok := d.(string); !ok { return c.error(errors.ErrInvalidVarType).Context(name) @@ -356,7 +356,7 @@ func storeFloat64ViaPointer(c *Context, name string, src interface{}, destinatio if c.typeStrictness > defs.RelaxedTypeEnforcement { d, err = data.Coerce(src, float64(0)) if err != nil { - return err + return c.error(err) } } else if _, ok := d.(string); !ok { return c.error(errors.ErrInvalidVarType).Context(name) @@ -374,7 +374,7 @@ func storeInt64ViaPointer(c *Context, name string, src interface{}, actual *int6 if c.typeStrictness > defs.RelaxedTypeEnforcement { d, err = data.Coerce(src, int64(1)) if err != nil { - return err + return c.error(err) } } else if _, ok := d.(string); !ok { return c.error(errors.ErrInvalidVarType).Context(name) @@ -392,7 +392,7 @@ func storeIntViaPointer(c *Context, name string, src interface{}, actual *int) e if c.typeStrictness > defs.RelaxedTypeEnforcement { d, err = data.Coerce(src, int(1)) if err != nil { - return err + return c.error(err) } } else if _, ok := d.(string); !ok { return c.error(errors.ErrInvalidVarType).Context(name) @@ -410,7 +410,7 @@ func storeInt32ViaPointer(c *Context, name string, src interface{}, actual *int3 if c.typeStrictness > defs.RelaxedTypeEnforcement { d, err = data.Coerce(src, int32(1)) if err != nil { - return err + return c.error(err) } } else if _, ok := d.(string); !ok { return c.error(errors.ErrInvalidVarType).Context(name) @@ -428,7 +428,7 @@ func storeByteViaPointer(c *Context, name string, src interface{}, actual *byte) if c.typeStrictness > defs.RelaxedTypeEnforcement { d, err = data.Coerce(src, byte(1)) if err != nil { - return err + return c.error(err) } } else if _, ok := d.(string); !ok { return c.error(errors.ErrInvalidVarType).Context(name) @@ -446,7 +446,7 @@ func storeBoolViaPointer(c *Context, name string, src interface{}, actual *bool) if c.typeStrictness > defs.RelaxedTypeEnforcement { d, err = data.Coerce(src, true) if err != nil { - return err + return c.error(err) } } else if _, ok := d.(string); !ok { return c.error(errors.ErrInvalidVarType).Context(name) @@ -475,7 +475,7 @@ func storeAlwaysByteCode(c *Context, i interface{}) error { v, err = c.Pop() if err != nil { - return err + return c.error(err) } if isStackMarker(v) { diff --git a/bytecode/types.go b/bytecode/types.go index 7a6cc2ae..a6a7ab48 100644 --- a/bytecode/types.go +++ b/bytecode/types.go @@ -98,7 +98,7 @@ func unwrapByteCode(c *Context, i interface{}) error { if c.typeStrictness != defs.StrictTypeEnforcement { newValue, err = data.Coerce(value, newType.InstanceOf(newType.BaseType())) if err != nil { - return err + return c.error(err) } } else { if !actualType.IsType(newType) { diff --git a/data/coerce.go b/data/coerce.go index 761895be..c82efdd7 100644 --- a/data/coerce.go +++ b/data/coerce.go @@ -251,7 +251,7 @@ func coerceToInt(v interface{}) (interface{}, error) { st, err := strconv.Atoi(value) if err != nil { - return nil, err + return nil, errors.ErrInvalidInteger.Context(value) } return st, nil @@ -297,7 +297,7 @@ func coerceToInt64(v interface{}) (interface{}, error) { st, err := strconv.Atoi(value) if err != nil { - return nil, err + return nil, errors.ErrInvalidInteger.Context(value) } return int64(st), nil @@ -358,7 +358,7 @@ func coerceInt32(v interface{}) (interface{}, error) { intValue, err := strconv.Atoi(value) if err != nil { - return nil, err + return nil, errors.ErrInvalidInteger.Context(value) } return coerceInt32(intValue) @@ -429,7 +429,7 @@ func coerceToByte(v interface{}) (interface{}, error) { st, err := strconv.Atoi(value) if err != nil { - return nil, err + return nil, errors.ErrInvalidInteger.Context(value) } return coerceToByte(st) diff --git a/debugger/breaks.go b/debugger/breaks.go index 85cebb09..6a2342b1 100644 --- a/debugger/breaks.go +++ b/debugger/breaks.go @@ -104,7 +104,7 @@ func breakCommand(t *tokenizer.Tokenizer) error { err = breakAtLine(name, line) } } else { - err = errors.New(e2) + err = errors.New(errors.ErrInvalidInteger) } clauses++ diff --git a/debugger/show.go b/debugger/show.go index a0b13e92..009f46b2 100644 --- a/debugger/show.go +++ b/debugger/show.go @@ -55,7 +55,7 @@ func showCommand(s *symbols.SymbolTable, tokens *tokenizer.Tokenizer, line int, depth, e2 = strconv.Atoi(tx.Spelling()) if e2 != nil { - err = errors.New(e2) + err = errors.ErrInvalidInteger } } } diff --git a/debugger/source.go b/debugger/source.go index 74cb42dc..29b66329 100644 --- a/debugger/source.go +++ b/debugger/source.go @@ -29,7 +29,7 @@ func showSource(tx *tokenizer.Tokenizer, tokens *tokenizer.Tokenizer, err error) } if e2 != nil { - err = errors.New(e2) + err = errors.New(errors.ErrInvalidInteger) } } diff --git a/errors/messages.go b/errors/messages.go index dbdada2d..909e6448 100644 --- a/errors/messages.go +++ b/errors/messages.go @@ -81,7 +81,7 @@ var ErrInvalidFunctionName = Message("func.name") var ErrInvalidIdentifier = Message("identifier") var ErrInvalidImport = Message("import") var ErrInvalidInstruction = Message("instruction") -var ErrInvalidInteger = Message("integer.option") +var ErrInvalidInteger = Message("integer.value") var ErrInvalidKeyword = Message("keyword.option") var ErrInvalidLineNumber = Message("line.number") var ErrInvalidList = Message("list") diff --git a/i18n/languages/messages_en.txt b/i18n/languages/messages_en.txt index a96b511f..16476310 100644 --- a/i18n/languages/messages_en.txt +++ b/i18n/languages/messages_en.txt @@ -177,7 +177,7 @@ import=import not permitted inside a block or loop import.not.found=attempt to use imported package not in package cache initializer.count=incorrect number of initialization values instruction=invalid instruction -integer.option=invalid integer option value +integer.value=invalid integer value interface.imp=missing interface implementation invalid.alignment.spec=invalid alignment specification invalid.auto=invalid use of auto increment/decrement operation diff --git a/server/admin/logging.go b/server/admin/logging.go index 05d29673..90535aff 100644 --- a/server/admin/logging.go +++ b/server/admin/logging.go @@ -76,12 +76,17 @@ func GetLoggingHandler(session *server.Session, w http.ResponseWriter, r *http.R // The request can optionally have a "keep" URL parameter which overrides the default number // of log entries to keep. A keep value of less than 1 is the same as 1. func PurgeLogHandler(session *server.Session, w http.ResponseWriter, r *http.Request) int { + var err error + keep := ui.LogRetainCount q := r.URL.Query() if v, found := q["keep"]; found { if len(v) == 1 { - keep, _ = strconv.Atoi(v[0]) + keep, err = strconv.Atoi(v[0]) + if err != nil { + return util.ErrorResponse(w, session.ID, "Invalid keep value: "+v[0], http.StatusBadRequest) + } } } diff --git a/server/assets/handler.go b/server/assets/handler.go index ada7e287..19065280 100644 --- a/server/assets/handler.go +++ b/server/assets/handler.go @@ -16,6 +16,7 @@ import ( "github.com/tucats/ego/app-cli/ui" "github.com/tucats/ego/defs" "github.com/tucats/ego/server/server" + "github.com/tucats/ego/util" ) // AssetsHandler is the handler for the GET method on the assets endpoint. The handler @@ -78,11 +79,18 @@ func AssetsHandler(session *server.Session, w http.ResponseWriter, r *http.Reque ranges := strings.Split(text, "-") if len(ranges) > 0 { - start, _ = strconv.Atoi(ranges[0]) + start, err = strconv.Atoi(ranges[0]) + if err != nil { + return util.ErrorResponse(w, session.ID, "Invalid range header: "+h[0], http.StatusBadRequest) + } } if len(ranges) > 1 { - end, _ = strconv.Atoi(ranges[1]) + end, err = strconv.Atoi(ranges[1]) + if err != nil { + return util.ErrorResponse(w, session.ID, "Invalid range header: "+h[0], http.StatusBadRequest) + } + hasRange = fmt.Sprintf(" range %d-%d;", start, end) } } diff --git a/server/server/admin.go b/server/server/admin.go index 778bfff0..ef492383 100644 --- a/server/server/admin.go +++ b/server/server/admin.go @@ -152,7 +152,7 @@ func LogHandler(session *Session, w http.ResponseWriter, r *http.Request) int { ui.Log(ui.RouteLogger, "[%d] Using native handler to access log lines", session.ID) - // IF present, get the "tail" value that says how many lines of output we are + // If present, get the "tail" value that says how many lines of output we are // asked to retrieve. If not present, default to 50 lines. If th estring value // is invalid, return an error response to the caller. if v, found := session.Parameters["tail"]; found && len(v) > 0 { @@ -160,7 +160,7 @@ func LogHandler(session *Session, w http.ResponseWriter, r *http.Request) int { if err != nil { ui.Log(ui.AuthLogger, "[%d] Unexpected error %v", session.ID, err) - return util.ErrorResponse(w, session.ID, err.Error(), http.StatusBadRequest) + return util.ErrorResponse(w, session.ID, "Invalid tail integer value: "+v[0], http.StatusBadRequest) } } @@ -171,7 +171,7 @@ func LogHandler(session *Session, w http.ResponseWriter, r *http.Request) int { if err != nil { ui.Log(ui.AuthLogger, "[%d] Unexpected error %v", session.ID, err) - return util.ErrorResponse(w, session.ID, err.Error(), http.StatusBadRequest) + return util.ErrorResponse(w, session.ID, "Invalid session id value: "+v[0], http.StatusBadRequest) } } diff --git a/server/services/cache.go b/server/services/cache.go index 83bd6812..5d3ae27b 100644 --- a/server/services/cache.go +++ b/server/services/cache.go @@ -40,7 +40,13 @@ func setupServiceCache() { if MaxCachedEntries < 0 { txt := settings.Get(defs.MaxCacheSizeSetting) - MaxCachedEntries, _ = strconv.Atoi(txt) + + n, err := strconv.Atoi(txt) + if err != nil { + ui.Log(ui.ServicesLogger, "Ignoring invalid config value for %s: %s", defs.MaxCacheSizeSetting, txt) + } else { + MaxCachedEntries = n + } } serviceCacheMutex.Unlock() diff --git a/util/time.go b/util/time.go index 9343fafd..aae6ce09 100644 --- a/util/time.go +++ b/util/time.go @@ -58,7 +58,7 @@ func parseDuruationWithDays(durationString string) (days int, hours int, mins in if chars != "" { value, err = strconv.Atoi(chars) if err != nil { - return days, hours, mins, secs, ms, errors.New(err).Context(chars) + return days, hours, mins, secs, ms, errors.ErrInvalidInteger.Context(chars) } } @@ -110,7 +110,7 @@ func parseDuruationWithDays(durationString string) (days int, hours int, mins in if chars != "" { mins, err = strconv.Atoi(chars) if err != nil { - return days, hours, mins, secs, ms, errors.New(err).Context(chars) + return days, hours, mins, secs, ms, errors.ErrInvalidInteger.Context(chars) } }