From 6fee55dcff976f3eeae5879fe91d2f27780d0da4 Mon Sep 17 00:00:00 2001 From: tobi Date: Wed, 15 Oct 2025 18:57:57 +0200 Subject: [chore] Rationalize HTTP return codes for fedi endpoints, other tidying up (#4503) # Description > If this is a code change, please include a summary of what you've coded, and link to the issue(s) it closes/implements. > > If this is a documentation change, please briefly describe what you've changed and why. This pull request does some refactoring of the fedi API endpoints and processing functions, and the authenticate + pub key deref functions, to try to return fewer silly HTTP codes like 410 Gone (when a *remote* account is gone, not a local one), and 500 errors where something isn't really an error. Also does some general tidying up and renaming for consistency. ## 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). - [x] 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. - [x] I/we have commented the added code, particularly in hard-to-understand areas. - [ ] I/we have made any necessary changes to documentation. - [ ] 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/4503 Co-authored-by: tobi Co-committed-by: tobi --- internal/web/thread.go | 69 ++++++++++++++++++-------------------------------- 1 file changed, 24 insertions(+), 45 deletions(-) (limited to 'internal/web/thread.go') diff --git a/internal/web/thread.go b/internal/web/thread.go index 3844bc6f3..fa3e54fac 100644 --- a/internal/web/thread.go +++ b/internal/web/thread.go @@ -19,8 +19,6 @@ package web import ( "context" - "encoding/json" - "fmt" "net/http" "strings" @@ -47,14 +45,14 @@ func (m *Module) threadGETHandler(c *gin.Context) { return instance, nil } - // Parse account targetUsername and status ID from the URL. - targetUsername, errWithCode := apiutil.ParseUsername(c.Param(apiutil.UsernameKey)) + // Parse account requestedUser and status ID from the URL. + requestedUser, errWithCode := apiutil.ParseUsername(c.Param(apiutil.UsernameKey)) if errWithCode != nil { apiutil.WebErrorHandler(c, errWithCode, instanceGet) return } - targetStatusID, errWithCode := apiutil.ParseWebStatusID(c.Param(apiutil.WebStatusIDKey)) + statusID, errWithCode := apiutil.ParseWebStatusID(c.Param(apiutil.WebStatusIDKey)) if errWithCode != nil { apiutil.WebErrorHandler(c, errWithCode, instanceGet) return @@ -67,8 +65,8 @@ func (m *Module) threadGETHandler(c *gin.Context) { // // todo: Update this logic when different username patterns // are allowed, and/or when status slugs are introduced. - targetUsername = strings.ToLower(targetUsername) - targetStatusID = strings.ToUpper(targetStatusID) + requestedUser = strings.ToLower(requestedUser) + statusID = strings.ToUpper(statusID) // Check what type of content is being requested. If we're getting an AP // request on this endpoint we should render the AP representation instead. @@ -78,38 +76,44 @@ func (m *Module) threadGETHandler(c *gin.Context) { return } - if accept == string(apiutil.AppActivityJSON) || accept == string(apiutil.AppActivityLDJSON) { + if apiutil.ASContentType(accept) { // AP status representation has been requested. - m.returnAPStatus(c, targetUsername, targetStatusID, accept, instanceGet) + status, errWithCode := m.processor.Fedi().StatusGet(c.Request.Context(), requestedUser, statusID) + if errWithCode != nil { + apiutil.WebErrorHandler(c, errWithCode, instanceGet) + return + } + + apiutil.JSONType(c, http.StatusOK, accept, status) return } // text/html has been requested. Proceed with getting the web view of the status. // Fetch the target account so we can do some checks on it. - targetAccount, errWithCode := m.processor.Account().GetWeb(ctx, targetUsername) + acct, errWithCode := m.processor.Account().GetWeb(ctx, requestedUser) if errWithCode != nil { apiutil.WebErrorHandler(c, errWithCode, instanceGet) return } - // If target account is suspended, this page should not be visible. - if targetAccount.Suspended { - err := fmt.Errorf("target account %s is suspended", targetUsername) + // If requested account is suspended, this page should not be visible. + if acct.Suspended { + err := gtserror.Newf("account %s is suspended", requestedUser) apiutil.WebErrorHandler(c, gtserror.NewErrorNotFound(err), instanceGet) return } - // Get the thread context. This will fetch the target status as well. - context, errWithCode := m.processor.Status().WebContextGet(ctx, targetStatusID) + // Get the thread context. This will fetch the status as well. + context, errWithCode := m.processor.Status().WebContextGet(ctx, statusID) if errWithCode != nil { apiutil.WebErrorHandler(c, errWithCode, instanceGet) return } - // Ensure status actually belongs to target account. - if context.Status.Account.ID != targetAccount.ID { - err := fmt.Errorf("target account %s does not own status %s", targetUsername, targetStatusID) + // Ensure status actually belongs to requested account. + if context.Status.Account.ID != acct.ID { + err := gtserror.Newf("account %s does not own status %s", requestedUser, statusID) apiutil.WebErrorHandler(c, gtserror.NewErrorNotFound(err), instanceGet) return } @@ -128,7 +132,7 @@ func (m *Module) threadGETHandler(c *gin.Context) { ) // User-selected theme if set. - if theme := targetAccount.Theme; theme != "" { + if theme := acct.Theme; theme != "" { stylesheets = append( stylesheets, themesPathPrefix+"/"+theme, @@ -138,7 +142,7 @@ func (m *Module) threadGETHandler(c *gin.Context) { // Custom CSS for this user last in cascade. stylesheets = append( stylesheets, - "/@"+targetAccount.Username+"/custom.css", + "/@"+acct.Username+"/custom.css", ) page := apiutil.WebPage{ @@ -164,28 +168,3 @@ func (m *Module) threadGETHandler(c *gin.Context) { apiutil.TemplateWebPage(c, page) } - -// returnAPStatus returns an ActivityPub representation of target status, -// created by targetUsername. It will do http signature authentication. -func (m *Module) returnAPStatus( - c *gin.Context, - targetUsername string, - targetStatusID string, - accept string, - instanceGet func(ctx context.Context) (*apimodel.InstanceV1, gtserror.WithCode), -) { - status, errWithCode := m.processor.Fedi().StatusGet(c.Request.Context(), targetUsername, targetStatusID) - if errWithCode != nil { - apiutil.WebErrorHandler(c, errWithCode, instanceGet) - return - } - - b, err := json.Marshal(status) - if err != nil { - err := gtserror.Newf("could not marshal json: %w", err) - apiutil.WebErrorHandler(c, gtserror.NewErrorInternalError(err), instanceGet) - return - } - - c.Data(http.StatusOK, accept, b) -} -- cgit v1.2.3