Skip to content
This repository has been archived by the owner on Sep 7, 2021. It is now read-only.
This repository is currently being migrated. It's locked while the migration is in progress.

Fix bug on insert where #1436

Merged
merged 3 commits into from
Sep 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions session_insert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -908,6 +908,15 @@ func TestInsertWhere(t *testing.T) {
assert.True(t, has)
assert.EqualValues(t, "trest3", j3.Name)
assert.EqualValues(t, 3, j3.Index)

inserted, err = testEngine.Table(new(InsertWhere)).Where("repo_id=?", 1).
SetExpr("`index`", "coalesce(MAX(`index`),0)+1").
Insert(map[string]interface{}{
"repo_id": 1,
"name": "10';delete * from insert_where; --",
})
assert.NoError(t, err)
assert.EqualValues(t, 1, inserted)
}

type NightlyRate struct {
Expand Down
53 changes: 48 additions & 5 deletions statement_args.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,60 @@ package xorm

import (
"fmt"
"reflect"
"strings"
"time"

"xorm.io/builder"
"xorm.io/core"
)

func quoteNeeded(a interface{}) bool {
switch a.(type) {
case int, int8, int16, int32, int64:
return false
case uint, uint8, uint16, uint32, uint64:
return false
case float32, float64:
return false
case bool:
return false
case string:
return true
case time.Time, *time.Time:
return true
case builder.Builder, *builder.Builder:
return false
}

t := reflect.TypeOf(a)
switch t.Kind() {
case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
return false
case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64:
return false
case reflect.Float32, reflect.Float64:
return false
case reflect.Bool:
return false
case reflect.String:
return true
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it safer to panic if no match is found? It's possible that some future type of field will be implemented and someone forgets to add it here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For an unknown type, it will be converted to a string via fmt.Sprintf("%v", arg) and replace single quote to two single quotes. Currently all go's basic types have been supported. I think that's enough.


return true
}

func convertArg(arg interface{}) string {
if quoteNeeded(arg) {
argv := fmt.Sprintf("%v", arg)
return "'" + strings.Replace(argv, "'", "''", -1) + "'"
}

return fmt.Sprintf("%v", arg)
}

func (statement *Statement) writeArg(w *builder.BytesWriter, arg interface{}) error {
switch argv := arg.(type) {
case string:
if _, err := w.WriteString("'" + argv + "'"); err != nil {
return err
}
case bool:
if statement.Engine.dialect.DBType() == core.MSSQL {
if argv {
Expand Down Expand Up @@ -50,7 +93,7 @@ func (statement *Statement) writeArg(w *builder.BytesWriter, arg interface{}) er
return err
}
default:
if _, err := w.WriteString(fmt.Sprintf("%v", argv)); err != nil {
if _, err := w.WriteString(convertArg(arg)); err != nil {
return err
}
}
Expand Down