From 81c17a7e2529c59efc4e74c5b32c1fb71fb12fa2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emir=20Beganovi=C4=87?= Date: Wed, 25 Sep 2019 13:22:43 +0200 Subject: [PATCH 1/4] Revert "Fix #2517 : Check for incomplete parentheses to prevent SQL injection." (#2674) This reverts commit e3cc5ea4d403078a370e299629da56cd011b6583. --- query_test.go | 17 ----------------- scope.go | 21 --------------------- 2 files changed, 38 deletions(-) diff --git a/query_test.go b/query_test.go index 2b7e0dff..15bf8b3c 100644 --- a/query_test.go +++ b/query_test.go @@ -133,23 +133,6 @@ func TestStringPrimaryKeyForNumericValueStartingWithZero(t *testing.T) { t.Errorf("Fetch a record from with a string primary key for a numeric value starting with zero should work, but failed, zip code is %v", address.ZipCode) } } -func TestStringAgainstIncompleteParentheses(t *testing.T) { - type AddressByZipCode struct { - ZipCode string `gorm:"primary_key"` - Address string - } - - DB.AutoMigrate(&AddressByZipCode{}) - DB.Create(&AddressByZipCode{ZipCode: "00502", Address: "Holtsville"}) - - var address AddressByZipCode - var addresses []AddressByZipCode - _ = DB.First(&address, "address_by_zip_codes=00502)) UNION ALL SELECT NULL,version(),current_database(),NULL,NULL,NULL,NULL,NULL--").Find(&addresses).GetErrors() - if len(addresses) > 0 { - t.Errorf("Fetch a record from with a string that has incomplete parentheses should be fail, zip code is %v", address.ZipCode) - } - -} func TestFindAsSliceOfPointers(t *testing.T) { DB.Save(&User{Name: "user"}) diff --git a/scope.go b/scope.go index 541fe522..c962c165 100644 --- a/scope.go +++ b/scope.go @@ -277,23 +277,6 @@ func (scope *Scope) AddToVars(value interface{}) string { return scope.Dialect().BindVar(len(scope.SQLVars)) } -// IsCompleteParentheses check if the string has complete parentheses to prevent SQL injection -func (scope *Scope) IsCompleteParentheses(value string) bool { - count := 0 - for i, _ := range value { - if value[i] == 40 { // ( - count++ - } else if value[i] == 41 { // ) - count-- - } - if count < 0 { - break - } - i++ - } - return count == 0 -} - // SelectAttrs return selected attributes func (scope *Scope) SelectAttrs() []string { if scope.selectAttrs == nil { @@ -573,10 +556,6 @@ func (scope *Scope) buildCondition(clause map[string]interface{}, include bool) } if value != "" { - if !scope.IsCompleteParentheses(value) { - scope.Err(fmt.Errorf("incomplete parentheses found: %v", value)) - return - } if !include { if comparisonRegexp.MatchString(value) { str = fmt.Sprintf("NOT (%v)", value) From e5d0267c0bee4a92af603ea570fa9121e6440b11 Mon Sep 17 00:00:00 2001 From: Jay Chung Date: Sat, 5 Oct 2019 12:12:47 +0800 Subject: [PATCH 2/4] Fix typo of example code --- callback.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/callback.go b/callback.go index 202af06e..719b0a78 100644 --- a/callback.go +++ b/callback.go @@ -119,8 +119,8 @@ func (cp *CallbackProcessor) Remove(callbackName string) { // Replace a registered callback with new callback // db.Callback().Create().Replace("gorm:update_time_stamp_when_create", func(*Scope) { -// scope.SetColumn("Created", now) -// scope.SetColumn("Updated", now) +// scope.SetColumn("CreatedAt", now) +// scope.SetColumn("UpdatedAt", now) // }) func (cp *CallbackProcessor) Replace(callbackName string, callback func(scope *Scope)) { cp.logger.Print(fmt.Sprintf("[info] replacing callback `%v` from %v\n", callbackName, fileWithLineNum())) From 820b5f244abf7ef16f362de39b19adfef31fff2d Mon Sep 17 00:00:00 2001 From: Alex Stockwell Date: Thu, 17 Oct 2019 07:54:11 -0700 Subject: [PATCH 3/4] MSSQL Create() fix: Add LastInsertIDReturningSuffix to dialect (#2690) * MSSQL Create() fix: Add LastInsertIDReturningSuffix to dialect Per https://github.com/denisenkom/go-mssqldb/issues/355 * MSSQL Create() fix: Added OUTPUT query to Create() builder --- callback_create.go | 47 +++++++++++++++++++++++++++++------------ dialect.go | 2 ++ dialect_common.go | 4 ++++ dialect_postgres.go | 4 ++++ dialects/mssql/mssql.go | 8 +++++++ 5 files changed, 52 insertions(+), 13 deletions(-) diff --git a/callback_create.go b/callback_create.go index 87aba8ee..3527858b 100644 --- a/callback_create.go +++ b/callback_create.go @@ -101,10 +101,11 @@ func createCallback(scope *Scope) { } lastInsertIDReturningSuffix := scope.Dialect().LastInsertIDReturningSuffix(quotedTableName, returningColumn) + lastInsertIDOutputInterstitial := scope.Dialect().LastInsertIDOutputInterstitial(quotedTableName, returningColumn, columns) if len(columns) == 0 { scope.Raw(fmt.Sprintf( - "INSERT %v INTO %v %v%v%v", + "INSERT%v INTO %v %v%v%v", addExtraSpaceIfExist(insertModifier), quotedTableName, scope.Dialect().DefaultValueStr(), @@ -113,18 +114,19 @@ func createCallback(scope *Scope) { )) } else { scope.Raw(fmt.Sprintf( - "INSERT %v INTO %v (%v) VALUES (%v)%v%v", + "INSERT%v INTO %v (%v)%v VALUES (%v)%v%v", addExtraSpaceIfExist(insertModifier), scope.QuotedTableName(), strings.Join(columns, ","), + addExtraSpaceIfExist(lastInsertIDOutputInterstitial), strings.Join(placeholders, ","), addExtraSpaceIfExist(extraOption), addExtraSpaceIfExist(lastInsertIDReturningSuffix), )) } - // execute create sql - if lastInsertIDReturningSuffix == "" || primaryField == nil { + // execute create sql: no primaryField + if primaryField == nil { if result, err := scope.SQLDB().Exec(scope.SQL, scope.SQLVars...); scope.Err(err) == nil { // set rows affected count scope.db.RowsAffected, _ = result.RowsAffected() @@ -136,16 +138,35 @@ func createCallback(scope *Scope) { } } } - } else { - if primaryField.Field.CanAddr() { - if err := scope.SQLDB().QueryRow(scope.SQL, scope.SQLVars...).Scan(primaryField.Field.Addr().Interface()); scope.Err(err) == nil { - primaryField.IsBlank = false - scope.db.RowsAffected = 1 - } - } else { - scope.Err(ErrUnaddressable) - } + return } + + // execute create sql: lastInsertID implemention for majority of dialects + if lastInsertIDReturningSuffix == "" && lastInsertIDOutputInterstitial == "" { + if result, err := scope.SQLDB().Exec(scope.SQL, scope.SQLVars...); scope.Err(err) == nil { + // set rows affected count + scope.db.RowsAffected, _ = result.RowsAffected() + + // set primary value to primary field + if primaryField != nil && primaryField.IsBlank { + if primaryValue, err := result.LastInsertId(); scope.Err(err) == nil { + scope.Err(primaryField.Set(primaryValue)) + } + } + } + return + } + + // execute create sql: dialects with additional lastInsertID requirements (currently postgres & mssql) + if primaryField.Field.CanAddr() { + if err := scope.SQLDB().QueryRow(scope.SQL, scope.SQLVars...).Scan(primaryField.Field.Addr().Interface()); scope.Err(err) == nil { + primaryField.IsBlank = false + scope.db.RowsAffected = 1 + } + } else { + scope.Err(ErrUnaddressable) + } + return } } diff --git a/dialect.go b/dialect.go index 831c0a8e..b6f95df7 100644 --- a/dialect.go +++ b/dialect.go @@ -40,6 +40,8 @@ type Dialect interface { LimitAndOffsetSQL(limit, offset interface{}) string // SelectFromDummyTable return select values, for most dbs, `SELECT values` just works, mysql needs `SELECT value FROM DUAL` SelectFromDummyTable() string + // LastInsertIDOutputInterstitial most dbs support LastInsertId, but mssql needs to use `OUTPUT` + LastInsertIDOutputInterstitial(tableName, columnName string, columns []string) string // LastInsertIdReturningSuffix most dbs support LastInsertId, but postgres needs to use `RETURNING` LastInsertIDReturningSuffix(tableName, columnName string) string // DefaultValueStr diff --git a/dialect_common.go b/dialect_common.go index e3a5b702..16da76dc 100644 --- a/dialect_common.go +++ b/dialect_common.go @@ -157,6 +157,10 @@ func (commonDialect) SelectFromDummyTable() string { return "" } +func (commonDialect) LastInsertIDOutputInterstitial(tableName, columnName string, columns []string) string { + return "" +} + func (commonDialect) LastInsertIDReturningSuffix(tableName, columnName string) string { return "" } diff --git a/dialect_postgres.go b/dialect_postgres.go index 53d31388..d2df3131 100644 --- a/dialect_postgres.go +++ b/dialect_postgres.go @@ -120,6 +120,10 @@ func (s postgres) CurrentDatabase() (name string) { return } +func (s postgres) LastInsertIDOutputInterstitial(tableName, key string, columns []string) string { + return "" +} + func (s postgres) LastInsertIDReturningSuffix(tableName, key string) string { return fmt.Sprintf("RETURNING %v.%v", tableName, key) } diff --git a/dialects/mssql/mssql.go b/dialects/mssql/mssql.go index 8c2360fc..eb79f7e7 100644 --- a/dialects/mssql/mssql.go +++ b/dialects/mssql/mssql.go @@ -190,6 +190,14 @@ func (mssql) SelectFromDummyTable() string { return "" } +func (mssql) LastInsertIDOutputInterstitial(tableName, columnName string, columns []string) string { + if len(columns) == 0 { + // No OUTPUT to query + return "" + } + return fmt.Sprintf("OUTPUT Inserted.%v", columnName) +} + func (mssql) LastInsertIDReturningSuffix(tableName, columnName string) string { return "" } From d2007b3c826bf2f528d8dae0913f77cbac3ef7fd Mon Sep 17 00:00:00 2001 From: Devin Samarin Date: Thu, 17 Oct 2019 07:56:19 -0700 Subject: [PATCH 4/4] Describe name of field for invalid SQL datatypes (#2689) --- dialect_mysql.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dialect_mysql.go b/dialect_mysql.go index 5a1ad708..1addaf36 100644 --- a/dialect_mysql.go +++ b/dialect_mysql.go @@ -120,7 +120,7 @@ func (s *mysql) DataTypeOf(field *StructField) string { } if sqlType == "" { - panic(fmt.Sprintf("invalid sql type %s (%s) for mysql", dataValue.Type().Name(), dataValue.Kind().String())) + panic(fmt.Sprintf("invalid sql type %s (%s) in field %s for mysql", dataValue.Type().Name(), dataValue.Kind().String(), field.Name)) } if strings.TrimSpace(additionalType) == "" {