diff options
author | Michael Muré <batolettre@gmail.com> | 2021-04-17 17:40:11 +0200 |
---|---|---|
committer | Michael Muré <batolettre@gmail.com> | 2021-04-17 17:40:11 +0200 |
commit | 51a2c85954e77068c6afbb4ca54159086220aefd (patch) | |
tree | 9b424181369a67f69502a27186bd266a19a28506 | |
parent | 62fb09a53cc626ac581f33b466a1cdf14eb6ed89 (diff) | |
download | git-bug-51a2c85954e77068c6afbb4ca54159086220aefd.tar.gz |
make sure every text input is safe and validated
fix #630
-rw-r--r-- | api/graphql/resolvers/mutation.go | 39 | ||||
-rw-r--r-- | bridge/github/import.go | 27 | ||||
-rw-r--r-- | bridge/gitlab/import.go | 29 | ||||
-rw-r--r-- | bridge/jira/import.go | 31 | ||||
-rw-r--r-- | bridge/launchpad/import.go | 7 | ||||
-rw-r--r-- | bug/label.go | 9 | ||||
-rw-r--r-- | bug/op_create.go | 8 | ||||
-rw-r--r-- | bug/op_set_metadata.go | 11 | ||||
-rw-r--r-- | bug/op_set_title.go | 17 | ||||
-rw-r--r-- | cache/bug_cache.go | 2 | ||||
-rw-r--r-- | commands/add.go | 6 | ||||
-rw-r--r-- | commands/comment_add.go | 3 | ||||
-rw-r--r-- | commands/label_add.go | 3 | ||||
-rw-r--r-- | commands/label_rm.go | 3 | ||||
-rw-r--r-- | commands/title_edit.go | 3 | ||||
-rw-r--r-- | identity/version.go | 22 | ||||
-rw-r--r-- | termui/termui.go | 12 | ||||
-rw-r--r-- | util/text/transform.go | 30 | ||||
-rw-r--r-- | util/text/validate.go | 12 |
19 files changed, 147 insertions, 127 deletions
diff --git a/api/graphql/resolvers/mutation.go b/api/graphql/resolvers/mutation.go index 9cd936a6..00c9e3c1 100644 --- a/api/graphql/resolvers/mutation.go +++ b/api/graphql/resolvers/mutation.go @@ -5,11 +5,12 @@ import ( "time" "github.com/MichaelMure/git-bug/api/auth" - "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/api/graphql/graph" "github.com/MichaelMure/git-bug/api/graphql/models" "github.com/MichaelMure/git-bug/bug" "github.com/MichaelMure/git-bug/cache" + "github.com/MichaelMure/git-bug/entity" + "github.com/MichaelMure/git-bug/util/text" ) var _ graph.MutationResolver = &mutationResolver{} @@ -50,7 +51,12 @@ func (r mutationResolver) NewBug(ctx context.Context, input models.NewBugInput) return nil, err } - b, op, err := repo.NewBugRaw(author, time.Now().Unix(), input.Title, input.Message, input.Files, nil) + b, op, err := repo.NewBugRaw(author, + time.Now().Unix(), + text.CleanupOneLine(input.Title), + text.Cleanup(input.Message), + input.Files, + nil) if err != nil { return nil, err } @@ -73,7 +79,11 @@ func (r mutationResolver) AddComment(ctx context.Context, input models.AddCommen return nil, err } - op, err := b.AddCommentRaw(author, time.Now().Unix(), input.Message, input.Files, nil) + op, err := b.AddCommentRaw(author, + time.Now().Unix(), + text.Cleanup(input.Message), + input.Files, + nil) if err != nil { return nil, err } @@ -101,7 +111,13 @@ func (r mutationResolver) EditComment(ctx context.Context, input models.EditComm return nil, err } - op, err := b.EditCommentRaw(author, time.Now().Unix(), entity.Id(input.Target), input.Message, nil) + op, err := b.EditCommentRaw( + author, + time.Now().Unix(), + entity.Id(input.Target), + text.Cleanup(input.Message), + nil, + ) if err != nil { return nil, err } @@ -129,7 +145,13 @@ func (r mutationResolver) ChangeLabels(ctx context.Context, input *models.Change return nil, err } - results, op, err := b.ChangeLabelsRaw(author, time.Now().Unix(), input.Added, input.Removed, nil) + results, op, err := b.ChangeLabelsRaw( + author, + time.Now().Unix(), + text.CleanupOneLineArray(input.Added), + text.CleanupOneLineArray(input.Removed), + nil, + ) if err != nil { return nil, err } @@ -219,7 +241,12 @@ func (r mutationResolver) SetTitle(ctx context.Context, input models.SetTitleInp return nil, err } - op, err := b.SetTitleRaw(author, time.Now().Unix(), input.Title, nil) + op, err := b.SetTitleRaw( + author, + time.Now().Unix(), + text.CleanupOneLine(input.Title), + nil, + ) if err != nil { return nil, err } diff --git a/bridge/github/import.go b/bridge/github/import.go index bf43a877..f410cc65 100644 --- a/bridge/github/import.go +++ b/bridge/github/import.go @@ -211,17 +211,13 @@ func (gi *githubImporter) ensureIssue(ctx context.Context, repo *cache.RepoCache // if there are no issue edits then the issue struct holds the bug creation textInput = string(issue.Body) } - cleanText, err := text.Cleanup(textInput) - if err != nil { - return nil, err - } // create bug b, _, err = repo.NewBugRaw( author, issue.CreatedAt.Unix(), - title, // TODO: this is the *current* title, not the original one - cleanText, + text.CleanupOneLine(title), // TODO: this is the *current* title, not the original one + text.Cleanup(textInput), nil, map[string]string{ core.MetaKeyOrigin: target, @@ -269,7 +265,7 @@ func (gi *githubImporter) ensureTimelineItem(ctx context.Context, repo *cache.Re author, item.LabeledEvent.CreatedAt.Unix(), []string{ - string(item.LabeledEvent.Label.Name), + text.CleanupOneLine(string(item.LabeledEvent.Label.Name)), }, nil, map[string]string{metaKeyGithubId: id}, @@ -300,7 +296,7 @@ func (gi *githubImporter) ensureTimelineItem(ctx context.Context, repo *cache.Re item.UnlabeledEvent.CreatedAt.Unix(), nil, []string{ - string(item.UnlabeledEvent.Label.Name), + text.CleanupOneLine(string(item.UnlabeledEvent.Label.Name)), }, map[string]string{metaKeyGithubId: id}, ) @@ -382,7 +378,7 @@ func (gi *githubImporter) ensureTimelineItem(ctx context.Context, repo *cache.Re // The title provided by the GraphQL API actually consists of a space followed // by a zero width space (U+200B). This title would cause the NewBugRaw() // function to return an error: empty title. - title := string(item.RenamedTitleEvent.CurrentTitle) + title := text.CleanupOneLine(string(item.RenamedTitleEvent.CurrentTitle)) if title == " \u200b" { // U+200B == zero width space title = EmptyTitlePlaceholder } @@ -429,17 +425,12 @@ func (gi *githubImporter) ensureCommentEdit(ctx context.Context, repo *cache.Rep return nil } - cleanText, err := text.Cleanup(string(*edit.Diff)) - if err != nil { - return err - } - // comment edition op, err := b.EditCommentRaw( editor, edit.CreatedAt.Unix(), target, - cleanText, + text.Cleanup(string(*edit.Diff)), map[string]string{ metaKeyGithubId: parseId(edit.Id), }, @@ -476,16 +467,12 @@ func (gi *githubImporter) ensureComment(ctx context.Context, repo *cache.RepoCac // if there are not comment edits, then the comment struct holds the comment creation textInput = string(comment.Body) } - cleanText, err := text.Cleanup(textInput) - if err != nil { - return err - } // add comment operation op, err := b.AddCommentRaw( author, comment.CreatedAt.Unix(), - cleanText, + text.Cleanup(textInput), nil, map[string]string{ metaKeyGithubId: parseId(comment.Id), diff --git a/bridge/gitlab/import.go b/bridge/gitlab/import.go index 7939f4e4..cc99c12e 100644 --- a/bridge/gitlab/import.go +++ b/bridge/gitlab/import.go @@ -137,22 +137,12 @@ func (gi *gitlabImporter) ensureIssue(repo *cache.RepoCache, issue *gitlab.Issue return nil, err } - // if bug was never imported - cleanTitle, err := text.Cleanup(issue.Title) - if err != nil { - return nil, err - } - cleanDesc, err := text.Cleanup(issue.Description) - if err != nil { - return nil, err - } - - // create bug + // if bug was never imported, create bug b, _, err = repo.NewBugRaw( author, issue.CreatedAt.Unix(), - cleanTitle, - cleanDesc, + text.CleanupOneLine(issue.Title), + text.Cleanup(issue.Description), nil, map[string]string{ core.MetaKeyOrigin: target, @@ -238,7 +228,7 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n author, note.UpdatedAt.Unix(), firstComment.Id(), - issue.Description, + text.Cleanup(issue.Description), map[string]string{ metaKeyGitlabId: gitlabID, }, @@ -251,10 +241,7 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n } case NOTE_COMMENT: - cleanText, err := text.Cleanup(body) - if err != nil { - return err - } + cleanText := text.Cleanup(body) // if we didn't import the comment if errResolve == cache.ErrNoMatchingOp { @@ -312,7 +299,7 @@ func (gi *gitlabImporter) ensureNote(repo *cache.RepoCache, b *cache.BugCache, n op, err := b.SetTitleRaw( author, note.CreatedAt.Unix(), - body, + text.CleanupOneLine(body), map[string]string{ metaKeyGitlabId: gitlabID, }, @@ -361,7 +348,7 @@ func (gi *gitlabImporter) ensureLabelEvent(repo *cache.RepoCache, b *cache.BugCa _, err = b.ForceChangeLabelsRaw( author, labelEvent.CreatedAt.Unix(), - []string{labelEvent.Label.Name}, + []string{text.CleanupOneLine(labelEvent.Label.Name)}, nil, map[string]string{ metaKeyGitlabId: parseID(labelEvent.ID), @@ -373,7 +360,7 @@ func (gi *gitlabImporter) ensureLabelEvent(repo *cache.RepoCache, b *cache.BugCa author, labelEvent.CreatedAt.Unix(), nil, - []string{labelEvent.Label.Name}, + []string{text.CleanupOneLine(labelEvent.Label.Name)}, map[string]string{ metaKeyGitlabId: parseID(labelEvent.ID), }, diff --git a/bridge/jira/import.go b/bridge/jira/import.go index 00148bb6..d601febb 100644 --- a/bridge/jira/import.go +++ b/bridge/jira/import.go @@ -232,19 +232,11 @@ func (ji *jiraImporter) ensureIssue(repo *cache.RepoCache, issue Issue) (*cache. } if err == bug.ErrBugNotExist { - cleanText, err := text.Cleanup(string(issue.Fields.Description)) - if err != nil { - return nil, err - } - - // NOTE(josh): newlines in titles appears to be rare, but it has been seen - // in the wild. It does not appear to be allowed in the JIRA web interface. - title := strings.Replace(issue.Fields.Summary, "\n", "", -1) b, _, err = repo.NewBugRaw( author, issue.Fields.Created.Unix(), - title, - cleanText, + text.CleanupOneLine(issue.Fields.Summary), + text.Cleanup(issue.Fields.Description), nil, map[string]string{ core.MetaKeyOrigin: target, @@ -289,10 +281,7 @@ func (ji *jiraImporter) ensureComment(repo *cache.RepoCache, b *cache.BugCache, // We don't know the original text... we only have the updated text. cleanText = "" } else { - cleanText, err = text.Cleanup(string(item.Body)) - if err != nil { - return err - } + cleanText = text.Cleanup(item.Body) } // add comment operation @@ -340,15 +329,11 @@ func (ji *jiraImporter) ensureComment(repo *cache.RepoCache, b *cache.BugCache, } // comment edition - cleanText, err := text.Cleanup(string(item.Body)) - if err != nil { - return err - } op, err := b.EditCommentRaw( editor, item.Updated.Unix(), targetOpID, - cleanText, + text.Cleanup(item.Body), map[string]string{ metaKeyJiraId: derivedID, }, @@ -513,8 +498,8 @@ func (ji *jiraImporter) ensureChange(repo *cache.RepoCache, b *cache.BugCache, e op, err := b.ForceChangeLabelsRaw( author, entry.Created.Unix(), - addedLabels, - removedLabels, + text.CleanupOneLineArray(addedLabels), + text.CleanupOneLineArray(removedLabels), map[string]string{ metaKeyJiraId: entry.ID, metaKeyJiraDerivedId: derivedID, @@ -571,7 +556,7 @@ func (ji *jiraImporter) ensureChange(repo *cache.RepoCache, b *cache.BugCache, e op, err := b.SetTitleRaw( author, entry.Created.Unix(), - string(item.ToString), + text.CleanupOneLine(item.ToString), map[string]string{ metaKeyJiraId: entry.ID, metaKeyJiraDerivedId: derivedID, @@ -589,7 +574,7 @@ func (ji *jiraImporter) ensureChange(repo *cache.RepoCache, b *cache.BugCache, e op, err := b.EditCreateCommentRaw( author, entry.Created.Unix(), - string(item.ToString), + text.Cleanup(item.ToString), map[string]string{ metaKeyJiraId: entry.ID, metaKeyJiraDerivedId: derivedID, diff --git a/bridge/launchpad/import.go b/bridge/launchpad/import.go index 6b5667ba..01f6e53b 100644 --- a/bridge/launchpad/import.go +++ b/bridge/launchpad/import.go @@ -9,6 +9,7 @@ import ( "github.com/MichaelMure/git-bug/bug" "github.com/MichaelMure/git-bug/cache" "github.com/MichaelMure/git-bug/entity" + "github.com/MichaelMure/git-bug/util/text" ) type launchpadImporter struct { @@ -83,8 +84,8 @@ func (li *launchpadImporter) ImportAll(ctx context.Context, repo *cache.RepoCach b, _, err = repo.NewBugRaw( owner, createdAt.Unix(), - lpBug.Title, - lpBug.Description, + text.CleanupOneLine(lpBug.Title), + text.Cleanup(lpBug.Description), nil, map[string]string{ core.MetaKeyOrigin: target, @@ -133,7 +134,7 @@ func (li *launchpadImporter) ImportAll(ctx context.Context, repo *cache.RepoCach op, err := b.AddCommentRaw( owner, createdAt.Unix(), - lpMessage.Content, + text.Cleanup(lpMessage.Content), nil, map[string]string{ metaKeyLaunchpadID: lpMessage.ID, diff --git a/bug/label.go b/bug/label.go index 9d1e99a7..79b5f591 100644 --- a/bug/label.go +++ b/bug/label.go @@ -4,7 +4,6 @@ import ( "crypto/sha256" "fmt" "image/color" - "strings" fcolor "github.com/fatih/color" @@ -58,12 +57,8 @@ func (l Label) Validate() error { return fmt.Errorf("empty") } - if strings.Contains(str, "\n") { - return fmt.Errorf("should be a single line") - } - - if !text.Safe(str) { - return fmt.Errorf("not fully printable") + if !text.SafeOneLine(str) { + return fmt.Errorf("label has unsafe characters") } return nil diff --git a/bug/op_create.go b/bug/op_create.go index 75b60bd8..1d01020c 100644 --- a/bug/op_create.go +++ b/bug/op_create.go @@ -3,7 +3,6 @@ package bug import ( "encoding/json" "fmt" - "strings" "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/entity/dag" @@ -94,11 +93,8 @@ func (op *CreateOperation) Validate() error { if text.Empty(op.Title) { return fmt.Errorf("title is empty") } - if strings.Contains(op.Title, "\n") { - return fmt.Errorf("title should be a single line") - } - if !text.Safe(op.Title) { - return fmt.Errorf("title is not fully printable") + if !text.SafeOneLine(op.Title) { + return fmt.Errorf("title has unsafe characters") } if !text.Safe(op.Message) { diff --git a/bug/op_set_metadata.go b/bug/op_set_metadata.go index ca19a838..28496fd8 100644 --- a/bug/op_set_metadata.go +++ b/bug/op_set_metadata.go @@ -2,11 +2,13 @@ package bug import ( "encoding/json" + "fmt" "github.com/pkg/errors" "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" + "github.com/MichaelMure/git-bug/util/text" ) var _ Operation = &SetMetadataOperation{} @@ -43,6 +45,15 @@ func (op *SetMetadataOperation) Validate() error { return errors.Wrap(err, "target invalid") } + for key, val := range op.NewMetadata { + if !text.SafeOneLine(key) { + return fmt.Errorf("metadata key is unsafe") + } + if !text.Safe(val) { + return fmt.Errorf("metadata value is not fully printable") + } + } + return nil } diff --git a/bug/op_set_title.go b/bug/op_set_title.go index c6a26746..badd192c 100644 --- a/bug/op_set_title.go +++ b/bug/op_set_title.go @@ -3,7 +3,6 @@ package bug import ( "encoding/json" "fmt" - "strings" "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/identity" @@ -49,20 +48,12 @@ func (op *SetTitleOperation) Validate() error { return fmt.Errorf("title is empty") } - if strings.Contains(op.Title, "\n") { - return fmt.Errorf("title should be a single line") + if !text.SafeOneLine(op.Title) { + return fmt.Errorf("title has unsafe characters") } - if !text.Safe(op.Title) { - return fmt.Errorf("title should be fully printable") - } - - if strings.Contains(op.Was, "\n") { - return fmt.Errorf("previous title should be a single line") - } - - if !text.Safe(op.Was) { - return fmt.Errorf("previous title should be fully printable") + if !text.SafeOneLine(op.Was) { + return fmt.Errorf("previous title has unsafe characters") } return nil diff --git a/cache/bug_cache.go b/cache/bug_cache.go index bbe9830f..a248e872 100644 --- a/cache/bug_cache.go +++ b/cache/bug_cache.go @@ -235,6 +235,7 @@ func (c *BugCache) SetTitleRaw(author *IdentityCache, unixTime int64, title stri return op, c.notifyUpdated() } +// Convenience function to edit the body of a bug (the first comment) func (c *BugCache) EditCreateComment(body string) (*bug.EditCommentOperation, error) { author, err := c.repoCache.GetUserIdentity() if err != nil { @@ -244,6 +245,7 @@ func (c *BugCache) EditCreateComment(body string) (*bug.EditCommentOperation, er return c.EditCreateCommentRaw(author, time.Now().Unix(), body, nil) } +// Convenience function to edit the body of a bug (the first comment) func (c *BugCache) EditCreateCommentRaw(author *IdentityCache, unixTime int64, body string, metadata map[string]string) (*bug.EditCommentOperation, error) { c.mu.Lock() op, err := bug.EditCreateComment(c.bug, author.Identity, unixTime, body) diff --git a/commands/add.go b/commands/add.go index 17fbbc93..ad3b164b 100644 --- a/commands/add.go +++ b/commands/add.go @@ -4,6 +4,7 @@ import ( "github.com/spf13/cobra" "github.com/MichaelMure/git-bug/input" + "github.com/MichaelMure/git-bug/util/text" ) type addOptions struct { @@ -60,7 +61,10 @@ func runAdd(env *Env, opts addOptions) error { } } - b, _, err := env.backend.NewBug(opts.title, opts.message) + b, _, err := env.backend.NewBug( + text.CleanupOneLine(opts.title), + text.Cleanup(opts.message), + ) if err != nil { return err } diff --git a/commands/comment_add.go b/commands/comment_add.go index 6caf4943..438e983a 100644 --- a/commands/comment_add.go +++ b/commands/comment_add.go @@ -5,6 +5,7 @@ import ( _select "github.com/MichaelMure/git-bug/commands/select" "github.com/MichaelMure/git-bug/input" + "github.com/MichaelMure/git-bug/util/text" ) type commentAddOptions struct { @@ -62,7 +63,7 @@ func runCommentAdd(env *Env, opts commentAddOptions, args []string) error { } } - _, err = b.AddComment(opts.message) + _, err = b.AddComment(text.Cleanup(opts.message)) if err != nil { return err } diff --git a/commands/label_add.go b/commands/label_add.go index ce971f04..30c0d77f 100644 --- a/commands/label_add.go +++ b/commands/label_add.go @@ -4,6 +4,7 @@ import ( "github.com/spf13/cobra" _select "github.com/MichaelMure/git-bug/commands/select" + "github.com/MichaelMure/git-bug/util/text" ) func newLabelAddCommand() *cobra.Command { @@ -30,7 +31,7 @@ func runLabelAdd(env *Env, args []string) error { added := args - changes, _, err := b.ChangeLabels(added, nil) + changes, _, err := b.ChangeLabels(text.CleanupOneLineArray(added), nil) for _, change := range changes { env.out.Println(change) diff --git a/commands/label_rm.go b/commands/label_rm.go index f59ec02d..9fb3e096 100644 --- a/commands/label_rm.go +++ b/commands/label_rm.go @@ -4,6 +4,7 @@ import ( "github.com/spf13/cobra" _select "github.com/MichaelMure/git-bug/commands/select" + "github.com/MichaelMure/git-bug/util/text" ) func newLabelRmCommand() *cobra.Command { @@ -30,7 +31,7 @@ func runLabelRm(env *Env, args []string) error { removed := args - changes, _, err := b.ChangeLabels(nil, removed) + changes, _, err := b.ChangeLabels(nil, text.CleanupOneLineArray(removed)) for _, change := range changes { env.out.Println(change) diff --git a/commands/title_edit.go b/commands/title_edit.go index 3750f2cc..455c9384 100644 --- a/commands/title_edit.go +++ b/commands/title_edit.go @@ -5,6 +5,7 @@ import ( _select "github.com/MichaelMure/git-bug/commands/select" "github.com/MichaelMure/git-bug/input" + "github.com/MichaelMure/git-bug/util/text" ) type titleEditOptions struct { @@ -58,7 +59,7 @@ func runTitleEdit(env *Env, opts titleEditOptions, args []string) error { env.err.Println("No change, aborting.") } - _, err = b.SetTitle(opts.title) + _, err = b.SetTitle(text.CleanupOneLine(opts.title)) if err != nil { return err } diff --git a/identity/version.go b/identity/version.go index 1c35831e..9a52d089 100644 --- a/identity/version.go +++ b/identity/version.go @@ -4,7 +4,6 @@ import ( "crypto/rand" "encoding/json" "fmt" - "strings" "time" "github.com/pkg/errors" @@ -186,25 +185,16 @@ func (v *version) Validate() error { if text.Empty(v.name) && text.Empty(v.login) { return fmt.Errorf("either name or login should be set") } - if strings.Contains(v.name, "\n") { - return fmt.Errorf("name should be a single line") - } - if !text.Safe(v.name) { - return fmt.Errorf("name is not fully printable") + if !text.SafeOneLine(v.name) { + return fmt.Errorf("name has unsafe characters") } - if strings.Contains(v.login, "\n") { - return fmt.Errorf("login should be a single line") - } - if !text.Safe(v.login) { - return fmt.Errorf("login is not fully printable") + if !text.SafeOneLine(v.login) { + return fmt.Errorf("login has unsafe characters") } - if strings.Contains(v.email, "\n") { - return fmt.Errorf("email should be a single line") - } - if !text.Safe(v.email) { - return fmt.Errorf("email is not fully printable") + if !text.SafeOneLine(v.email) { + return fmt.Errorf("email has unsafe characters") } if v.avatarURL != "" && !text.ValidUrl(v.avatarURL) { diff --git a/termui/termui.go b/termui/termui.go index ec5387a4..3e7f43b9 100644 --- a/termui/termui.go +++ b/termui/termui.go @@ -11,6 +11,7 @@ import ( "github.com/MichaelMure/git-bug/entity" "github.com/MichaelMure/git-bug/input" "github.com/MichaelMure/git-bug/query" + "github.com/MichaelMure/git-bug/util/text" ) var errTerminateMainloop = errors.New("terminate gocui mainloop") @@ -199,7 +200,10 @@ func newBugWithEditor(repo *cache.RepoCache) error { return errTerminateMainloop } else { - b, _, err = repo.NewBug(title, message) + b, _, err = repo.NewBug( + text.CleanupOneLine(title), + text.Cleanup(message), + ) if err != nil { return err } @@ -235,7 +239,7 @@ func addCommentWithEditor(bug *cache.BugCache) error { if err == input.ErrEmptyMessage { ui.msgPopup.Activate(msgPopupErrorTitle, "Empty message, aborting.") } else { - _, err := bug.AddComment(message) + _, err := bug.AddComment(text.Cleanup(message)) if err != nil { return err } @@ -270,7 +274,7 @@ func editCommentWithEditor(bug *cache.BugCache, target entity.Id, preMessage str } else if message == preMessage { ui.msgPopup.Activate(msgPopupErrorTitle, "No changes found, aborting.") } else { - _, err := bug.EditComment(target, message) + _, err := bug.EditComment(target, text.Cleanup(message)) if err != nil { return err } @@ -307,7 +311,7 @@ func setTitleWithEditor(bug *cache.BugCache) error { } else if title == snap.Title { ui.msgPopup.Activate(msgPopupErrorTitle, "No change, aborting.") } else { - _, err := bug.SetTitle(title) + _, err := bug.SetTitle(text.CleanupOneLine(title)) if err != nil { return err } diff --git a/util/text/transform.go b/util/text/transform.go index 59dc4e03..395a57be 100644 --- a/util/text/transform.go +++ b/util/text/transform.go @@ -8,7 +8,7 @@ import ( "golang.org/x/text/transform" ) -func Cleanup(text string) (string, error) { +func Cleanup(text string) string { // windows new line, Github, really ? text = strings.Replace(text, "\r\n", "\n", -1) @@ -23,9 +23,33 @@ func Cleanup(text string) (string, error) { })) sanitized, _, err := transform.String(t, text) if err != nil { - return "", err + // transform.String should never return an error as our transformer doesn't returns one. + // Confirmed with fuzzing. + panic(err) } // trim extra new line not displayed in the github UI but still present in the data - return strings.TrimSpace(sanitized), nil + return strings.TrimSpace(sanitized) +} + +func CleanupOneLine(text string) string { + // remove all unicode control characters *including* + // '\n', '\r' and '\t' + t := runes.Remove(runes.Predicate(unicode.IsControl)) + sanitized, _, err := transform.String(t, text) + if err != nil { + // transform.String should never return an error as our transformer doesn't returns one. + // Confirmed with fuzzing. + panic(err) + } + + // trim extra new line not displayed in the github UI but still present in the data + return strings.TrimSpace(sanitized) +} + +func CleanupOneLineArray(texts []string) []string { + for i := range texts { + texts[i] = CleanupOneLine(texts[i]) + } + return texts } diff --git a/util/text/validate.go b/util/text/validate.go index 51e94fb4..4c3f7065 100644 --- a/util/text/validate.go +++ b/util/text/validate.go @@ -33,6 +33,18 @@ func Safe(s string) bool { return true } +// Safe will tell if a character in the string is considered unsafe +// Currently trigger on all unicode control character +func SafeOneLine(s string) bool { + for _, r := range s { + if unicode.IsControl(r) { + return false + } + } + + return true +} + // ValidUrl will tell if the string contains what seems to be a valid URL func ValidUrl(s string) bool { if strings.Contains(s, "\n") { |