From 24f9e1122111a73fbd49c46e0f137ddb0f5d45eb Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Mon, 22 Nov 2021 11:49:11 +0100 Subject: [PATCH] Fix image description unnecessarily html-escaping innocent characters (#321) * implement SanitizeCaption function * tidy up text test setup --- internal/processing/media/create.go | 2 +- internal/processing/media/update.go | 2 +- internal/text/caption.go | 29 ++++++++++ internal/text/caption_test.go | 82 +++++++++++++++++++++++++++++ internal/text/common_test.go | 26 --------- internal/text/formatter_test.go | 26 ++++++++- internal/text/link_test.go | 24 --------- internal/text/markdown_test.go | 26 --------- internal/text/plain_test.go | 26 --------- 9 files changed, 138 insertions(+), 105 deletions(-) create mode 100644 internal/text/caption.go create mode 100644 internal/text/caption_test.go diff --git a/internal/processing/media/create.go b/internal/processing/media/create.go index 0783bfae8..adc44a4ea 100644 --- a/internal/processing/media/create.go +++ b/internal/processing/media/create.go @@ -56,7 +56,7 @@ func (p *processor) Create(ctx context.Context, account *gtsmodel.Account, form CreatedAt: time.Now(), UpdatedAt: time.Now(), AccountID: account.ID, - Description: text.RemoveHTML(form.Description), + Description: text.SanitizeCaption(form.Description), FileMeta: gtsmodel.FileMeta{ Focus: gtsmodel.Focus{ X: focusx, diff --git a/internal/processing/media/update.go b/internal/processing/media/update.go index b3455bc91..42e050121 100644 --- a/internal/processing/media/update.go +++ b/internal/processing/media/update.go @@ -45,7 +45,7 @@ func (p *processor) Update(ctx context.Context, account *gtsmodel.Account, media } if form.Description != nil { - attachment.Description = text.RemoveHTML(*form.Description) + attachment.Description = text.SanitizeCaption(*form.Description) if err := p.db.UpdateByPrimaryKey(ctx, attachment); err != nil { return nil, gtserror.NewErrorInternalError(fmt.Errorf("database error updating description: %s", err)) } diff --git a/internal/text/caption.go b/internal/text/caption.go new file mode 100644 index 000000000..d1af33e53 --- /dev/null +++ b/internal/text/caption.go @@ -0,0 +1,29 @@ +/* + GoToSocial + Copyright (C) 2021 GoToSocial Authors admin@gotosocial.org + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU Affero General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Affero General Public License for more details. + + You should have received a copy of the GNU Affero General Public License + along with this program. If not, see . +*/ + +package text + +// SanitizeCaption runs image captions (or indeed any plain text) through basic sanitization. +// It returns plain text rather than HTML, in contrast to other functions in this package. +func SanitizeCaption(in string) string { + content := preformat(in) + + content = RemoveHTML(content) + + return postformat(content) +} diff --git a/internal/text/caption_test.go b/internal/text/caption_test.go new file mode 100644 index 000000000..794c82bf5 --- /dev/null +++ b/internal/text/caption_test.go @@ -0,0 +1,82 @@ +/* + GoToSocial + Copyright (C) 2021 GoToSocial Authors admin@gotosocial.org + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU Affero General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Affero General Public License for more details. + + You should have received a copy of the GNU Affero General Public License + along with this program. If not, see . +*/ + +package text_test + +import ( + "testing" + + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/text" +) + +type CaptionTestSuite struct { + suite.Suite +} + +func (suite *CaptionTestSuite) TestSanitizeCaption1() { + dodgyCaption := "this is just a normal caption ;)" + sanitized := text.SanitizeCaption(dodgyCaption) + suite.Equal("this is just a normal caption ;)", sanitized) +} + +func (suite *CaptionTestSuite) TestSanitizeCaption2() { + dodgyCaption := "here's a LOUD caption" + sanitized := text.SanitizeCaption(dodgyCaption) + suite.Equal("here's a LOUD caption", sanitized) +} + +func (suite *CaptionTestSuite) TestSanitizeCaption3() { + dodgyCaption := "" + sanitized := text.SanitizeCaption(dodgyCaption) + suite.Equal("", sanitized) +} + +func (suite *CaptionTestSuite) TestSanitizeCaption4() { + dodgyCaption := ` + + +here is +a multi line +caption +with some newlines + + + +` + sanitized := text.SanitizeCaption(dodgyCaption) + suite.Equal("here is\na multi line\ncaption\nwith some newlines", sanitized) +} + +func (suite *CaptionTestSuite) TestSanitizeCaption5() { + // html-escaped: " hello world" + dodgyCaption := `<script>console.log('aha!')</script> hello world` + sanitized := text.SanitizeCaption(dodgyCaption) + suite.Equal("hello world", sanitized) +} + +func (suite *CaptionTestSuite) TestSanitizeCaption6() { + // html-encoded: " hello world" + dodgyCaption := `<script>console.log('aha!')</script> hello world` + sanitized := text.SanitizeCaption(dodgyCaption) + suite.Equal("hello world", sanitized) +} + +func TestCaptionTestSuite(t *testing.T) { + suite.Run(t, new(CaptionTestSuite)) +} diff --git a/internal/text/common_test.go b/internal/text/common_test.go index 19851956e..9d61b6113 100644 --- a/internal/text/common_test.go +++ b/internal/text/common_test.go @@ -25,8 +25,6 @@ import ( "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" - "github.com/superseriousbusiness/gotosocial/internal/text" - "github.com/superseriousbusiness/gotosocial/testrig" ) const ( @@ -74,30 +72,6 @@ type CommonTestSuite struct { TextStandardTestSuite } -func (suite *CommonTestSuite) SetupSuite() { - suite.testTokens = testrig.NewTestTokens() - suite.testClients = testrig.NewTestClients() - suite.testApplications = testrig.NewTestApplications() - suite.testUsers = testrig.NewTestUsers() - suite.testAccounts = testrig.NewTestAccounts() - suite.testAttachments = testrig.NewTestAttachments() - suite.testStatuses = testrig.NewTestStatuses() - suite.testTags = testrig.NewTestTags() - suite.testMentions = testrig.NewTestMentions() -} - -func (suite *CommonTestSuite) SetupTest() { - suite.config = testrig.NewTestConfig() - suite.db = testrig.NewTestDB() - suite.formatter = text.NewFormatter(suite.config, suite.db) - - testrig.StandardDBSetup(suite.db, nil) -} - -func (suite *CommonTestSuite) TearDownTest() { - testrig.StandardDBTeardown(suite.db) -} - func (suite *CommonTestSuite) TestReplaceMentions() { foundMentions := []*gtsmodel.Mention{ suite.testMentions["zork_mention_foss_satan"], diff --git a/internal/text/formatter_test.go b/internal/text/formatter_test.go index d01f1418e..8b4d176e2 100644 --- a/internal/text/formatter_test.go +++ b/internal/text/formatter_test.go @@ -24,9 +24,9 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/text" + "github.com/superseriousbusiness/gotosocial/testrig" ) -// nolint type TextStandardTestSuite struct { // standard suite interfaces suite.Suite @@ -47,3 +47,27 @@ type TextStandardTestSuite struct { // module being tested formatter text.Formatter } + +func (suite *TextStandardTestSuite) SetupSuite() { + suite.testTokens = testrig.NewTestTokens() + suite.testClients = testrig.NewTestClients() + suite.testApplications = testrig.NewTestApplications() + suite.testUsers = testrig.NewTestUsers() + suite.testAccounts = testrig.NewTestAccounts() + suite.testAttachments = testrig.NewTestAttachments() + suite.testStatuses = testrig.NewTestStatuses() + suite.testTags = testrig.NewTestTags() + suite.testMentions = testrig.NewTestMentions() +} + +func (suite *TextStandardTestSuite) SetupTest() { + suite.config = testrig.NewTestConfig() + suite.db = testrig.NewTestDB() + suite.formatter = text.NewFormatter(suite.config, suite.db) + + testrig.StandardDBSetup(suite.db, nil) +} + +func (suite *TextStandardTestSuite) TearDownTest() { + testrig.StandardDBTeardown(suite.db) +} diff --git a/internal/text/link_test.go b/internal/text/link_test.go index 0709e4ad1..98143bdd4 100644 --- a/internal/text/link_test.go +++ b/internal/text/link_test.go @@ -25,7 +25,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/text" - "github.com/superseriousbusiness/gotosocial/testrig" ) const text1 = ` @@ -70,29 +69,6 @@ type LinkTestSuite struct { TextStandardTestSuite } -func (suite *LinkTestSuite) SetupSuite() { - suite.testTokens = testrig.NewTestTokens() - suite.testClients = testrig.NewTestClients() - suite.testApplications = testrig.NewTestApplications() - suite.testUsers = testrig.NewTestUsers() - suite.testAccounts = testrig.NewTestAccounts() - suite.testAttachments = testrig.NewTestAttachments() - suite.testStatuses = testrig.NewTestStatuses() - suite.testTags = testrig.NewTestTags() -} - -func (suite *LinkTestSuite) SetupTest() { - suite.config = testrig.NewTestConfig() - suite.db = testrig.NewTestDB() - suite.formatter = text.NewFormatter(suite.config, suite.db) - - testrig.StandardDBSetup(suite.db, nil) -} - -func (suite *LinkTestSuite) TearDownTest() { - testrig.StandardDBTeardown(suite.db) -} - func (suite *LinkTestSuite) TestParseSimple() { f := suite.formatter.FromPlain(context.Background(), simple, nil, nil) assert.Equal(suite.T(), simpleExpected, f) diff --git a/internal/text/markdown_test.go b/internal/text/markdown_test.go index 3faa69c08..0c55cba9c 100644 --- a/internal/text/markdown_test.go +++ b/internal/text/markdown_test.go @@ -25,8 +25,6 @@ import ( "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" - "github.com/superseriousbusiness/gotosocial/internal/text" - "github.com/superseriousbusiness/gotosocial/testrig" ) const ( @@ -67,30 +65,6 @@ type MarkdownTestSuite struct { TextStandardTestSuite } -func (suite *MarkdownTestSuite) SetupSuite() { - suite.testTokens = testrig.NewTestTokens() - suite.testClients = testrig.NewTestClients() - suite.testApplications = testrig.NewTestApplications() - suite.testUsers = testrig.NewTestUsers() - suite.testAccounts = testrig.NewTestAccounts() - suite.testAttachments = testrig.NewTestAttachments() - suite.testStatuses = testrig.NewTestStatuses() - suite.testTags = testrig.NewTestTags() - suite.testMentions = testrig.NewTestMentions() -} - -func (suite *MarkdownTestSuite) SetupTest() { - suite.config = testrig.NewTestConfig() - suite.db = testrig.NewTestDB() - suite.formatter = text.NewFormatter(suite.config, suite.db) - - testrig.StandardDBSetup(suite.db, suite.testAccounts) -} - -func (suite *MarkdownTestSuite) TearDownTest() { - testrig.StandardDBTeardown(suite.db) -} - func (suite *MarkdownTestSuite) TestParseSimple() { s := suite.formatter.FromMarkdown(context.Background(), simpleMarkdown, nil, nil) suite.Equal(simpleMarkdownExpected, s) diff --git a/internal/text/plain_test.go b/internal/text/plain_test.go index b353fb284..b8a50d3a1 100644 --- a/internal/text/plain_test.go +++ b/internal/text/plain_test.go @@ -26,8 +26,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" - "github.com/superseriousbusiness/gotosocial/internal/text" - "github.com/superseriousbusiness/gotosocial/testrig" ) const ( @@ -49,30 +47,6 @@ type PlainTestSuite struct { TextStandardTestSuite } -func (suite *PlainTestSuite) SetupSuite() { - suite.testTokens = testrig.NewTestTokens() - suite.testClients = testrig.NewTestClients() - suite.testApplications = testrig.NewTestApplications() - suite.testUsers = testrig.NewTestUsers() - suite.testAccounts = testrig.NewTestAccounts() - suite.testAttachments = testrig.NewTestAttachments() - suite.testStatuses = testrig.NewTestStatuses() - suite.testTags = testrig.NewTestTags() - suite.testMentions = testrig.NewTestMentions() -} - -func (suite *PlainTestSuite) SetupTest() { - suite.config = testrig.NewTestConfig() - suite.db = testrig.NewTestDB() - suite.formatter = text.NewFormatter(suite.config, suite.db) - - testrig.StandardDBSetup(suite.db, nil) -} - -func (suite *PlainTestSuite) TearDownTest() { - testrig.StandardDBTeardown(suite.db) -} - func (suite *PlainTestSuite) TestParseSimple() { f := suite.formatter.FromPlain(context.Background(), simple, nil, nil) assert.Equal(suite.T(), simpleExpected, f)