diff options
Diffstat (limited to 'internal')
8 files changed, 306 insertions, 34 deletions
diff --git a/internal/cache/size.go b/internal/cache/size.go index 4cca91666..24101683a 100644 --- a/internal/cache/size.go +++ b/internal/cache/size.go @@ -372,6 +372,7 @@ func sizeofDomainPermissionSubscription() uintptr { FetchedAt: exampleTime, SuccessfullyFetchedAt: exampleTime, ETag: exampleID, + LastModified: exampleTime, Error: exampleTextSmall, })) } diff --git a/internal/db/bundb/migrations/20241022153016_domain_permission_subscriptions/domainpermissionsubscription.go b/internal/db/bundb/migrations/20241022153016_domain_permission_subscriptions/domainpermissionsubscription.go index 851f44f15..b24dc7da5 100644 --- a/internal/db/bundb/migrations/20241022153016_domain_permission_subscriptions/domainpermissionsubscription.go +++ b/internal/db/bundb/migrations/20241022153016_domain_permission_subscriptions/domainpermissionsubscription.go @@ -33,6 +33,7 @@ type DomainPermissionSubscription struct { FetchPassword string `bun:",nullzero"` FetchedAt time.Time `bun:"type:timestamptz,nullzero"` SuccessfullyFetchedAt time.Time `bun:"type:timestamptz,nullzero"` + LastModified time.Time `bun:"type:timestamptz,nullzero"` ETag string `bun:"etag,nullzero"` Error string `bun:",nullzero"` } diff --git a/internal/db/bundb/migrations/20250119112746_domain_perm_sub_caching_tweaks.go b/internal/db/bundb/migrations/20250119112746_domain_perm_sub_caching_tweaks.go new file mode 100644 index 000000000..c37e4aa7f --- /dev/null +++ b/internal/db/bundb/migrations/20250119112746_domain_perm_sub_caching_tweaks.go @@ -0,0 +1,76 @@ +// 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 <http://www.gnu.org/licenses/>. + +package migrations + +import ( + "context" + "fmt" + "reflect" + + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/log" + "github.com/uptrace/bun" +) + +func init() { + up := func(ctx context.Context, db *bun.DB) error { + return db.RunInTx(ctx, nil, func(ctx context.Context, tx bun.Tx) error { + // Bail if "last_modified" + // column already created. + if exists, err := doesColumnExist( + ctx, + tx, + "domain_permission_subscriptions", + "last_modified", + ); err != nil { + return err + } else if exists { + return nil + } + + // Derive column definition. + var permSub *gtsmodel.DomainPermissionSubscription + permSubType := reflect.TypeOf(permSub) + colDef, err := getBunColumnDef(tx, permSubType, "LastModified") + if err != nil { + return fmt.Errorf("error making column def: %w", err) + } + + log.Info(ctx, "adding domain_permission_subscriptions.last_modified column...") + if _, err := tx. + NewAddColumn(). + Model(permSub). + ColumnExpr(colDef). + Exec(ctx); err != nil { + return fmt.Errorf("error adding column: %w", err) + } + + return nil + }) + } + + down := func(ctx context.Context, db *bun.DB) error { + return db.RunInTx(ctx, nil, func(ctx context.Context, tx bun.Tx) error { + return nil + }) + } + + if err := Migrations.Register(up, down); err != nil { + panic(err) + } +} diff --git a/internal/gtsmodel/domainpermissionsubscription.go b/internal/gtsmodel/domainpermissionsubscription.go index b6a0b8f43..724b2164f 100644 --- a/internal/gtsmodel/domainpermissionsubscription.go +++ b/internal/gtsmodel/domainpermissionsubscription.go @@ -34,7 +34,8 @@ type DomainPermissionSubscription struct { FetchPassword string `bun:",nullzero"` // Password to send when doing a GET of URI using basic auth. FetchedAt time.Time `bun:"type:timestamptz,nullzero"` // Time when fetch of URI was last attempted. SuccessfullyFetchedAt time.Time `bun:"type:timestamptz,nullzero"` // Time when the domain permission list was last *successfuly* fetched, to be transmitted as If-Modified-Since header. - ETag string `bun:"etag,nullzero"` // Etag last received from the server (if any) on successful fetch. + LastModified time.Time `bun:"type:timestamptz,nullzero"` // "Last-Modified" time received from the server (if any) on last successful fetch. Used for HTTP request caching. + ETag string `bun:"etag,nullzero"` // "ETag" header last received from the server (if any) on last successful fetch. Used for HTTP request caching. Error string `bun:",nullzero"` // If latest fetch attempt errored, this field stores the error message. Cleared on latest successful fetch. } diff --git a/internal/subscriptions/domainperms.go b/internal/subscriptions/domainperms.go index ee4730023..b94f284bf 100644 --- a/internal/subscriptions/domainperms.go +++ b/internal/subscriptions/domainperms.go @@ -253,13 +253,9 @@ func (s *Subscriptions) ProcessDomainPermissionSubscription( // to indicate a successful fetch, and return. if resp.Unmodified { l.Debug("received 304 Not Modified from remote") + permSub.ETag = resp.ETag + permSub.LastModified = resp.LastModified permSub.SuccessfullyFetchedAt = permSub.FetchedAt - if permSub.ETag == "" && resp.ETag != "" { - // We didn't have an ETag before but - // we have one now: probably the remote - // added ETag support in the meantime. - permSub.ETag = resp.ETag - } return nil, nil } @@ -308,6 +304,7 @@ func (s *Subscriptions) ProcessDomainPermissionSubscription( // This can now be considered a successful fetch. permSub.SuccessfullyFetchedAt = permSub.FetchedAt permSub.ETag = resp.ETag + permSub.LastModified = resp.LastModified permSub.Error = "" // Keep track of which domain perms are diff --git a/internal/subscriptions/subscriptions_test.go b/internal/subscriptions/subscriptions_test.go index 6c47fed21..d86d98691 100644 --- a/internal/subscriptions/subscriptions_test.go +++ b/internal/subscriptions/subscriptions_test.go @@ -107,7 +107,7 @@ func (suite *SubscriptionsTestSuite) TestDomainBlocksCSV() { } // The just-fetched perm sub should - // have ETag and count etc set now. + // have cache meta and count etc set now. permSub, err := testStructs.State.DB.GetDomainPermissionSubscriptionByID( ctx, testSubscription.ID, ) @@ -121,7 +121,8 @@ func (suite *SubscriptionsTestSuite) TestDomainBlocksCSV() { suite.FailNow(err.Error()) } - suite.Equal("bigbums6969", permSub.ETag) + suite.Equal("\"bigbums6969\"", permSub.ETag) + suite.EqualValues(1726956000, permSub.LastModified.Unix()) suite.EqualValues(3, count) suite.WithinDuration(time.Now(), permSub.FetchedAt, 1*time.Minute) suite.WithinDuration(time.Now(), permSub.SuccessfullyFetchedAt, 1*time.Minute) @@ -186,7 +187,7 @@ func (suite *SubscriptionsTestSuite) TestDomainBlocksJSON() { } // The just-fetched perm sub should - // have ETag and count etc set now. + // have cache meta and count etc set now. permSub, err := testStructs.State.DB.GetDomainPermissionSubscriptionByID( ctx, testSubscription.ID, ) @@ -200,7 +201,8 @@ func (suite *SubscriptionsTestSuite) TestDomainBlocksJSON() { suite.FailNow(err.Error()) } - suite.Equal("don't modify me daddy", permSub.ETag) + suite.Equal("\"don't modify me daddy\"", permSub.ETag) + suite.EqualValues(1726956000, permSub.LastModified.Unix()) suite.EqualValues(3, count) suite.WithinDuration(time.Now(), permSub.FetchedAt, 1*time.Minute) suite.WithinDuration(time.Now(), permSub.SuccessfullyFetchedAt, 1*time.Minute) @@ -265,7 +267,7 @@ func (suite *SubscriptionsTestSuite) TestDomainBlocksPlain() { } // The just-fetched perm sub should - // have ETag and count etc set now. + // have cache meta and count etc set now. permSub, err := testStructs.State.DB.GetDomainPermissionSubscriptionByID( ctx, testSubscription.ID, ) @@ -279,13 +281,14 @@ func (suite *SubscriptionsTestSuite) TestDomainBlocksPlain() { suite.FailNow(err.Error()) } - suite.Equal("this is a legit etag i swear", permSub.ETag) + suite.Equal("\"this is a legit etag i swear\"", permSub.ETag) + suite.EqualValues(1726956000, permSub.LastModified.Unix()) suite.EqualValues(3, count) suite.WithinDuration(time.Now(), permSub.FetchedAt, 1*time.Minute) suite.WithinDuration(time.Now(), permSub.SuccessfullyFetchedAt, 1*time.Minute) } -func (suite *SubscriptionsTestSuite) TestDomainBlocksCSVETag() { +func (suite *SubscriptionsTestSuite) TestDomainBlocksCSVCaching() { var ( ctx = context.Background() testStructs = testrig.SetupTestStructs(rMediaPath, rTemplatePath) @@ -297,7 +300,7 @@ func (suite *SubscriptionsTestSuite) TestDomainBlocksCSVETag() { ) // Create a subscription for a CSV list of baddies. - // Include the ETag so it gets sent with the request. + // Include ETag + LastModified so they get sent with the request. testSubscription = >smodel.DomainPermissionSubscription{ ID: "01JGE681TQSBPAV59GZXPKE62H", Priority: 255, @@ -309,7 +312,8 @@ func (suite *SubscriptionsTestSuite) TestDomainBlocksCSVETag() { CreatedByAccount: testAccount, URI: "https://lists.example.org/baddies.csv", ContentType: gtsmodel.DomainPermSubContentTypeCSV, - ETag: "bigbums6969", + ETag: "\"bigbums6969\"", + LastModified: testrig.TimeMustParse("2024-09-21T22:00:00Z"), } ) defer testrig.TearDownTestStructs(testStructs) @@ -339,7 +343,7 @@ func (suite *SubscriptionsTestSuite) TestDomainBlocksCSVETag() { } // The just-fetched perm sub should - // have ETag and count etc set now. + // have cache meta and count etc set now. permSub, err := testStructs.State.DB.GetDomainPermissionSubscriptionByID( ctx, testSubscription.ID, ) @@ -353,12 +357,157 @@ func (suite *SubscriptionsTestSuite) TestDomainBlocksCSVETag() { suite.FailNow(err.Error()) } - suite.Equal("bigbums6969", permSub.ETag) + suite.Equal("\"bigbums6969\"", permSub.ETag) + suite.EqualValues(1726956000, permSub.LastModified.Unix()) suite.Zero(count) suite.WithinDuration(time.Now(), permSub.FetchedAt, 1*time.Minute) suite.WithinDuration(time.Now(), permSub.SuccessfullyFetchedAt, 1*time.Minute) } +func (suite *SubscriptionsTestSuite) TestDomainBlocksCSVFutureLastModified() { + var ( + ctx = context.Background() + testStructs = testrig.SetupTestStructs(rMediaPath, rTemplatePath) + testAccount = suite.testAccounts["admin_account"] + subscriptions = subscriptions.New( + testStructs.State, + testStructs.TransportController, + testStructs.TypeConverter, + ) + + // Create a subscription for a CSV list of baddies. + // Request the future last modified value. + testSubscription = >smodel.DomainPermissionSubscription{ + ID: "01JGE681TQSBPAV59GZXPKE62H", + Priority: 255, + Title: "whatever!", + PermissionType: gtsmodel.DomainPermissionBlock, + AsDraft: util.Ptr(false), + AdoptOrphans: util.Ptr(true), + CreatedByAccountID: testAccount.ID, + CreatedByAccount: testAccount, + URI: "https://lists.example.org/baddies.csv?future=true", + ContentType: gtsmodel.DomainPermSubContentTypeCSV, + } + ) + defer testrig.TearDownTestStructs(testStructs) + + // Store test subscription. + if err := testStructs.State.DB.PutDomainPermissionSubscription( + ctx, testSubscription, + ); err != nil { + suite.FailNow(err.Error()) + } + + // Process all subscriptions. + subscriptions.ProcessDomainPermissionSubscriptions(ctx, testSubscription.PermissionType) + + // We should now have blocks for + // each domain on the subscribed list. + for _, domain := range []string{ + "bumfaces.net", + "peepee.poopoo", + "nothanks.com", + } { + var ( + perm gtsmodel.DomainPermission + err error + ) + if !testrig.WaitFor(func() bool { + perm, err = testStructs.State.DB.GetDomainBlock(ctx, domain) + return err == nil + }) { + suite.FailNowf("", "timed out waiting for domain %s", domain) + } + + suite.Equal(testSubscription.ID, perm.GetSubscriptionID()) + } + + // The just-fetched perm sub should have ETag + // set now, but last modified should be thrown away. + permSub, err := testStructs.State.DB.GetDomainPermissionSubscriptionByID( + ctx, testSubscription.ID, + ) + if err != nil { + suite.FailNow(err.Error()) + } + + suite.Equal("\"bigbums6969\"", permSub.ETag) + suite.Zero(permSub.LastModified) +} + +func (suite *SubscriptionsTestSuite) TestDomainBlocksCSVGarbageLastModified() { + var ( + ctx = context.Background() + testStructs = testrig.SetupTestStructs(rMediaPath, rTemplatePath) + testAccount = suite.testAccounts["admin_account"] + subscriptions = subscriptions.New( + testStructs.State, + testStructs.TransportController, + testStructs.TypeConverter, + ) + + // Create a subscription for a CSV list of baddies. + // Request the garbage last modified value. + testSubscription = >smodel.DomainPermissionSubscription{ + ID: "01JGE681TQSBPAV59GZXPKE62H", + Priority: 255, + Title: "whatever!", + PermissionType: gtsmodel.DomainPermissionBlock, + AsDraft: util.Ptr(false), + AdoptOrphans: util.Ptr(true), + CreatedByAccountID: testAccount.ID, + CreatedByAccount: testAccount, + URI: "https://lists.example.org/baddies.csv?garbage=true", + ContentType: gtsmodel.DomainPermSubContentTypeCSV, + } + ) + defer testrig.TearDownTestStructs(testStructs) + + // Store test subscription. + if err := testStructs.State.DB.PutDomainPermissionSubscription( + ctx, testSubscription, + ); err != nil { + suite.FailNow(err.Error()) + } + + // Process all subscriptions. + subscriptions.ProcessDomainPermissionSubscriptions(ctx, testSubscription.PermissionType) + + // We should now have blocks for + // each domain on the subscribed list. + for _, domain := range []string{ + "bumfaces.net", + "peepee.poopoo", + "nothanks.com", + } { + var ( + perm gtsmodel.DomainPermission + err error + ) + if !testrig.WaitFor(func() bool { + perm, err = testStructs.State.DB.GetDomainBlock(ctx, domain) + return err == nil + }) { + suite.FailNowf("", "timed out waiting for domain %s", domain) + } + + suite.Equal(testSubscription.ID, perm.GetSubscriptionID()) + } + + // The just-fetched perm sub should have ETag + // set now, but last modified should be thrown away. + permSub, err := testStructs.State.DB.GetDomainPermissionSubscriptionByID( + ctx, testSubscription.ID, + ) + if err != nil { + suite.FailNow(err.Error()) + } + + suite.Equal("\"bigbums6969\"", permSub.ETag) + suite.Zero(permSub.LastModified) +} + func (suite *SubscriptionsTestSuite) TestDomainBlocks404() { var ( ctx = context.Background() diff --git a/internal/transport/derefdomainpermlist.go b/internal/transport/derefdomainpermlist.go index c81117bc6..ba61c8b39 100644 --- a/internal/transport/derefdomainpermlist.go +++ b/internal/transport/derefdomainpermlist.go @@ -21,9 +21,11 @@ import ( "context" "io" "net/http" + "time" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/log" ) type DereferenceDomainPermissionsResp struct { @@ -39,6 +41,10 @@ type DereferenceDomainPermissionsResp struct { // May be set // if 200 or 304. ETag string + + // May be set + // if 200 or 304. + LastModified time.Time } func (t *transport) DereferenceDomainPermissions( @@ -60,27 +66,27 @@ func (t *transport) DereferenceDomainPermissions( // Set relevant Accept headers. // Allow fallback in case target doesn't // negotiate content type correctly. - req.Header.Add("Accept-Charset", "utf-8") - req.Header.Add("Accept", permSub.ContentType.String()+","+"*/*") + req.Header.Set("Accept-Charset", "utf-8") + req.Header.Set("Accept", permSub.ContentType.String()+","+"*/*") // If skipCache is true, we want to skip setting Cache // headers so that we definitely don't get a 304 back. if !skipCache { - // If we've successfully fetched this list - // before, set If-Modified-Since to last - // success to make the request conditional. + // If we've got a Last-Modified stored for this list, + // set If-Modified-Since to make the request conditional. // // See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/If-Modified-Since - if !permSub.SuccessfullyFetchedAt.IsZero() { - timeStr := permSub.SuccessfullyFetchedAt.Format(http.TimeFormat) - req.Header.Add("If-Modified-Since", timeStr) + if !permSub.LastModified.IsZero() { + // http.Time wants UTC. + lmUTC := permSub.LastModified.UTC() + req.Header.Set("If-Modified-Since", lmUTC.Format(http.TimeFormat)) } // If we've got an ETag stored for this list, set // If-None-Match to make the request conditional. // See https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/ETag#caching_of_unchanged_resources. - if len(permSub.ETag) != 0 { - req.Header.Add("If-None-Match", permSub.ETag) + if permSub.ETag != "" { + req.Header.Set("If-None-Match", permSub.ETag) } } @@ -99,11 +105,12 @@ func (t *transport) DereferenceDomainPermissions( return nil, err } - // Check already if we were given an ETag - // we can use, as ETag is often returned - // even on 304 Not Modified responses. + // Check already if we were given a valid ETag or + // Last-Modified we can use, as these cache headers + // are often returned even on Not Modified responses. permsResp := &DereferenceDomainPermissionsResp{ - ETag: rsp.Header.Get("Etag"), + ETag: rsp.Header.Get("ETag"), + LastModified: validateLastModified(ctx, rsp.Header.Get("Last-Modified")), } if rsp.StatusCode == http.StatusNotModified { @@ -119,3 +126,43 @@ func (t *transport) DereferenceDomainPermissions( return permsResp, nil } + +// Validate Last-Modified to ensure it's not +// garbagio, and not more than a minute in the +// future (to allow for clock issues + rounding). +func validateLastModified( + ctx context.Context, + lastModified string, +) time.Time { + if lastModified == "" { + // Not set, + // no problem. + return time.Time{} + } + + // Try to parse and see what we get. + switch lm, err := http.ParseTime(lastModified); { + case err != nil: + // No good, + // chuck it. + log.Debugf(ctx, + "discarding invalid Last-Modified header %s: %+v", + lastModified, err, + ) + return time.Time{} + + case lm.Unix() > time.Now().Add(1*time.Minute).Unix(): + // In the future, + // chuck it. + log.Debugf(ctx, + "discarding in-the-future Last-Modified header %s", + lastModified, + ) + return time.Time{} + + default: + // It's fine, + // keep it. + return lm + } +} diff --git a/internal/transport/transport.go b/internal/transport/transport.go index 45d43ff18..eb8f0cace 100644 --- a/internal/transport/transport.go +++ b/internal/transport/transport.go @@ -81,7 +81,7 @@ type Transport interface { // DereferenceDomainPermissions dereferences the // permissions list present at the given permSub's URI. // - // If "force", then If-Modified-Since and If-None-Match + // If "skipCache", then If-Modified-Since and If-None-Match // headers will *NOT* be sent with the outgoing request. // // If err == nil and Unmodified == false, then it's up @@ -89,7 +89,7 @@ type Transport interface { DereferenceDomainPermissions( ctx context.Context, permSub *gtsmodel.DomainPermissionSubscription, - force bool, + skipCache bool, ) (*DereferenceDomainPermissionsResp, error) // Finger performs a webfinger request with the given username and domain, and returns the bytes from the response body. |