0

better chat message sanitization (#1266)

* strip <p> in chat sanitization, keep the content

* update sanitization tests

* update tests

* rm <p></p> comparison for empty messages
This commit is contained in:
Meisam 2021-07-28 00:26:27 +02:00 committed by GitHub
parent 92284f6ca1
commit 109d2669ab
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 12 additions and 10 deletions

View File

@ -73,7 +73,7 @@ func (m *MessageEvent) RenderAndSanitizeMessageBody() {
// Empty will return if this message's contents is empty. // Empty will return if this message's contents is empty.
func (m *MessageEvent) Empty() bool { func (m *MessageEvent) Empty() bool {
return m.Body == "" || m.Body == "<p></p>" return m.Body == ""
} }
// RenderBody will render markdown to html without any sanitization. // RenderBody will render markdown to html without any sanitization.
@ -136,7 +136,9 @@ func sanitize(raw string) string {
p.AddTargetBlankToFullyQualifiedLinks(true) p.AddTargetBlankToFullyQualifiedLinks(true)
// Allow breaks // Allow breaks
p.AllowElements("br", "p") p.AllowElements("br")
p.AllowElementsContent("p")
// Allow img tags from the the local emoji directory only // Allow img tags from the the local emoji directory only
p.AllowAttrs("src").Matching(regexp.MustCompile(`(?i)^/img/emoji`)).OnElements("img") p.AllowAttrs("src").Matching(regexp.MustCompile(`(?i)^/img/emoji`)).OnElements("img")

View File

@ -19,11 +19,11 @@ func TestRenderAndSanitize(t *testing.T) {
<script src="http://hackers.org/hack.js"></script> <script src="http://hackers.org/hack.js"></script>
` `
expected := `<p>Test one two three! I go to <a href="http://yahoo.com" rel="nofollow noreferrer noopener" target="_blank">http://yahoo.com</a> and search for <em>sports</em> and <strong>answers</strong>. expected := `Test one two three! I go to <a href="http://yahoo.com" rel="nofollow noreferrer noopener" target="_blank">http://yahoo.com</a> and search for <em>sports</em> and <strong>answers</strong>.
Here is an iframe </p> Here is an iframe
blah blah blah blah blah blah
<p><a href="http://owncast.online" rel="nofollow noreferrer noopener" target="_blank">test link</a> <a href="http://owncast.online" rel="nofollow noreferrer noopener" target="_blank">test link</a>
<img class="emoji" src="/img/emoji/bananadance.gif"></p>` <img class="emoji" src="/img/emoji/bananadance.gif">`
result := events.RenderAndSanitize(messageContent) result := events.RenderAndSanitize(messageContent)
if result != expected { if result != expected {
@ -34,7 +34,7 @@ blah blah blah
// Test to make sure we block remote images in chat messages. // Test to make sure we block remote images in chat messages.
func TestBlockRemoteImages(t *testing.T) { func TestBlockRemoteImages(t *testing.T) {
messageContent := `<img src="https://via.placeholder.com/img/emoji/350x150"> test ![](https://via.placeholder.com/img/emoji/350x150)` messageContent := `<img src="https://via.placeholder.com/img/emoji/350x150"> test ![](https://via.placeholder.com/img/emoji/350x150)`
expected := `<p> test </p>` expected := `test`
result := events.RenderAndSanitize(messageContent) result := events.RenderAndSanitize(messageContent)
if result != expected { if result != expected {
@ -45,7 +45,7 @@ func TestBlockRemoteImages(t *testing.T) {
// Test to make sure emoji images are allowed in chat messages. // Test to make sure emoji images are allowed in chat messages.
func TestAllowEmojiImages(t *testing.T) { func TestAllowEmojiImages(t *testing.T) {
messageContent := `<img alt=":beerparrot:" title=":beerparrot:" src="/img/emoji/beerparrot.gif"> test ![](/img/emoji/beerparrot.gif)` messageContent := `<img alt=":beerparrot:" title=":beerparrot:" src="/img/emoji/beerparrot.gif"> test ![](/img/emoji/beerparrot.gif)`
expected := `<p><img alt=":beerparrot:" title=":beerparrot:" src="/img/emoji/beerparrot.gif"> test <img src="/img/emoji/beerparrot.gif"></p>` expected := `<img alt=":beerparrot:" title=":beerparrot:" src="/img/emoji/beerparrot.gif"> test <img src="/img/emoji/beerparrot.gif">`
result := events.RenderAndSanitize(messageContent) result := events.RenderAndSanitize(messageContent)
if result != expected { if result != expected {

View File

@ -27,7 +27,7 @@ test('can fetch chat messages', async (done) => {
.auth('admin', 'abc123') .auth('admin', 'abc123')
.expect(200); .expect(200);
const expectedBody = `<p>${testMessage.body}</p>` const expectedBody = `${testMessage.body}`
const message = res.body.filter(function (msg) { const message = res.body.filter(function (msg) {
return msg.body === expectedBody return msg.body === expectedBody
})[0]; })[0];

View File

@ -34,7 +34,7 @@ test('verify message has become hidden', async (done) => {
.auth('admin', 'abc123') .auth('admin', 'abc123')
const message = res.body.filter(obj => { const message = res.body.filter(obj => {
return obj.body === `<p>${testVisibilityMessage.body}</p>`; return obj.body === `${testVisibilityMessage.body}`;
}); });
expect(message.length).toBe(1); expect(message.length).toBe(1);
expect(message[0].hiddenAt).toBeTruthy(); expect(message[0].hiddenAt).toBeTruthy();