From 51a2c85954e77068c6afbb4ca54159086220aefd Mon Sep 17 00:00:00 2001 From: Michael Muré Date: Sat, 17 Apr 2021 17:40:11 +0200 Subject: make sure every text input is safe and validated fix #630 --- bridge/github/import.go | 27 +++++++-------------------- bridge/gitlab/import.go | 29 ++++++++--------------------- bridge/jira/import.go | 31 ++++++++----------------------- bridge/launchpad/import.go | 7 ++++--- 4 files changed, 27 insertions(+), 67 deletions(-) (limited to 'bridge') 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, -- cgit