From 5b63a44a9577b98a918eacda26e67ac1710985de Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 10 Nov 2021 16:10:09 +0100 Subject: [PATCH 1/5] Fix offBy1 errors - Partially resolves #17596 - Resolve errors from go-critic `offBy1: Index() can return -1; maybe you wanted to do Index()+1`. --- cmd/docs.go | 8 +++++++- models/migrations/migrations.go | 13 ++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/cmd/docs.go b/cmd/docs.go index 52233c7ac872d..72970516c95f1 100644 --- a/cmd/docs.go +++ b/cmd/docs.go @@ -43,7 +43,13 @@ func runDocs(ctx *cli.Context) error { // Clean up markdown. The following bug was fixed in v2, but is present in v1. // It affects markdown output (even though the issue is referring to man pages) // https://github.com/urfave/cli/issues/1040 - docs = docs[strings.Index(docs, "#"):] + firstHashtagIndex := strings.Index(docs, "#") + + // Only replace docs, when firstHashtagIndex isn't -1, + // as -1 shouldn't be passed into a slice. + if firstHashtagIndex != -1 { + docs = docs[firstHashtagIndex:] + } } out := os.Stdout diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 2686dfb3cff96..6c5274145ef0a 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -7,6 +7,7 @@ package migrations import ( "context" + "errors" "fmt" "os" "reflect" @@ -791,8 +792,18 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin } tableSQL := string(res[0]["sql"]) + // Get the index after the column definitions. + endColumnsIndex := strings.Index(tableSQL, "(") + + // If endColumnsIndex is -1, which shouldn't be passed + // into a slice. We should return a error, as that also + // means that the substring wasn't found. + if endColumnsIndex == -1 { + return errors.New("Couldn't get index after column defintions") + } + // Separate out the column definitions - tableSQL = tableSQL[strings.Index(tableSQL, "("):] + tableSQL = tableSQL[endColumnsIndex:] // Remove the required columnNames for _, name := range columnNames { From 9b8c07dcebdbff6af00c1639a3918870f160a38f Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 10 Nov 2021 16:47:42 +0100 Subject: [PATCH 2/5] Match golang spec --- models/migrations/migrations.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 6c5274145ef0a..c8771b541d841 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -799,7 +799,7 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin // into a slice. We should return a error, as that also // means that the substring wasn't found. if endColumnsIndex == -1 { - return errors.New("Couldn't get index after column defintions") + return errors.New("couldn't get index after column defintions") } // Separate out the column definitions From 1a525b1ed5f9e3d7b89501dc97f675f9193df2d7 Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 10 Nov 2021 16:59:56 +0100 Subject: [PATCH 3/5] Remove comments --- cmd/docs.go | 2 -- models/migrations/migrations.go | 3 --- 2 files changed, 5 deletions(-) diff --git a/cmd/docs.go b/cmd/docs.go index 72970516c95f1..a92b290f4713d 100644 --- a/cmd/docs.go +++ b/cmd/docs.go @@ -45,8 +45,6 @@ func runDocs(ctx *cli.Context) error { // https://github.com/urfave/cli/issues/1040 firstHashtagIndex := strings.Index(docs, "#") - // Only replace docs, when firstHashtagIndex isn't -1, - // as -1 shouldn't be passed into a slice. if firstHashtagIndex != -1 { docs = docs[firstHashtagIndex:] } diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index c8771b541d841..35322b8141510 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -795,9 +795,6 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin // Get the index after the column definitions. endColumnsIndex := strings.Index(tableSQL, "(") - // If endColumnsIndex is -1, which shouldn't be passed - // into a slice. We should return a error, as that also - // means that the substring wasn't found. if endColumnsIndex == -1 { return errors.New("couldn't get index after column defintions") } From e0ab55278e31d80f9eedb92d6b5f8a6cc370e1fd Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 11 Nov 2021 02:43:15 +0800 Subject: [PATCH 4/5] Update migrations.go --- models/migrations/migrations.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 35322b8141510..dce7dc314f1f2 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -792,15 +792,14 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin } tableSQL := string(res[0]["sql"]) - // Get the index after the column definitions. - endColumnsIndex := strings.Index(tableSQL, "(") - - if endColumnsIndex == -1 { - return errors.New("couldn't get index after column defintions") + // Get the string offset for column defintions: `CREATE TABLE ( column-definitions... )` + columnDefinitionsIndex := strings.Index(tableSQL, "(") + if columnDefinitionsIndex == -1 { + return errors.New("couldn't find column defintions") } // Separate out the column definitions - tableSQL = tableSQL[endColumnsIndex:] + tableSQL = tableSQL[columnDefinitionsIndex:] // Remove the required columnNames for _, name := range columnNames { From 741bb6f431f296591931a865d475f74ce798d5df Mon Sep 17 00:00:00 2001 From: Gusted Date: Wed, 10 Nov 2021 23:55:48 +0100 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: delvh --- cmd/docs.go | 2 +- models/migrations/migrations.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/docs.go b/cmd/docs.go index a92b290f4713d..073c57497305f 100644 --- a/cmd/docs.go +++ b/cmd/docs.go @@ -45,7 +45,7 @@ func runDocs(ctx *cli.Context) error { // https://github.com/urfave/cli/issues/1040 firstHashtagIndex := strings.Index(docs, "#") - if firstHashtagIndex != -1 { + if firstHashtagIndex > 0 { docs = docs[firstHashtagIndex:] } } diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index dce7dc314f1f2..c0d8f111d3764 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -792,10 +792,10 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin } tableSQL := string(res[0]["sql"]) - // Get the string offset for column defintions: `CREATE TABLE ( column-definitions... )` + // Get the string offset for column definitions: `CREATE TABLE ( column-definitions... )` columnDefinitionsIndex := strings.Index(tableSQL, "(") - if columnDefinitionsIndex == -1 { - return errors.New("couldn't find column defintions") + if columnDefinitionsIndex < 0 { + return errors.New("couldn't find column definitions") } // Separate out the column definitions