[bugfix] Fix HTML escaping in instance title (#607)

* move caption sanitization -> sanitize.go

* use sanitizeplaintext rather than removehtml

* rename sanitizecaption to sanitizeplaintext

* avoid removing html twice from statuses

* unexport remoteHTML
it's no longer used outside the text package so this
makes it less confusing

* test instance PATCH

internal/api/client/instance/instance_test.go

@@ -0,0 +1,126 @@
+/*
+   GoToSocial
+   Copyright (C) 2021-2022 GoToSocial Authors [email protected]
+
+   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 <http://www.gnu.org/licenses/>.
+*/
+
+package instance_test
+
+import (
+	"bytes"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+
+	"codeberg.org/gruf/go-store/kv"
+	"github.com/gin-gonic/gin"
+	"github.com/spf13/viper"
+	"github.com/stretchr/testify/suite"
+	"github.com/superseriousbusiness/gotosocial/internal/api/client/instance"
+	"github.com/superseriousbusiness/gotosocial/internal/concurrency"
+	"github.com/superseriousbusiness/gotosocial/internal/config"
+	"github.com/superseriousbusiness/gotosocial/internal/db"
+	"github.com/superseriousbusiness/gotosocial/internal/email"
+	"github.com/superseriousbusiness/gotosocial/internal/federation"
+	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
+	"github.com/superseriousbusiness/gotosocial/internal/media"
+	"github.com/superseriousbusiness/gotosocial/internal/messages"
+	"github.com/superseriousbusiness/gotosocial/internal/oauth"
+	"github.com/superseriousbusiness/gotosocial/internal/processing"
+	"github.com/superseriousbusiness/gotosocial/testrig"
+)
+
+type InstanceStandardTestSuite struct {
+	// standard suite interfaces
+	suite.Suite
+	db           db.DB
+	storage      *kv.KVStore
+	mediaManager media.Manager
+	federator    federation.Federator
+	processor    processing.Processor
+	emailSender  email.Sender
+	sentEmails   map[string]string
+
+	// standard suite models
+	testTokens       map[string]*gtsmodel.Token
+	testClients      map[string]*gtsmodel.Client
+	testApplications map[string]*gtsmodel.Application
+	testUsers        map[string]*gtsmodel.User
+	testAccounts     map[string]*gtsmodel.Account
+	testAttachments  map[string]*gtsmodel.MediaAttachment
+	testStatuses     map[string]*gtsmodel.Status
+
+	// module being tested
+	instanceModule *instance.Module
+}
+
+func (suite *InstanceStandardTestSuite) 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()
+}
+
+func (suite *InstanceStandardTestSuite) SetupTest() {
+	testrig.InitTestConfig()
+	testrig.InitTestLog()
+
+	fedWorker := concurrency.NewWorkerPool[messages.FromFederator](-1, -1)
+	clientWorker := concurrency.NewWorkerPool[messages.FromClientAPI](-1, -1)
+
+	suite.db = testrig.NewTestDB()
+	suite.storage = testrig.NewTestStorage()
+	suite.mediaManager = testrig.NewTestMediaManager(suite.db, suite.storage)
+	suite.federator = testrig.NewTestFederator(suite.db, testrig.NewTestTransportController(testrig.NewMockHTTPClient(nil), suite.db, fedWorker), suite.storage, suite.mediaManager, fedWorker)
+	suite.sentEmails = make(map[string]string)
+	suite.emailSender = testrig.NewEmailSender("../../../../web/template/", suite.sentEmails)
+	suite.processor = testrig.NewTestProcessor(suite.db, suite.storage, suite.federator, suite.emailSender, suite.mediaManager, clientWorker, fedWorker)
+	suite.instanceModule = instance.New(suite.processor).(*instance.Module)
+	testrig.StandardDBSetup(suite.db, nil)
+	testrig.StandardStorageSetup(suite.storage, "../../../../testrig/media")
+}
+
+func (suite *InstanceStandardTestSuite) TearDownTest() {
+	testrig.StandardDBTeardown(suite.db)
+	testrig.StandardStorageTeardown(suite.storage)
+}
+
+func (suite *InstanceStandardTestSuite) newContext(recorder *httptest.ResponseRecorder, requestMethod string, requestBody []byte, requestPath string, bodyContentType string) *gin.Context {
+	ctx, _ := gin.CreateTestContext(recorder)
+
+	ctx.Set(oauth.SessionAuthorizedAccount, suite.testAccounts["admin_account"])
+	ctx.Set(oauth.SessionAuthorizedToken, oauth.DBTokenToToken(suite.testTokens["admin_account"]))
+	ctx.Set(oauth.SessionAuthorizedApplication, suite.testApplications["admin_account"])
+	ctx.Set(oauth.SessionAuthorizedUser, suite.testUsers["admin_account"])
+
+	protocol := viper.GetString(config.Keys.Protocol)
+	host := viper.GetString(config.Keys.Host)
+
+	baseURI := fmt.Sprintf("%s://%s", protocol, host)
+	requestURI := fmt.Sprintf("%s/%s", baseURI, requestPath)
+
+	ctx.Request = httptest.NewRequest(http.MethodPatch, requestURI, bytes.NewReader(requestBody)) // the endpoint we're hitting
+
+	if bodyContentType != "" {
+		ctx.Request.Header.Set("Content-Type", bodyContentType)
+	}
+
+	ctx.Request.Header.Set("accept", "application/json")
+
+	return ctx
+}

internal/api/client/instance/instancepatch_test.go

@@ -0,0 +1,130 @@
+/*
+   GoToSocial
+   Copyright (C) 2021-2022 GoToSocial Authors [email protected]
+
+   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 <http://www.gnu.org/licenses/>.
+*/
+
+package instance_test
+
+import (
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/suite"
+	"github.com/superseriousbusiness/gotosocial/internal/api/client/instance"
+	"github.com/superseriousbusiness/gotosocial/testrig"
+)
+
+type InstancePatchTestSuite struct {
+	InstanceStandardTestSuite
+}
+
+func (suite *InstancePatchTestSuite) TestInstancePatch1() {
+	requestBody, w, err := testrig.CreateMultipartFormData(
+		"", "",
+		map[string]string{
+			"title":            "Example Instance",
+			"contact_username": "admin",
+			"contact_email":    "[email protected]",
+		})
+	if err != nil {
+		panic(err)
+	}
+	bodyBytes := requestBody.Bytes()
+
+	// set up the request
+	recorder := httptest.NewRecorder()
+	ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, instance.InstanceInformationPath, w.FormDataContentType())
+
+	// call the handler
+	suite.instanceModule.InstanceUpdatePATCHHandler(ctx)
+
+	// we should have OK because our request was valid
+	suite.Equal(http.StatusOK, recorder.Code)
+
+	result := recorder.Result()
+	defer result.Body.Close()
+
+	b, err := io.ReadAll(result.Body)
+	suite.NoError(err)
+
+	suite.Equal(`{"uri":"http://localhost:8080","title":"Example Instance","description":"","short_description":"","email":"[email protected]","version":"","registrations":true,"approval_required":true,"invites_enabled":false,"urls":{"streaming_api":"wss://localhost:8080"},"stats":{"domain_count":0,"status_count":16,"user_count":4},"thumbnail":"","contact_account":{"id":"01F8MH17FWEB39HZJ76B6VXSKF","username":"admin","acct":"admin","display_name":"","locked":false,"bot":false,"created_at":"2022-05-17T13:10:59Z","note":"","url":"http://localhost:8080/@admin","avatar":"","avatar_static":"","header":"","header_static":"","followers_count":1,"following_count":1,"statuses_count":4,"last_status_at":"2021-10-20T10:41:37Z","emojis":[],"fields":[]},"max_toot_chars":5000}`, string(b))
+}
+
+func (suite *InstancePatchTestSuite) TestInstancePatch2() {
+	requestBody, w, err := testrig.CreateMultipartFormData(
+		"", "",
+		map[string]string{
+			"title": "<p>Geoff's Instance</p>",
+		})
+	if err != nil {
+		panic(err)
+	}
+	bodyBytes := requestBody.Bytes()
+
+	// set up the request
+	recorder := httptest.NewRecorder()
+	ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, instance.InstanceInformationPath, w.FormDataContentType())
+
+	// call the handler
+	suite.instanceModule.InstanceUpdatePATCHHandler(ctx)
+
+	// we should have OK because our request was valid
+	suite.Equal(http.StatusOK, recorder.Code)
+
+	result := recorder.Result()
+	defer result.Body.Close()
+
+	b, err := io.ReadAll(result.Body)
+	suite.NoError(err)
+
+	suite.Equal(`{"uri":"http://localhost:8080","title":"Geoff's Instance","description":"","short_description":"","email":"","version":"","registrations":true,"approval_required":true,"invites_enabled":false,"urls":{"streaming_api":"wss://localhost:8080"},"stats":{"domain_count":0,"status_count":16,"user_count":4},"thumbnail":"","max_toot_chars":5000}`, string(b))
+}
+
+func (suite *InstancePatchTestSuite) TestInstancePatch3() {
+	requestBody, w, err := testrig.CreateMultipartFormData(
+		"", "",
+		map[string]string{
+			"short_description": "<p>This is some html, which is <em>allowed</em> in short descriptions.</p>",
+		})
+	if err != nil {
+		panic(err)
+	}
+	bodyBytes := requestBody.Bytes()
+
+	// set up the request
+	recorder := httptest.NewRecorder()
+	ctx := suite.newContext(recorder, http.MethodPatch, bodyBytes, instance.InstanceInformationPath, w.FormDataContentType())
+
+	// call the handler
+	suite.instanceModule.InstanceUpdatePATCHHandler(ctx)
+
+	// we should have OK because our request was valid
+	suite.Equal(http.StatusOK, recorder.Code)
+
+	result := recorder.Result()
+	defer result.Body.Close()
+
+	b, err := io.ReadAll(result.Body)
+	suite.NoError(err)
+
+	suite.Equal(`{"uri":"http://localhost:8080","title":"localhost:8080","description":"","short_description":"\u003cp\u003eThis is some html, which is \u003cem\u003eallowed\u003c/em\u003e in short descriptions.\u003c/p\u003e","email":"","version":"","registrations":true,"approval_required":true,"invites_enabled":false,"urls":{"streaming_api":"wss://localhost:8080"},"stats":{"domain_count":0,"status_count":16,"user_count":4},"thumbnail":"","max_toot_chars":5000}`, string(b))
+}
+
+func TestInstancePatchTestSuite(t *testing.T) {
+	suite.Run(t, &InstancePatchTestSuite{})
+}

internal/processing/account/create.go

@@ -64,7 +64,7 @@ func (p *processor) Create(ctx context.Context, applicationToken oauth2.TokenInf
 	}
 
 	l.Trace("creating new username and account")
-	user, err := p.db.NewSignup(ctx, form.Username, text.RemoveHTML(reason), approvalRequired, form.Email, form.Password, form.IP, form.Locale, application.ID, false, false)
+	user, err := p.db.NewSignup(ctx, form.Username, text.SanitizePlaintext(reason), approvalRequired, form.Email, form.Password, form.IP, form.Locale, application.ID, false, false)
 	if err != nil {
 		return nil, fmt.Errorf("error creating new signup in the database: %s", err)
 	}

internal/processing/account/update.go

@@ -53,7 +53,7 @@ func (p *processor) Update(ctx context.Context, account *gtsmodel.Account, form
 		if err := validate.DisplayName(*form.DisplayName); err != nil {
 			return nil, err
 		}
-		account.DisplayName = text.RemoveHTML(*form.DisplayName)
+		account.DisplayName = text.SanitizePlaintext(*form.DisplayName)
 	}
 
 	if form.Note != nil {

internal/processing/admin/createdomainblock.go

@@ -59,8 +59,8 @@ func (p *processor) DomainBlockCreate(ctx context.Context, account *gtsmodel.Acc
 			ID:                 blockID,
 			Domain:             domain,
 			CreatedByAccountID: account.ID,
-			PrivateComment:     text.RemoveHTML(privateComment),
-			PublicComment:      text.RemoveHTML(publicComment),
+			PrivateComment:     text.SanitizePlaintext(privateComment),
+			PublicComment:      text.SanitizePlaintext(publicComment),
 			Obfuscate:          obfuscate,
 			SubscriptionID:     subscriptionID,
 		}

internal/processing/instance.go

@@ -65,7 +65,7 @@ func (p *processor) InstancePatch(ctx context.Context, form *apimodel.InstanceSe
 		if err := validate.SiteTitle(*form.Title); err != nil {
 			return nil, gtserror.NewErrorBadRequest(err, fmt.Sprintf("site title invalid: %s", err))
 		}
-		i.Title = text.RemoveHTML(*form.Title) // don't allow html in site title
+		i.Title = text.SanitizePlaintext(*form.Title) // don't allow html in site title
 	}
 
 	// validate & update site contact account if it's set on the form

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.SanitizeCaption(*form.Description)
+		attachment.Description = text.SanitizePlaintext(*form.Description)
 		if err := p.db.UpdateByPrimaryKey(ctx, attachment); err != nil {
 			return nil, gtserror.NewErrorInternalError(fmt.Errorf("database error updating description: %s", err))
 		}

internal/processing/status/create.go

@@ -49,7 +49,7 @@ func (p *processor) Create(ctx context.Context, account *gtsmodel.Account, appli
 		Local:                    true,
 		AccountID:                account.ID,
 		AccountURI:               account.URI,
-		ContentWarning:           text.SanitizeCaption(form.SpoilerText),
+		ContentWarning:           text.SanitizePlaintext(form.SpoilerText),
 		ActivityStreamsType:      ap.ObjectNote,
 		Sensitive:                form.Sensitive,
 		Language:                 form.Language,

internal/processing/status/util.go

@@ -27,7 +27,6 @@ import (
 	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
 	"github.com/superseriousbusiness/gotosocial/internal/db"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
-	"github.com/superseriousbusiness/gotosocial/internal/text"
 	"github.com/superseriousbusiness/gotosocial/internal/util"
 )
 
@@ -269,16 +268,13 @@ func (p *processor) ProcessContent(ctx context.Context, form *apimodel.AdvancedS
 		form.Format = apimodel.StatusFormatDefault
 	}
 
-	// remove any existing html from the status
-	content := text.RemoveHTML(form.Status)
-
 	// parse content out of the status depending on what format has been submitted
 	var formatted string
 	switch form.Format {
 	case apimodel.StatusFormatPlain:
-		formatted = p.formatter.FromPlain(ctx, content, status.Mentions, status.Tags)
+		formatted = p.formatter.FromPlain(ctx, form.Status, status.Mentions, status.Tags)
 	case apimodel.StatusFormatMarkdown:
-		formatted = p.formatter.FromMarkdown(ctx, content, status.Mentions, status.Tags)
+		formatted = p.formatter.FromMarkdown(ctx, form.Status, status.Mentions, status.Tags)
 	default:
 		return fmt.Errorf("format %s not recognised as a valid status format", form.Format)
 	}