MM-21116: Improve channelSearch SQL query generation (#13371)

* MM-21116: Improve channelSearch SQL query generation

The likeTerm from buildFulltextClause was being built inefficient using strings.
We change it to use squirrel to build the query.

And while at it, we also change the len check with checking with
an empty string which is more idiomatic. Both compile to the same
code, so there is no difference performance wise.

* Use a better variable name
Этот коммит содержится в:
Agniva De Sarker
2019-12-12 23:13:33 +05:30
коммит произвёл GitHub
родитель 4d3f7ae06c
Коммит 7d499d2750
2 изменённых файлов: 26 добавлений и 4 удалений

Просмотреть файл

@@ -2253,11 +2253,15 @@ func (s SqlChannelStore) channelSearchQuery(term string, opts store.ChannelSearc
}
likeClause, likeTerm := s.buildLIKEClause(term, "c.Name, c.DisplayName, c.Purpose")
if len(likeTerm) > 0 {
likeClause = strings.ReplaceAll(likeClause, ":LikeTerm", "'"+likeTerm+"'")
if likeTerm != "" {
likeClause = strings.ReplaceAll(likeClause, ":LikeTerm", "?")
fulltextClause, fulltextTerm := s.buildFulltextClause(term, "c.Name, c.DisplayName, c.Purpose")
fulltextClause = strings.ReplaceAll(fulltextClause, ":FulltextTerm", "'"+fulltextTerm+"'")
query = query.Where("(" + likeClause + " OR " + fulltextClause + ")")
fulltextClause = strings.ReplaceAll(fulltextClause, ":FulltextTerm", "?")
query = query.Where(sq.Or{
sq.Expr(likeClause, likeTerm, likeTerm, likeTerm), // Keep the number of likeTerms same as the number
// of columns (c.Name, c.DisplayName, c.Purpose)
sq.Expr(fulltextClause, fulltextTerm),
})
}
if len(opts.ExcludeChannelNames) > 0 {

Просмотреть файл

@@ -8,8 +8,10 @@ import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/mattermost/mattermost-server/v5/model"
"github.com/mattermost/mattermost-server/v5/store"
"github.com/mattermost/mattermost-server/v5/store/storetest"
)
@@ -17,6 +19,22 @@ func TestChannelStore(t *testing.T) {
StoreTestWithSqlSupplier(t, storetest.TestChannelStore)
}
func TestChannelSearchQuerySQLInjection(t *testing.T) {
for _, st := range storeTypes {
t.Run(st.Name, func(t *testing.T) {
s := &SqlChannelStore{
SqlStore: st.SqlSupplier,
}
opts := store.ChannelSearchOpts{}
builder := s.channelSearchQuery("'or'1'=sleep(3))); -- -", opts, false)
query, _, err := builder.ToSql()
require.Nil(t, err)
assert.NotContains(t, query, "sleep")
})
}
}
func TestChannelStoreInternalDataTypes(t *testing.T) {
t.Run("NewChannelMemberFromModel", func(t *testing.T) { testNewChannelMemberFromModel(t) })
t.Run("ChannelMemberWithSchemeRolesToModel", func(t *testing.T) { testChannelMemberWithSchemeRolesToModel(t) })