Support restricted words (#257)

* prepared interfaces and naive implementation for restricted words

* fixed whitespace tokenizer last token handling

* removed unnecessary check in whitespace tokenizer last token handling

* removed unnecessary interfaces, added matcher to service

* now using more explicit naming

* added config parameter/env variable to configure restricted words
provided simple matcher and lister implementation into service

* fixed linter warnings

* fixed linter warnings (part 2)

* added functional tests for rest api
moved check for restricted words into validation stage

* fixed missing error check in rest private test

* fixed failed test that expected validation in specific order

* fixed linter warning

* removed unnecessary 'else'

* removed redundant check from 'DataStore.EditComment'

* replaced exact matching for restricted words with wildcard matching

* fixed race condition when creating wildcardTrie
removed unnecessary wildcard masks
refactored tests in a table format

* improved documentation for restricted words param

* removed wildcardTrie caching as a premature optimization
removed unnecessary validation method ValidateCommentText
moved restricted words validation into service methods that create and update comment

* fixed error name due to linter warning
This commit is contained in:
Aleksandr Melnikov
2019-01-21 16:48:35 -06:00
committed by Umputun
parent 2c988aa226
commit ab6c9a515c
8 changed files with 350 additions and 33 deletions
+1
View File
@@ -130,6 +130,7 @@ _this is the recommended way to run remark42_
| max-votes | MAX_VOTES | `-1` | votes limit per comment, `-1` - unlimited |
| low-score | LOW_SCORE | `-5` | low score threshold |
| critical-score | CRITICAL_SCORE | `-10` | critical score threshold |
| restricted-words | RESTRICTED_WORDS | | words banned in comments (can use `*`), _multi_ |
| edit-time | EDIT_TIME | `5m` | edit window |
| read-age | READONLY_AGE | | read-only age of comments, days |
| img-proxy | IMG_PROXY | `false` | enable http->https proxy for images |
+23 -21
View File
@@ -12,7 +12,7 @@ import (
"syscall"
"time"
bolt "github.com/coreos/bbolt"
"github.com/coreos/bbolt"
log "github.com/go-pkgz/lgr"
auth_cache "github.com/patrickmn/go-cache"
"github.com/pkg/errors"
@@ -44,20 +44,21 @@ type ServerCommand struct {
Notify NotifyGroup `group:"notify" namespace:"notify" env-namespace:"NOTIFY"`
SSL SSLGroup `group:"ssl" namespace:"ssl" env-namespace:"SSL"`
Sites []string `long:"site" env:"SITE" default:"remark" description:"site names" env-delim:","`
AdminPasswd string `long:"admin-passwd" env:"ADMIN_PASSWD" default:"" description:"admin basic auth password"`
BackupLocation string `long:"backup" env:"BACKUP_PATH" default:"./var/backup" description:"backups location"`
MaxBackupFiles int `long:"max-back" env:"MAX_BACKUP_FILES" default:"10" description:"max backups to keep"`
ImageProxy bool `long:"img-proxy" env:"IMG_PROXY" description:"enable image proxy"`
MaxCommentSize int `long:"max-comment" env:"MAX_COMMENT_SIZE" default:"2048" description:"max comment size"`
MaxVotes int `long:"max-votes" env:"MAX_VOTES" default:"-1" description:"maximum number of votes per comment"`
LowScore int `long:"low-score" env:"LOW_SCORE" default:"-5" description:"low score threshold"`
CriticalScore int `long:"critical-score" env:"CRITICAL_SCORE" default:"-10" description:"critical score threshold"`
ReadOnlyAge int `long:"read-age" env:"READONLY_AGE" default:"0" description:"read-only age of comments, days"`
EditDuration time.Duration `long:"edit-time" env:"EDIT_TIME" default:"5m" description:"edit window"`
Port int `long:"port" env:"REMARK_PORT" default:"8080" description:"port"`
WebRoot string `long:"web-root" env:"REMARK_WEB_ROOT" default:"./web" description:"web root directory"`
UpdateLimit float64 `long:"update-limit" env:"UPDATE_LIMIT" default:"0.5" description:"updates/sec limit"`
Sites []string `long:"site" env:"SITE" default:"remark" description:"site names" env-delim:","`
AdminPasswd string `long:"admin-passwd" env:"ADMIN_PASSWD" default:"" description:"admin basic auth password"`
BackupLocation string `long:"backup" env:"BACKUP_PATH" default:"./var/backup" description:"backups location"`
MaxBackupFiles int `long:"max-back" env:"MAX_BACKUP_FILES" default:"10" description:"max backups to keep"`
ImageProxy bool `long:"img-proxy" env:"IMG_PROXY" description:"enable image proxy"`
MaxCommentSize int `long:"max-comment" env:"MAX_COMMENT_SIZE" default:"2048" description:"max comment size"`
MaxVotes int `long:"max-votes" env:"MAX_VOTES" default:"-1" description:"maximum number of votes per comment"`
LowScore int `long:"low-score" env:"LOW_SCORE" default:"-5" description:"low score threshold"`
CriticalScore int `long:"critical-score" env:"CRITICAL_SCORE" default:"-10" description:"critical score threshold"`
ReadOnlyAge int `long:"read-age" env:"READONLY_AGE" default:"0" description:"read-only age of comments, days"`
EditDuration time.Duration `long:"edit-time" env:"EDIT_TIME" default:"5m" description:"edit window"`
Port int `long:"port" env:"REMARK_PORT" default:"8080" description:"port"`
WebRoot string `long:"web-root" env:"REMARK_WEB_ROOT" default:"./web" description:"web root directory"`
UpdateLimit float64 `long:"update-limit" env:"UPDATE_LIMIT" default:"0.5" description:"updates/sec limit"`
RestrictedWords []string `long:"restricted-words" env:"RESTRICTED_WORDS" default:"" description:"words prohibited to use in comments" env-delim:","`
Auth struct {
TTL struct {
@@ -211,12 +212,13 @@ func (s *ServerCommand) newServerApp() (*serverApp, error) {
}
dataService := &service.DataStore{
Interface: storeEngine,
EditDuration: s.EditDuration,
AdminStore: adminStore,
MaxCommentSize: s.MaxCommentSize,
MaxVotes: s.MaxVotes,
TitleExtractor: service.NewTitleExtractor(http.Client{Timeout: time.Second * 5}),
Interface: storeEngine,
EditDuration: s.EditDuration,
AdminStore: adminStore,
MaxCommentSize: s.MaxCommentSize,
MaxVotes: s.MaxVotes,
TitleExtractor: service.NewTitleExtractor(http.Client{Timeout: time.Second * 5}),
RestrictedWordsMatcher: service.NewRestrictedWordsMatcher(service.StaticRestrictedWordsLister{Words: s.RestrictedWords}),
}
loadingCache, err := s.makeCache()
+8
View File
@@ -57,6 +57,10 @@ func (s *Rest) createCommentCtrl(w http.ResponseWriter, r *http.Request) {
}
id, err := s.DataService.Create(comment)
if err == service.ErrRestrictedWordsFound {
rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "invalid comment")
return
}
if err != nil {
rest.SendErrorJSON(w, r, http.StatusInternalServerError, err, "can't save comment")
return
@@ -121,6 +125,10 @@ func (s *Rest) updateCommentCtrl(w http.ResponseWriter, r *http.Request) {
}
res, err := s.DataService.EditComment(locator, id, editReq)
if err == service.ErrRestrictedWordsFound {
rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "invalid comment")
return
}
if err != nil {
rest.SendErrorJSON(w, r, http.StatusBadRequest, err, "can't update comment")
return
+43
View File
@@ -101,6 +101,24 @@ func TestRest_CreateTooBig(t *testing.T) {
assert.Equal(t, "can't bind comment", c["details"])
}
func TestRest_CreateWithRestrictedWord(t *testing.T) {
ts, _, teardown := startupT(t)
defer teardown()
badComment := fmt.Sprintf(`{"text": "What the duck is that?", "locator":{"url": "https://radio-t.com/blah1", "site": "radio-t"}}`)
resp, err := post(t, ts.URL+"/api/v1/comment", badComment)
assert.Nil(t, err)
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
b, err := ioutil.ReadAll(resp.Body)
assert.Nil(t, err)
c := R.JSON{}
err = json.Unmarshal(b, &c)
assert.Nil(t, err)
assert.Equal(t, "comment contains restricted words", c["error"])
assert.Equal(t, "invalid comment", c["details"])
}
func TestRest_CreateRejected(t *testing.T) {
ts, _, teardown := startupT(t)
@@ -258,6 +276,31 @@ func TestRest_UpdateNotOwner(t *testing.T) {
assert.Equal(t, 400, b.StatusCode, string(body), "update is not json")
}
func TestRest_UpdateWithRestrictedWords(t *testing.T) {
ts, _, teardown := startupT(t)
defer teardown()
c1 := store.Comment{Text: "What the quack is that?", ParentID: "p1",
Locator: store.Locator{SiteID: "radio-t", URL: "https://radio-t.com/blah1"}}
id := addComment(t, c1, ts)
client := http.Client{}
req, err := http.NewRequest(http.MethodPut, ts.URL+"/api/v1/comment/"+id+"?site=radio-t&url=https://radio-t.com/blah1",
strings.NewReader(`{"text":"What the duck is that?", "summary":"my edit"}`))
assert.Nil(t, err)
req.Header.Add("X-JWT", devToken)
b, err := client.Do(req)
assert.Nil(t, err)
body, err := ioutil.ReadAll(b.Body)
assert.Nil(t, err)
c := R.JSON{}
err = json.Unmarshal(body, &c)
assert.Nil(t, err)
assert.Equal(t, 400, b.StatusCode, string(body))
assert.Equal(t, "comment contains restricted words", c["error"])
assert.Equal(t, "invalid comment", c["details"])
}
func TestRest_Vote(t *testing.T) {
ts, _, teardown := startupT(t)
defer teardown()
+7 -5
View File
@@ -180,13 +180,15 @@ func startupT(t *testing.T) (ts *httptest.Server, srv *Rest, teardown func()) {
require.Nil(t, err)
adminStore := adminstore.NewStaticStore("123456", []string{"a1", "a2"}, "admin@remark-42.com")
restrictedWordsMatcher := service.NewRestrictedWordsMatcher(service.StaticRestrictedWordsLister{Words: []string{"duck"}})
dataStore := &service.DataStore{
Interface: b,
EditDuration: 5 * time.Minute,
MaxCommentSize: 4000,
AdminStore: adminStore,
MaxVotes: service.UnlimitedVotes,
Interface: b,
EditDuration: 5 * time.Minute,
MaxCommentSize: 4000,
AdminStore: adminStore,
MaxVotes: service.UnlimitedVotes,
RestrictedWordsMatcher: restrictedWordsMatcher,
}
srv = &Rest{
@@ -0,0 +1,173 @@
package service
import (
"fmt"
"strings"
"unicode"
"unicode/utf8"
)
// RestrictedWordsLister provides restricted words in comments per site
type RestrictedWordsLister interface {
List(siteID string) (restricted []string, err error)
}
// StaticRestrictedWordsLister provides same restricted words in comments for every site
type StaticRestrictedWordsLister struct {
Words []string
}
// List provides restricted words in comments (ignores siteID)
func (l StaticRestrictedWordsLister) List(siteID string) (restricted []string, err error) {
return l.Words, nil
}
// RestrictedWordsMatcher matches comment text against restricted words
type RestrictedWordsMatcher struct {
lister RestrictedWordsLister
}
// NewRestrictedWordsMatcher creates new RestrictedWordsMatcher using provided RestrictedWordsLister
func NewRestrictedWordsMatcher(lister RestrictedWordsLister) *RestrictedWordsMatcher {
return &RestrictedWordsMatcher{lister: lister}
}
// Match matches comment text against restricted words for specified site
func (m *RestrictedWordsMatcher) Match(siteID string, text string) bool {
tokens := m.tokenize(text)
restrictedWords, err := m.lister.List(siteID)
if err != nil {
fmt.Printf("failed to get restricted patterns for site %s: %v", siteID, err)
return false
}
trie := newWildcardTrie(restrictedWords...)
for _, token := range tokens {
if trie.check(token) {
return true
}
}
return false
}
func (m *RestrictedWordsMatcher) tokenize(text string) []string {
tokens := make([]string, 0, 10) // accumulator for tokens
word := false // flag shows if current range is word
start := 0 // beginning of the current range
for pos, r := range text {
if unicode.IsLetter(r) || unicode.IsNumber(r) {
if !word {
// everything from start to pos - 1 is not a word, so reset start and start word tracking
start = pos
word = true
}
continue
}
if word && start < pos {
// everything from start to pos - 1 is a word, so add it as a token and reset start
tokens = append(tokens, strings.ToLower(text[start:pos]))
start = pos
}
// exited the word
word = false
}
// since we append tokens when we already left the word (on next iteration),
// we need to do it manually for the last iteration
if word {
tokens = append(tokens, strings.ToLower(text[start:]))
}
return tokens
}
type wildcardTrie struct {
terminal bool
children map[rune]*wildcardTrie
}
func newWildcardTrie(patterns ...string) *wildcardTrie {
trie := &wildcardTrie{terminal: false, children: make(map[rune]*wildcardTrie)}
for _, p := range patterns {
trie.addPattern(p)
}
return trie
}
func (trie *wildcardTrie) addPattern(pattern string) {
// since pattern matching algorithm is recursive we do not allow long patterns
if utf8.RuneCountInString(pattern) < 1 || utf8.RuneCountInString(pattern) > 64 {
fmt.Printf("[WARN] invalid pattern length '%s': actual - %d, min allowed - 1, max allowed - 64", pattern, utf8.RuneCountInString(pattern))
return
}
node := trie
for _, r := range strings.ToLower(strings.TrimSpace(pattern)) {
if childNode, exists := node.children[r]; exists {
node = childNode
continue
}
childNode := newWildcardTrie()
node.children[r] = childNode
node = childNode
}
node.terminal = true
}
// check tests if any pattern stored in trie matches the token. Recursive. Max depth is longest pattern in trie.
func (trie *wildcardTrie) check(token string) bool {
if len(token) == 0 {
if trie.terminal {
return true
}
if childNode, exists := trie.children['*']; exists && childNode.terminal {
return true
}
return false
}
r, width := utf8.DecodeRuneInString(token)
if childNode, exists := trie.children[r]; exists {
if childNode.check(token[width:]) {
return true
}
}
if childNode, exists := trie.children['*']; exists {
if childNode.terminal {
return true
}
if childNode.checkAllSuffixes(token) {
return true
}
}
return false
}
func (trie *wildcardTrie) checkAllSuffixes(token string) bool {
suffix := token
for {
if len(suffix) == 0 {
return false
}
if trie.check(suffix) {
return true
}
_, width := utf8.DecodeRuneInString(suffix)
suffix = suffix[width:]
}
}
@@ -0,0 +1,76 @@
package service
import (
"github.com/stretchr/testify/assert"
"testing"
)
func TestMatcher_Tokenize(t *testing.T) {
matcher := NewRestrictedWordsMatcher(StaticRestrictedWordsLister{})
tbl := []struct {
input string
output []string
}{
{
" word0 word1 word2, word3,,, !word4 !word5? word6-word7 word8",
[]string{"word0", "word1", "word2", "word3", "word4", "word5", "word6", "word7", "word8"},
},
{"русский 中文 française ไทย", []string{"русский", "中文", "française", "ไทย"}},
{"word", []string{"word"}},
{"", []string{}},
{"\t\t\n\t \n\t \r\n \t ,,, !#$%^&*()", []string{}},
{"👍", []string{}},
}
for _, td := range tbl {
tokens := matcher.tokenize(td.input)
assert.Equal(t, td.output, tokens, "unexpected result for input '%v'", td.input)
}
}
func TestWildcardTrie_Check(t *testing.T) {
tbl := []struct {
input []string
match []string
nomatch []string
}{
{[]string{"abc", "abb", "aab"}, []string{"abc", "abb", "aab"}, []string{"aaa", "aaaa", "a", "ab"}},
{[]string{"abc", "*ck", "*z"}, []string{"abc", "duck", "quack", "ck", "xyz"}, []string{"quacker", "buzzer"}},
{[]string{"abc", "du*", "c*"}, []string{"abc", "duck", "dungeon", "du", "cup"}, []string{"bbc", "ddu", "scuba"}},
{[]string{"abc", "*uc*", "*x*"}, []string{"abc", "duck", "stuck", "uc", "wwxww", "xww", "wwx"}, []string{"bbc", "duke"}},
{[]string{"abc", "d*k", "st*ck"}, []string{"abc", "duck", "dk", "stck", "stuck", "stiiick"}, []string{"bbc", "adka", "st", "ck"}},
{[]string{"abc", "*a*a*"}, []string{"abc", "safari", "banana", "aa"}, []string{"bbc", "car", "a"}},
{
[]string{"ложить", "при*", "*ий", "*бег*", "про*жа", "*ไ*ย*", "*請*请*"},
[]string{"ложить", "приклад", "ихний", "прибегать", "пропажа", "ไทย", "ทไย", "ไยท", "請問请问"},
[]string{"положить", "гранпри", "бийск", "請", "ยไท"},
},
}
for _, td := range tbl {
n := newWildcardTrie(td.input...)
for _, token := range td.match {
assert.True(t, n.check(token), "should match token '%s' for restricted words '%v'", token, td.input)
}
for _, token := range td.nomatch {
assert.False(t, n.check(token), "should not match token '%s' for restricted words '%v'", token, td.input)
}
}
}
func TestMatcher_MatchIfContainsRestrictedWords(t *testing.T) {
matcher := NewRestrictedWordsMatcher(StaticRestrictedWordsLister{[]string{"duck"}})
text := "What the duck it that?"
assert.True(t, matcher.Match("fakeID", text))
}
func TestMatcher_DoNotMatchIfNoRestrictedWords(t *testing.T) {
matcher := NewRestrictedWordsMatcher(StaticRestrictedWordsLister{[]string{"quack"}})
text := "What the duck it that?"
assert.False(t, matcher.Match("fakeID", text))
}
+19 -7
View File
@@ -7,8 +7,8 @@ import (
log "github.com/go-pkgz/lgr"
"github.com/google/uuid"
multierror "github.com/hashicorp/go-multierror"
cache "github.com/patrickmn/go-cache"
"github.com/hashicorp/go-multierror"
"github.com/patrickmn/go-cache"
"github.com/pkg/errors"
"github.com/umputun/remark/backend/app/store"
@@ -19,11 +19,12 @@ import (
// DataStore wraps store.Interface with additional methods
type DataStore struct {
engine.Interface
EditDuration time.Duration
AdminStore admin.Store
MaxCommentSize int
MaxVotes int
TitleExtractor *TitleExtractor
EditDuration time.Duration
AdminStore admin.Store
MaxCommentSize int
MaxVotes int
TitleExtractor *TitleExtractor
RestrictedWordsMatcher *RestrictedWordsMatcher
// granular locks
scopedLocks struct {
@@ -60,6 +61,9 @@ const maxLastCommentsReply = 1000
// UnlimitedVotes doesn't restrict MaxVotes
const UnlimitedVotes = -1
// ErrRestrictedWordsFound returned in case comment text contains restricted words
var ErrRestrictedWordsFound = errors.New("comment contains restricted words")
// Create prepares comment and forward to Interface.Create
func (s *DataStore) Create(comment store.Comment) (commentID string, err error) {
@@ -67,6 +71,10 @@ func (s *DataStore) Create(comment store.Comment) (commentID string, err error)
return "", errors.Wrap(err, "failed to prepare comment")
}
if s.RestrictedWordsMatcher != nil && s.RestrictedWordsMatcher.Match(comment.Locator.SiteID, comment.Text) {
return "", ErrRestrictedWordsFound
}
// keep input title and set to extracted if missing
if s.TitleExtractor != nil && comment.PostTitle == "" {
if title, err := s.TitleExtractor.Get(comment.Locator.URL); err == nil {
@@ -195,6 +203,10 @@ func (s *DataStore) EditComment(locator store.Locator, commentID string, req Edi
return comment, s.Delete(locator, commentID, store.SoftDelete)
}
if s.RestrictedWordsMatcher != nil && s.RestrictedWordsMatcher.Match(comment.Locator.SiteID, req.Text) {
return comment, ErrRestrictedWordsFound
}
comment.Text = req.Text
comment.Orig = req.Orig
comment.Edit = &store.Edit{