From b212cd0169cdd0e346f706a48b1919af49f202c8 Mon Sep 17 00:00:00 2001 From: kim Date: Mon, 23 Dec 2024 15:04:58 +0000 Subject: [PATCH] add more code comments, move media description check back to media process in status create --- internal/processing/common/status.go | 11 ++---- internal/processing/media/create.go | 14 +++++++- internal/processing/media/update.go | 39 ++++++++++++++++++--- internal/processing/status/common.go | 41 +++++++++++++++++++++++ internal/processing/status/create_test.go | 2 +- 5 files changed, 91 insertions(+), 16 deletions(-) diff --git a/internal/processing/common/status.go b/internal/processing/common/status.go index 377f54c5d..01f2ab72d 100644 --- a/internal/processing/common/status.go +++ b/internal/processing/common/status.go @@ -31,7 +31,8 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/log" ) -// GetOwnStatus ... +// GetOwnStatus fetches the given status with ID, +// and ensures that it belongs to given requester. func (p *Processor) GetOwnStatus( ctx context.Context, requester *gtsmodel.Account, @@ -46,14 +47,6 @@ func (p *Processor) GetOwnStatus( return nil, gtserror.NewErrorInternalError(err) } - if target == nil { - const text = "target status not found" - return nil, gtserror.NewErrorNotFound( - errors.New(text), - text, - ) - } - switch { case target == nil: const text = "target status not found" diff --git a/internal/processing/media/create.go b/internal/processing/media/create.go index be2ff5460..5ea630618 100644 --- a/internal/processing/media/create.go +++ b/internal/processing/media/create.go @@ -51,6 +51,18 @@ func (p *Processor) Create(ctx context.Context, account *gtsmodel.Account, form return nil, errWithCode } + // If description provided, + // process and validate it. + // + // This may not yet be set as it + // is often set on status post. + if form.Description != "" { + form.Description, errWithCode = processDescription(form.Description) + if errWithCode != nil { + return nil, errWithCode + } + } + // Open multipart file reader. mpfile, err := form.File.Open() if err != nil { @@ -58,7 +70,7 @@ func (p *Processor) Create(ctx context.Context, account *gtsmodel.Account, form return nil, gtserror.NewErrorInternalError(err) } - // Wrap the multipart file reader to ensure is limited to max. + // Wrap multipart file reader to ensure is limited to max size. rc, _, _ := iotools.UpdateReadCloserLimit(mpfile, maxszInt64) // Create local media and write to instance storage. diff --git a/internal/processing/media/update.go b/internal/processing/media/update.go index 2348d3ee6..c8592395f 100644 --- a/internal/processing/media/update.go +++ b/internal/processing/media/update.go @@ -24,6 +24,7 @@ import ( apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util" + "github.com/superseriousbusiness/gotosocial/internal/config" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" @@ -48,17 +49,27 @@ func (p *Processor) Update(ctx context.Context, account *gtsmodel.Account, media var updatingColumns []string if form.Description != nil { - attachment.Description = text.SanitizeToPlaintext(*form.Description) + // Sanitize and validate incoming description. + description, errWithCode := processDescription( + *form.Description, + ) + if errWithCode != nil { + return nil, errWithCode + } + + attachment.Description = description updatingColumns = append(updatingColumns, "description") } if form.Focus != nil { - focusx, focusy, errWithCode := apiutil.ParseFocus(*form.Focus) - if err != nil { + // Parse focus details from API form input. + focusX, focusY, errWithCode := apiutil.ParseFocus(*form.Focus) + if errWithCode != nil { return nil, errWithCode } - attachment.FileMeta.Focus.X = focusx - attachment.FileMeta.Focus.Y = focusy + + attachment.FileMeta.Focus.X = focusX + attachment.FileMeta.Focus.Y = focusY updatingColumns = append(updatingColumns, "focus_x", "focus_y") } @@ -73,3 +84,21 @@ func (p *Processor) Update(ctx context.Context, account *gtsmodel.Account, media return &a, nil } + +// processDescription will sanitize and valid description against server configuration. +func processDescription(description string) (string, gtserror.WithCode) { + description = text.SanitizeToPlaintext(description) + chars := len([]rune(description)) + + if min := config.GetMediaDescriptionMinChars(); chars < min { + text := fmt.Sprintf("media description less than min chars (%d)", min) + return "", gtserror.NewErrorBadRequest(errors.New(text), text) + } + + if max := config.GetMediaDescriptionMaxChars(); chars > max { + text := fmt.Sprintf("media description exceeds max chars (%d)", max) + return "", gtserror.NewErrorBadRequest(errors.New(text), text) + } + + return description, nil +} diff --git a/internal/processing/status/common.go b/internal/processing/status/common.go index f5c22a6b0..3f2b7b6cb 100644 --- a/internal/processing/status/common.go +++ b/internal/processing/status/common.go @@ -1,3 +1,20 @@ +// GoToSocial +// Copyright (C) GoToSocial Authors admin@gotosocial.org +// SPDX-License-Identifier: AGPL-3.0-or-later +// +// 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 status import ( @@ -17,6 +34,9 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/validate" ) +// validateStatusContent will validate the common +// content fields across status write endpoints against +// current server configuration (e.g. max char counts). func validateStatusContent( status string, spoiler string, @@ -69,6 +89,10 @@ func validateStatusContent( return nil } +// statusContent encompasses the set of common processed +// status content fields from status write operations for +// an easily returnable type, without needing to allocate +// an entire gtsmodel.Status{} model. type statusContent struct { Content string ContentWarning string @@ -241,6 +265,10 @@ func (p *Processor) processMedia( return nil, nil } + // Get configured min/max supported descr chars. + minChars := config.GetMediaDescriptionMinChars() + maxChars := config.GetMediaDescriptionMaxChars() + // Pre-allocate slice of media attachments of expected length. attachments := make([]*gtsmodel.MediaAttachment, len(mediaIDs)) for i, id := range mediaIDs { @@ -266,6 +294,19 @@ func (p *Processor) processMedia( return nil, gtserror.NewErrorBadRequest(errors.New(text), text) } + // Check media description chars within range, + // this needs to be done here as lots of clients + // only update media description on status post. + switch chars := len([]rune(media.Description)); { + case chars < minChars: + text := fmt.Sprintf("media description less than min chars (%d)", minChars) + return nil, gtserror.NewErrorBadRequest(errors.New(text), text) + + case chars > maxChars: + text := fmt.Sprintf("media description exceeds max chars (%d)", maxChars) + return nil, gtserror.NewErrorBadRequest(errors.New(text), text) + } + // Set media at index. attachments[i] = media } diff --git a/internal/processing/status/create_test.go b/internal/processing/status/create_test.go index 84168880e..d0a5c7f92 100644 --- a/internal/processing/status/create_test.go +++ b/internal/processing/status/create_test.go @@ -170,7 +170,7 @@ func (suite *StatusCreateTestSuite) TestProcessMediaDescriptionTooShort() { } apiStatus, err := suite.status.Create(ctx, creatingAccount, creatingApplication, statusCreateForm) - suite.EqualError(err, "media 01F8MH8RMYQ6MSNY3JM2XT1CQ5 description too short, at least 100 required") + suite.EqualError(err, "media description less than min chars (100)") suite.Nil(apiStatus) }