a
/
gosora
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

security: stop exempting video sites in frame-src on pages which don't have video embeds 

perf: reduce allocs in action posts
perf: avoid allocing a map and slice for single reply posts when it is liked and the post also has an attachment.
perf: use hookgen for topic_reply_row_assign hook.
perf: stop unnecessarily indirecting r in topic_reply_row_assign hook.
perf: try optimising reqUserList logic in Topic.Replies()
This commit is contained in:
Azareal 2020-06-09 12:04:58 +10:00
parent 019efc8c62
commit 304c246cb2
4 changed files with 134 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
common/pages.go
View File

@ -44,6 +44,7 @@ type Header struct {
GoogSiteVerify string
IsoCode string
LooseCSP bool
ExternalMedia bool
//StartedAt time.Time
StartedAt int64
Elapsed1 string

25
common/parser.go
View File

@ -483,10 +483,15 @@ func (ps *ParseSettings) CopyPtr() *ParseSettings {
return n
}
func ParseMessage(msg string, sectionID int, sectionType string, settings *ParseSettings, user *User) string {
msg, _ = ParseMessage2(msg,sectionID,sectionType,settings,user)
return msg
}
// TODO: Write a test for this
// TODO: We need a lot more hooks here. E.g. To add custom media types and handlers.
// TODO: Use templates to reduce the amount of boilerplate?
func ParseMessage(msg string, sectionID int, sectionType string, settings *ParseSettings, user *User) string {
func ParseMessage2(msg string, sectionID int, sectionType string, settings *ParseSettings, user *User) (string,bool) {
if settings == nil {
settings = DefaultParseSettings
}
@ -510,7 +515,7 @@ func ParseMessage(msg string, sectionID int, sectionType string, settings *Parse
wordFilters, err := WordFilters.GetAll()
if err != nil {
LogError(err)
return ""
return "", false
}
for _, f := range wordFilters {
msg = strings.Replace(msg, f.Find, f.Replace, -1)
@ -519,13 +524,14 @@ func ParseMessage(msg string, sectionID int, sectionType string, settings *Parse
if len(msg) < 2 {
msg = strings.Replace(msg, "\n", "<br>", -1)
msg = GetHookTable().Sshook("parse_assign", msg)
return msg
return msg, false
}
// Search for URLs, mentions and hashlinks in the messages...
var sb strings.Builder
lastItem := 0
i := 0
var externalHead bool
//var c bool
//fmt.Println("msg:", "'"+msg+"'")
for ; len(msg) > i; i++ {
@ -775,6 +781,12 @@ func ParseMessage(msg string, sectionID int, sectionType string, settings *Parse
i += urlLen
lastItem = i
continue
case ERawExternal:
sb.WriteString(media.Body)
i += urlLen
lastItem = i
externalHead = true
continue
case ENone:
// Do nothing
// TODO: Add support for media plugins
@ -820,7 +832,7 @@ func ParseMessage(msg string, sectionID int, sectionType string, settings *Parse
msg = strings.Replace(msg, "\n", "<br>", -1)
msg = GetHookTable().Sshook("parse_assign", msg)
return msg
return msg, externalHead
}
// 6, 7, 8, 6, 2, 7
@ -993,6 +1005,7 @@ type MediaEmbed struct {
const (
ENone = iota
ERaw
ERawExternal
EImage
AImage
AVideo
@ -1073,7 +1086,7 @@ func parseMediaString(data string, settings *ParseSettings) (media MediaEmbed, o
if strings.HasSuffix(host, ".youtube.com") && path == "/watch" {
video, ok := query["v"]
if ok && len(video) >= 1 && video[0] != "" {
media.Type = ERaw
media.Type = ERawExternal
// TODO: Filter the URL to make sure no nasties end up in there
media.Body = "<iframe class='postIframe'src='https://www.youtube-nocookie.com/embed/" + video[0] + "'frameborder=0 allowfullscreen></iframe>"
return media, true
@ -1081,7 +1094,7 @@ func parseMediaString(data string, settings *ParseSettings) (media MediaEmbed, o
} else if strings.HasPrefix(host, "www.nicovideo.jp") && strings.HasPrefix(path, "/watch/sm") {
vid, err := strconv.ParseInt(strings.TrimPrefix(path, "/watch/sm"), 10, 64)
if err == nil {
media.Type = ERaw
media.Type = ERawExternal
media.Body = "<iframe class='postIframe'src='https://embed.nicovideo.jp/watch/sm"+strconv.FormatInt(vid, 10)+"?jsapi=1&amp;playerId=1'frameborder=0 allowfullscreen></iframe>"
return media, true
}

174
common/topic.go
View File

@ -415,8 +415,8 @@ func handleTopicReplies(umap map[int]struct{}, uid, tid int) error {
}
defer rows.Close()
var createdBy int
for rows.Next() {
var createdBy int
err := rows.Scan(&createdBy)
if err != nil {
return err
@ -705,12 +705,16 @@ func (ru *ReplyUser) Init3(u *User, tu *TopicUser) (group *Group, err error) {
switch action {
case "lock":
ru.ActionIcon = lockai
action = "topic.action_topic_lock"
case "unlock":
ru.ActionIcon = unlockai
action = "topic.action_topic_unlock"
case "stick":
ru.ActionIcon = stickai
action = "topic.action_topic_stick"
case "unstick":
ru.ActionIcon = unstickai
action = "topic.action_topic_unstick"
case "move":
if len(aarr) == 2 {
fid, _ := strconv.Atoi(aarr[1])
@ -725,14 +729,14 @@ func (ru *ReplyUser) Init3(u *User, tu *TopicUser) (group *Group, err error) {
ru.ActionType = p.GetTmplPhrasef("topic.action_topic_default", ru.ActionType)
return postGroup, nil
}
ru.ActionType = p.GetTmplPhrasef("topic.action_topic_"+action, ru.UserLink, ru.CreatedByName)
ru.ActionType = p.GetTmplPhrasef(action, ru.UserLink, ru.CreatedByName)
}
return postGroup, nil
}
// TODO: Factor TopicUser into a *Topic and *User, as this starting to become overly complicated x.x
func (t *TopicUser) Replies(offset int /*pFrag int, */, user *User) (rlist []*ReplyUser /*, ogdesc string*/, err error) {
func (t *TopicUser) Replies(offset int /*pFrag int, */, user *User) (rlist []*ReplyUser /*, ogdesc string*/, externalHead bool, err error) {
var likedMap, attachMap map[int]int
var likedQueryList, attachQueryList []int
@ -770,7 +774,7 @@ func (t *TopicUser) Replies(offset int /*pFrag int, */, user *User) (rlist []*Re
parseSettings = user.ParseSettings
}
r.ContentHtml = ParseMessage(r.Content, t.ParentID, "forums", parseSettings, user)
r.ContentHtml, externalHead = ParseMessage2(r.Content, t.ParentID, "forums", parseSettings, user)
// TODO: Do this more efficiently by avoiding the allocations entirely in ParseMessage, if there's nothing to do.
if r.ContentHtml == r.Content {
r.ContentHtml = r.Content
@ -803,7 +807,7 @@ func (t *TopicUser) Replies(offset int /*pFrag int, */, user *User) (rlist []*Re
parseSettings = user.ParseSettings
}
r.ContentHtml = ParseMessage(r.Content, t.ParentID, "forums", parseSettings, user)
r.ContentHtml, externalHead = ParseMessage2(r.Content, t.ParentID, "forums", parseSettings, user)
// TODO: Do this more efficiently by avoiding the allocations entirely in ParseMessage, if there's nothing to do.
if r.ContentHtml == r.Content {
r.ContentHtml = r.Content
@ -818,19 +822,24 @@ func (t *TopicUser) Replies(offset int /*pFrag int, */, user *User) (rlist []*Re
//log.Print("reply cached serve")
r := &ReplyUser{ /*ClassName: "", */ Reply: *re, CreatedByName: ruser.Name, UserLink: ruser.Link, Avatar: ruser.Avatar, MicroAvatar: ruser.MicroAvatar /*URLPrefix: ruser.URLPrefix, URLName: ruser.URLName, */, Group: ruser.Group, Level: ruser.Level, Tag: ruser.Tag}
if err = rf3(r); err != nil {
return nil, err
return nil, externalHead, err
}
if r.LikeCount > 0 && user.Liked > 0 {
likedMap = map[int]int{r.ID: len(rlist)}
likedMap = map[int]int{r.ID: 0}
likedQueryList = []int{r.ID}
}
if user.Perms.EditReply && r.AttachCount > 0 {
attachMap = map[int]int{r.ID: len(rlist)}
attachQueryList = []int{r.ID}
if likedMap == nil {
attachMap = map[int]int{r.ID: 0}
attachQueryList = []int{r.ID}
} else {
attachMap = likedMap
attachQueryList = likedQueryList
}
}
hTbl.VhookNoRet("topic_reply_row_assign", &r)
H_topic_reply_row_assign_hook(hTbl, r)
// TODO: Use a pointer instead to make it easier to abstract this loop? What impact would this have on escape analysis?
rlist = []*ReplyUser{r}
//log.Printf("r: %d-%d", r.ID, len(rlist)-1)
@ -856,7 +865,7 @@ func (t *TopicUser) Replies(offset int /*pFrag int, */, user *User) (rlist []*Re
}
}
hTbl.VhookNoRet("topic_reply_row_assign", &r)
H_topic_reply_row_assign_hook(hTbl, r)
// TODO: Use a pointer instead to make it easier to abstract this loop? What impact would this have on escape analysis?
rlist = append(rlist, r)
//log.Printf("r: %d-%d", r.ID, len(rlist)-1)
@ -864,47 +873,47 @@ func (t *TopicUser) Replies(offset int /*pFrag int, */, user *User) (rlist []*Re
if !user.Perms.ViewIPs && ruser != nil {
rows, err := topicStmts.getReplies3.Query(t.ID, offset, Config.ItemsPerPage)
if err != nil {
return nil, err
return nil, externalHead, err
}
defer rows.Close()
for rows.Next() {
r := &ReplyUser{Avatar: ruser.Avatar, MicroAvatar: ruser.MicroAvatar, UserLink: ruser.Link, CreatedByName: ruser.Name, Group: ruser.Group, Level: ruser.Level}
err := rows.Scan(&r.ID, &r.Content, &r.CreatedBy, &r.CreatedAt, &r.LastEdit, &r.LastEditBy, &r.LikeCount, &r.AttachCount, &r.ActionType)
if err != nil {
return nil, err
return nil, externalHead, err
}
if err = rf3(r); err != nil {
return nil, err
return nil, externalHead, err
}
rf2(r)
}
if err = rows.Err(); err != nil {
return nil, err
return nil, externalHead, err
}
} else if user.Perms.ViewIPs {
rows, err := topicStmts.getReplies.Query(t.ID, offset, Config.ItemsPerPage)
if err != nil {
return nil, err
return nil, externalHead, err
}
defer rows.Close()
for rows.Next() {
r := &ReplyUser{}
err := rows.Scan(&r.ID, &r.Content, &r.CreatedBy, &r.CreatedAt, &r.LastEdit, &r.LastEditBy, &r.Avatar, &r.CreatedByName, &r.Group /*&r.URLPrefix, &r.URLName,*/, &r.Level, &r.IP, &r.LikeCount, &r.AttachCount, &r.ActionType)
if err != nil {
return nil, err
return nil, externalHead, err
}
if err = rf(r); err != nil {
return nil, err
return nil, externalHead, err
}
rf2(r)
}
if err = rows.Err(); err != nil {
return nil, err
return nil, externalHead, err
}
} else if t.PostCount >= 20 {
rows, err := topicStmts.getReplies3.Query(t.ID, offset, Config.ItemsPerPage)
if err != nil {
return nil, err
return nil, externalHead, err
}
defer rows.Close()
reqUserList := make(map[int]bool)
@ -912,75 +921,110 @@ func (t *TopicUser) Replies(offset int /*pFrag int, */, user *User) (rlist []*Re
r := &ReplyUser{}
err := rows.Scan(&r.ID, &r.Content, &r.CreatedBy, &r.CreatedAt, &r.LastEdit, &r.LastEditBy /*&r.URLPrefix, &r.URLName,*/, &r.LikeCount, &r.AttachCount, &r.ActionType)
if err != nil {
return nil, err
return nil, externalHead, err
}
if r.CreatedBy != t.CreatedBy && r.CreatedBy != user.ID {
reqUserList[r.CreatedBy] = true
}
}
if err = rows.Err(); err != nil {
return nil, err
return nil, externalHead, err
}
var userList map[int]*User
if len(reqUserList) > 0 {
// Convert the user ID map to a slice, then bulk load the users
idSlice := make([]int, len(reqUserList))
var i int
for userID := range reqUserList {
idSlice[i] = userID
i++
}
userList, err = Users.BulkGetMap(idSlice)
if err != nil {
return nil, nil // TODO: Implement this!
}
}
for _, r := range rlist {
var u *User
if r.CreatedBy == t.CreatedBy {
r.CreatedByName = t.CreatedByName
r.Avatar = t.Avatar
r.MicroAvatar = t.MicroAvatar
r.Group = t.Group
r.Level = t.Level
} else {
if r.CreatedBy == user.ID {
u = user
} else {
u = userList[r.CreatedBy]
if len(reqUserList) == 1 {
var uitem *User
for uid, _ := range reqUserList {
uitem, err = Users.Get(uid)
if err != nil {
return nil, externalHead, nil // TODO: Implement this!
}
r.CreatedByName = u.Name
r.Avatar = u.Avatar
r.MicroAvatar = u.MicroAvatar
r.Group = u.Group
r.Level = u.Level
}
if err = rf(r); err != nil {
return nil, err
for _, r := range rlist {
if r.CreatedBy == t.CreatedBy {
r.CreatedByName = t.CreatedByName
r.Avatar = t.Avatar
r.MicroAvatar = t.MicroAvatar
r.Group = t.Group
r.Level = t.Level
} else {
var u *User
if r.CreatedBy == user.ID {
u = user
} else {
u = uitem
}
r.CreatedByName = u.Name
r.Avatar = u.Avatar
r.MicroAvatar = u.MicroAvatar
r.Group = u.Group
r.Level = u.Level
}
if err = rf(r); err != nil {
return nil, externalHead, err
}
rf2(r)
}
} else {
var userList map[int]*User
if len(reqUserList) > 0 {
// Convert the user ID map to a slice, then bulk load the users
idSlice := make([]int, len(reqUserList))
var i int
for userID := range reqUserList {
idSlice[i] = userID
i++
}
userList, err = Users.BulkGetMap(idSlice)
if err != nil {
return nil, externalHead, nil // TODO: Implement this!
}
}
for _, r := range rlist {
if r.CreatedBy == t.CreatedBy {
r.CreatedByName = t.CreatedByName
r.Avatar = t.Avatar
r.MicroAvatar = t.MicroAvatar
r.Group = t.Group
r.Level = t.Level
} else {
var u *User
if r.CreatedBy == user.ID {
u = user
} else {
u = userList[r.CreatedBy]
}
r.CreatedByName = u.Name
r.Avatar = u.Avatar
r.MicroAvatar = u.MicroAvatar
r.Group = u.Group
r.Level = u.Level
}
if err = rf(r); err != nil {
return nil, externalHead, err
}
rf2(r)
}
rf2(r)
}
} else {
rows, err := topicStmts.getReplies2.Query(t.ID, offset, Config.ItemsPerPage)
if err != nil {
return nil, err
return nil, externalHead, err
}
defer rows.Close()
for rows.Next() {
r := &ReplyUser{}
err := rows.Scan(&r.ID, &r.Content, &r.CreatedBy, &r.CreatedAt, &r.LastEdit, &r.LastEditBy, &r.Avatar, &r.CreatedByName, &r.Group /*&r.URLPrefix, &r.URLName,*/, &r.Level, &r.LikeCount, &r.AttachCount, &r.ActionType)
if err != nil {
return nil, err
return nil, externalHead, err
}
if err = rf(r); err != nil {
return nil, err
return nil, externalHead, err
}
rf2(r)
}
if err = rows.Err(); err != nil {
return nil, err
return nil, externalHead, err
}
}
}
@ -989,7 +1033,7 @@ func (t *TopicUser) Replies(offset int /*pFrag int, */, user *User) (rlist []*Re
if user.Liked > 0 && len(likedQueryList) > 0 /*&& user.LastLiked <= time.Now()*/ {
eids, err := Likes.BulkExists(likedQueryList, user.ID, "replies")
if err != nil {
return nil, err
return nil, externalHead, err
}
for _, eid := range eids {
rlist[likedMap[eid]].Liked = true
@ -1000,7 +1044,7 @@ func (t *TopicUser) Replies(offset int /*pFrag int, */, user *User) (rlist []*Re
//log.Printf("attachQueryList: %+v\n", attachQueryList)
amap, err := Attachments.BulkMiniGetList("replies", attachQueryList)
if err != nil && err != sql.ErrNoRows {
return nil, err
return nil, externalHead, err
}
//log.Printf("amap: %+v\n", amap)
//log.Printf("attachMap: %+v\n", attachMap)
@ -1015,7 +1059,7 @@ func (t *TopicUser) Replies(offset int /*pFrag int, */, user *User) (rlist []*Re
//hTbl.VhookNoRet("topic_reply_end", &rlist)
return rlist, nil
return rlist, externalHead, nil
}
// TODO: Test this

7
routes/topic.go
View File

@ -94,7 +94,7 @@ func ViewTopic(w http.ResponseWriter, r *http.Request, user *c.User, h *c.Header
}
// TODO: Cache ContentHTML when possible?
topic.ContentHTML = c.ParseMessage(topic.Content, topic.ParentID, "forums", parseSettings, user)
topic.ContentHTML, h.ExternalMedia = c.ParseMessage2(topic.Content, topic.ParentID, "forums", parseSettings, user)
// TODO: Do this more efficiently by avoiding the allocations entirely in ParseMessage, if there's nothing to do.
if topic.ContentHTML == topic.Content {
topic.ContentHTML = topic.Content
@ -152,13 +152,16 @@ func ViewTopic(w http.ResponseWriter, r *http.Request, user *c.User, h *c.Header
if strings.HasPrefix(r.URL.Fragment, "post-") {
pFrag, _ = strconv.Atoi(strings.TrimPrefix(r.URL.Fragment, "post-"))
}*/
rlist, err := topic.Replies(offset /* pFrag,*/, user)
rlist, externalHead, err := topic.Replies(offset /* pFrag,*/, user)
if err == sql.ErrNoRows {
return c.LocalError("Bad Page. Some of the posts may have been deleted or you got here by directly typing in the page number.", w, r, user)
} else if err != nil {
return c.InternalError(err, w, r)
}
tpage.ItemList = rlist
if externalHead {
h.ExternalMedia = true
}
}
h.Zone = "view_topic"