Skip to content

Commit

Permalink
fix: better reporting when invalid integer specified
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tucats committed Dec 20, 2024
1 parent dbdab02 commit 24afcb0
Show file tree
Hide file tree
Showing 19 changed files with 60 additions and 41 deletions.
2 changes: 1 addition & 1 deletion builtins/append.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions builtins/cast.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions bytecode/coerce.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
6 changes: 3 additions & 3 deletions bytecode/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
}
}

Expand Down
4 changes: 2 additions & 2 deletions bytecode/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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()
Expand Down
6 changes: 3 additions & 3 deletions bytecode/math.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down Expand Up @@ -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)
}
}

Expand All @@ -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)
}
}

Expand Down
18 changes: 9 additions & 9 deletions bytecode/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion bytecode/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 4 additions & 4 deletions data/coerce.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion debugger/breaks.go
Original file line number Diff line number Diff line change
Expand Up @@ -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++
Expand Down
2 changes: 1 addition & 1 deletion debugger/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion debugger/source.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
2 changes: 1 addition & 1 deletion errors/messages.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion i18n/languages/messages_en.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion server/admin/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}

Expand Down
12 changes: 10 additions & 2 deletions server/assets/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
Expand Down
6 changes: 3 additions & 3 deletions server/server/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,15 +152,15 @@ 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 {
count, err = strconv.Atoi(v[0])
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)
}
}

Expand All @@ -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)
}
}

Expand Down
8 changes: 7 additions & 1 deletion server/services/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions util/time.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)
}
}

Expand Down

0 comments on commit 24afcb0

Please sign in to comment.