From 3f1c3c0dacafce8a4421e5d2570adf2084f29756 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?nicole=20miko=C5=82ajczyk?= Date: Mon, 9 Jun 2025 16:26:49 +0200 Subject: [PATCH] [bugfix] return 422 for invalid status visibility (#4252) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: nicole mikołajczyk # Description closes #4247 ## Checklist Please put an x inside each checkbox to indicate that you've read and followed it: `[ ]` -> `[x]` If this is a documentation change, only the first checkbox must be filled (you can delete the others if you want). - [x] I/we have read the [GoToSocial contribution guidelines](https://codeberg.org/superseriousbusiness/gotosocial/src/branch/main/CONTRIBUTING.md). - [ ] I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat. - [x] I/we have not leveraged AI to create the proposed changes. - [x] I/we have performed a self-review of added code. - [x] I/we have written code that is legible and maintainable by others. - [ ] I/we have commented the added code, particularly in hard-to-understand areas. - [ ] I/we have made any necessary changes to documentation. - [x] I/we have added tests that cover new code. - [x] I/we have run tests and they pass locally with the changes. - [x] I/we have run `go fmt ./...` and `golangci-lint run`. Reviewed-on: https://codeberg.org/superseriousbusiness/gotosocial/pulls/4252 Co-authored-by: nicole mikołajczyk Co-committed-by: nicole mikołajczyk --- .../api/client/statuses/statuscreate_test.go | 4 +-- internal/processing/status/create.go | 19 +++++++++++--- internal/processing/status/create_test.go | 26 +++++++++++++++++++ 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/internal/api/client/statuses/statuscreate_test.go b/internal/api/client/statuses/statuscreate_test.go index 548eced29..090dae593 100644 --- a/internal/api/client/statuses/statuscreate_test.go +++ b/internal/api/client/statuses/statuscreate_test.go @@ -446,7 +446,7 @@ func (suite *StatusCreateTestSuite) TestPostNewStatusIntPolicyJSON() { func (suite *StatusCreateTestSuite) TestPostNewStatusMessedUpIntPolicy() { out, recorder := suite.postStatus(nil, `{ "status": "this is a brand new status! #helloworld", - "visibility": "followers_only", + "visibility": "private", "interaction_policy": { "can_reply": { "always": [ @@ -463,7 +463,7 @@ func (suite *StatusCreateTestSuite) TestPostNewStatusMessedUpIntPolicy() { // We should have a helpful error // message telling us how we screwed up. suite.Equal(`{ - "error": "Bad Request: error converting followers_only.can_reply.always: policyURI public is not feasible for visibility followers_only" + "error": "Bad Request: error converting private.can_reply.always: policyURI public is not feasible for visibility private" }`, out) } diff --git a/internal/processing/status/create.go b/internal/processing/status/create.go index f9f986256..3604d3a4a 100644 --- a/internal/processing/status/create.go +++ b/internal/processing/status/create.go @@ -218,7 +218,9 @@ func (p *Processor) Create( } // Process the incoming created status visibility. - processVisibility(form, requester.Settings.Privacy, status) + if errWithCode := processVisibility(form, requester.Settings.Privacy, status); errWithCode != nil { + return nil, errWithCode + } // Process policy AFTER visibility as it relies // on status.Visibility and form.Visibility being set. @@ -444,11 +446,20 @@ func processVisibility( form *apimodel.StatusCreateRequest, accountDefaultVis gtsmodel.Visibility, status *gtsmodel.Status, -) { +) gtserror.WithCode { switch { // Visibility set on form, use that. case form.Visibility != "": - status.Visibility = typeutils.APIVisToVis(form.Visibility) + visibility := typeutils.APIVisToVis(form.Visibility) + + if visibility == 0 { + const errText = "invalid visibility" + err := gtserror.New(errText) + errWithCode := gtserror.NewErrorUnprocessableEntity(err, err.Error()) + return errWithCode + } + + status.Visibility = visibility // Fall back to account default, set // this back on the form for later use. @@ -467,6 +478,8 @@ func processVisibility( // assuming federated (ie., not local-only) by default. localOnly := util.PtrOrValue(form.LocalOnly, false) status.Federated = util.Ptr(!localOnly) + + return nil } func processInteractionPolicy( diff --git a/internal/processing/status/create_test.go b/internal/processing/status/create_test.go index a2adb5f79..82bc801c4 100644 --- a/internal/processing/status/create_test.go +++ b/internal/processing/status/create_test.go @@ -18,6 +18,7 @@ package status_test import ( + "net/http" "testing" apimodel "code.superseriousbusiness.org/gotosocial/internal/api/model" @@ -240,6 +241,31 @@ func (suite *StatusCreateTestSuite) TestProcessNoContentTypeUsesDefault() { suite.Equal(apimodel.StatusContentTypeDefault, apiStatus.ContentType) } +func (suite *StatusCreateTestSuite) TestProcessInvalidVisibility() { + ctx := suite.T().Context() + creatingAccount := suite.testAccounts["local_account_1"] + creatingApplication := suite.testApplications["application_1"] + + statusCreateForm := &apimodel.StatusCreateRequest{ + Status: "my tests content is boring", + SpoilerText: "", + MediaIDs: []string{}, + Poll: nil, + InReplyToID: "", + Sensitive: false, + Visibility: "local", + LocalOnly: util.Ptr(false), + ScheduledAt: nil, + Language: "en", + ContentType: apimodel.StatusContentTypePlain, + } + + apiStatus, errWithCode := suite.status.Create(ctx, creatingAccount, creatingApplication, statusCreateForm) + suite.Nil(apiStatus) + suite.Equal(http.StatusUnprocessableEntity, errWithCode.Code()) + suite.Equal("Unprocessable Entity: processVisibility: invalid visibility", errWithCode.Safe()) +} + func TestStatusCreateTestSuite(t *testing.T) { suite.Run(t, new(StatusCreateTestSuite)) }