mirror of
				https://github.com/superseriousbusiness/gotosocial.git
				synced 2025-10-31 11:42:25 -05:00 
			
		
		
		
	[bugfix] return 422 for invalid status visibility (#4252)
Signed-off-by: nicole mikołajczyk <git@mkljczk.pl> # 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 <git@mkljczk.pl> Co-committed-by: nicole mikołajczyk <git@mkljczk.pl>
This commit is contained in:
		
					parent
					
						
							
								54a0af34fb
							
						
					
				
			
			
				commit
				
					
						3f1c3c0dac
					
				
			
		
					 3 changed files with 44 additions and 5 deletions
				
			
		|  | @ -446,7 +446,7 @@ func (suite *StatusCreateTestSuite) TestPostNewStatusIntPolicyJSON() { | ||||||
| func (suite *StatusCreateTestSuite) TestPostNewStatusMessedUpIntPolicy() { | func (suite *StatusCreateTestSuite) TestPostNewStatusMessedUpIntPolicy() { | ||||||
| 	out, recorder := suite.postStatus(nil, `{ | 	out, recorder := suite.postStatus(nil, `{ | ||||||
|   "status": "this is a brand new status! #helloworld", |   "status": "this is a brand new status! #helloworld", | ||||||
|   "visibility": "followers_only", |   "visibility": "private", | ||||||
|   "interaction_policy": { |   "interaction_policy": { | ||||||
|     "can_reply": { |     "can_reply": { | ||||||
|       "always": [ |       "always": [ | ||||||
|  | @ -463,7 +463,7 @@ func (suite *StatusCreateTestSuite) TestPostNewStatusMessedUpIntPolicy() { | ||||||
| 	// We should have a helpful error | 	// We should have a helpful error | ||||||
| 	// message telling us how we screwed up. | 	// message telling us how we screwed up. | ||||||
| 	suite.Equal(`{ | 	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) | }`, out) | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -218,7 +218,9 @@ func (p *Processor) Create( | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	// Process the incoming created status visibility. | 	// 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 | 	// Process policy AFTER visibility as it relies | ||||||
| 	// on status.Visibility and form.Visibility being set. | 	// on status.Visibility and form.Visibility being set. | ||||||
|  | @ -444,11 +446,20 @@ func processVisibility( | ||||||
| 	form *apimodel.StatusCreateRequest, | 	form *apimodel.StatusCreateRequest, | ||||||
| 	accountDefaultVis gtsmodel.Visibility, | 	accountDefaultVis gtsmodel.Visibility, | ||||||
| 	status *gtsmodel.Status, | 	status *gtsmodel.Status, | ||||||
| ) { | ) gtserror.WithCode { | ||||||
| 	switch { | 	switch { | ||||||
| 	// Visibility set on form, use that. | 	// Visibility set on form, use that. | ||||||
| 	case form.Visibility != "": | 	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 | 	// Fall back to account default, set | ||||||
| 	// this back on the form for later use. | 	// this back on the form for later use. | ||||||
|  | @ -467,6 +478,8 @@ func processVisibility( | ||||||
| 	// assuming federated (ie., not local-only) by default. | 	// assuming federated (ie., not local-only) by default. | ||||||
| 	localOnly := util.PtrOrValue(form.LocalOnly, false) | 	localOnly := util.PtrOrValue(form.LocalOnly, false) | ||||||
| 	status.Federated = util.Ptr(!localOnly) | 	status.Federated = util.Ptr(!localOnly) | ||||||
|  | 
 | ||||||
|  | 	return nil | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func processInteractionPolicy( | func processInteractionPolicy( | ||||||
|  |  | ||||||
|  | @ -18,6 +18,7 @@ | ||||||
| package status_test | package status_test | ||||||
| 
 | 
 | ||||||
| import ( | import ( | ||||||
|  | 	"net/http" | ||||||
| 	"testing" | 	"testing" | ||||||
| 
 | 
 | ||||||
| 	apimodel "code.superseriousbusiness.org/gotosocial/internal/api/model" | 	apimodel "code.superseriousbusiness.org/gotosocial/internal/api/model" | ||||||
|  | @ -240,6 +241,31 @@ func (suite *StatusCreateTestSuite) TestProcessNoContentTypeUsesDefault() { | ||||||
| 	suite.Equal(apimodel.StatusContentTypeDefault, apiStatus.ContentType) | 	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) { | func TestStatusCreateTestSuite(t *testing.T) { | ||||||
| 	suite.Run(t, new(StatusCreateTestSuite)) | 	suite.Run(t, new(StatusCreateTestSuite)) | ||||||
| } | } | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue