From b7589324f1113dde7362abc26375f37ad6cce9c3 Mon Sep 17 00:00:00 2001 From: tobi Date: Wed, 8 Jan 2025 21:16:19 +0100 Subject: [PATCH] [bugfix] More permissive CSV parsing for perm subs, text parse fix --- ... domainpermissionsubscriptiontest_test.go} | 81 ++++++++++++- internal/subscriptions/domainperms.go | 109 +++++++++++++----- internal/typeutils/internaltofrontend.go | 2 +- 3 files changed, 159 insertions(+), 33 deletions(-) rename internal/api/client/admin/{domainpermissionsubscruptiontest_test.go => domainpermissionsubscriptiontest_test.go} (62%) diff --git a/internal/api/client/admin/domainpermissionsubscruptiontest_test.go b/internal/api/client/admin/domainpermissionsubscriptiontest_test.go similarity index 62% rename from internal/api/client/admin/domainpermissionsubscruptiontest_test.go rename to internal/api/client/admin/domainpermissionsubscriptiontest_test.go index 46861aba1..c03b950a9 100644 --- a/internal/api/client/admin/domainpermissionsubscruptiontest_test.go +++ b/internal/api/client/admin/domainpermissionsubscriptiontest_test.go @@ -39,7 +39,7 @@ type DomainPermissionSubscriptionTestTestSuite struct { AdminStandardTestSuite } -func (suite *DomainPermissionSubscriptionTestTestSuite) TestDomainPermissionSubscriptionTest() { +func (suite *DomainPermissionSubscriptionTestTestSuite) TestDomainPermissionSubscriptionTestCSV() { var ( ctx = context.Background() testAccount = suite.testAccounts["admin_account"] @@ -120,6 +120,85 @@ func (suite *DomainPermissionSubscriptionTestTestSuite) TestDomainPermissionSubs suite.False(blocked) } +func (suite *DomainPermissionSubscriptionTestTestSuite) TestDomainPermissionSubscriptionTestText() { + var ( + ctx = context.Background() + testAccount = suite.testAccounts["admin_account"] + permSub = >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.txt", + ContentType: gtsmodel.DomainPermSubContentTypePlain, + } + ) + + // Create a subscription for a plaintext list of baddies. + err := suite.state.DB.PutDomainPermissionSubscription(ctx, permSub) + if err != nil { + suite.FailNow(err.Error()) + } + + // Prepare the request to the /test endpoint. + subPath := strings.ReplaceAll( + admin.DomainPermissionSubscriptionTestPath, + ":id", permSub.ID, + ) + path := "/api" + subPath + recorder := httptest.NewRecorder() + ginCtx := suite.newContext(recorder, http.MethodPost, nil, path, "application/json") + ginCtx.Params = gin.Params{ + gin.Param{ + Key: apiutil.IDKey, + Value: permSub.ID, + }, + } + + // Trigger the handler. + suite.adminModule.DomainPermissionSubscriptionTestPOSTHandler(ginCtx) + suite.Equal(http.StatusOK, recorder.Code) + + // Read the body back. + b, err := io.ReadAll(recorder.Body) + if err != nil { + suite.FailNow(err.Error()) + } + + dst := new(bytes.Buffer) + if err := json.Indent(dst, b, "", " "); err != nil { + suite.FailNow(err.Error()) + } + + // Ensure expected. + suite.Equal(`[ + { + "domain": "bumfaces.net" + }, + { + "domain": "peepee.poopoo" + }, + { + "domain": "nothanks.com" + } +]`, dst.String()) + + // No permissions should be created + // since this is a dry run / test. + blocked, err := suite.state.DB.AreDomainsBlocked( + ctx, + []string{"bumfaces.net", "peepee.poopoo", "nothanks.com"}, + ) + if err != nil { + suite.FailNow(err.Error()) + } + suite.False(blocked) +} + func TestDomainPermissionSubscriptionTestTestSuite(t *testing.T) { suite.Run(t, &DomainPermissionSubscriptionTestTestSuite{}) } diff --git a/internal/subscriptions/domainperms.go b/internal/subscriptions/domainperms.go index b1e22a0be..db9d6ca1e 100644 --- a/internal/subscriptions/domainperms.go +++ b/internal/subscriptions/domainperms.go @@ -533,19 +533,60 @@ func permsFromCSV( return nil, gtserror.NewfAt(3, "error decoding csv column headers: %w", err) } - if !slices.Equal( - columnHeaders, - []string{ - "#domain", - "#severity", - "#reject_media", - "#reject_reports", - "#public_comment", - "#obfuscate", - }, - ) { + var ( + domainI *int + severityI *int + publicCommentI *int + obfuscateI *int + ) + + for i, columnHeader := range columnHeaders { + // Remove leading # if present. + normal := strings.TrimLeft(columnHeader, "#") + + // Find index of each column header we + // care about, ensuring no duplicates. + switch normal { + + case "domain": + if domainI != nil { + body.Close() + err := gtserror.NewfAt(3, "duplicate domain column header in csv: %+v", columnHeaders) + return nil, err + } + domainI = &i + + case "severity": + if severityI != nil { + body.Close() + err := gtserror.NewfAt(3, "duplicate severity column header in csv: %+v", columnHeaders) + return nil, err + } + severityI = &i + + case "public_comment": + if publicCommentI != nil { + body.Close() + err := gtserror.NewfAt(3, "duplicate public_comment column header in csv: %+v", columnHeaders) + return nil, err + } + publicCommentI = &i + + case "obfuscate": + if obfuscateI != nil { + body.Close() + err := gtserror.NewfAt(3, "duplicate obfuscate column header in csv: %+v", columnHeaders) + return nil, err + } + obfuscateI = &i + } + } + + // Ensure we have at least a domain + // index, as that's the bare minimum. + if domainI == nil { body.Close() - err := gtserror.NewfAt(3, "unexpected column headers in csv: %+v", columnHeaders) + err := gtserror.NewfAt(3, "no domain column header in csv: %+v", columnHeaders) return nil, err } @@ -576,25 +617,19 @@ func permsFromCSV( continue } - var ( - domainRaw = record[0] - severity = record[1] - publicComment = record[4] - obfuscateStr = record[5] - ) - - if severity != "suspend" { - l.Warnf("skipping non-suspend record: %+v", record) - continue - } - - obfuscate, err := strconv.ParseBool(obfuscateStr) - if err != nil { - l.Warnf("couldn't parse obfuscate field of record: %+v", record) - continue + // Skip records that specify severity + // that's not "suspend" (we don't support + // "silence" or "limit" or whatever yet). + if severityI != nil { + severity := record[*severityI] + if severity != "suspend" { + l.Warnf("skipping non-suspend record: %+v", record) + continue + } } // Normalize + validate domain. + domainRaw := record[*domainI] domain, err := validateDomain(domainRaw) if err != nil { l.Warnf("skipping invalid domain %s: %+v", domainRaw, err) @@ -611,9 +646,21 @@ func permsFromCSV( perm = >smodel.DomainAllow{Domain: domain} } - // Set remaining fields. - perm.SetPublicComment(publicComment) - perm.SetObfuscate(&obfuscate) + // Set remaining optional fields + // if they're present in the CSV. + if publicCommentI != nil { + perm.SetPublicComment(record[*publicCommentI]) + } + + if obfuscateI != nil { + obfuscate, err := strconv.ParseBool(record[*obfuscateI]) + if err != nil { + l.Warnf("couldn't parse obfuscate field of record: %+v", record) + continue + } + + perm.SetObfuscate(&obfuscate) + } // We're done. perms = append(perms, perm) diff --git a/internal/typeutils/internaltofrontend.go b/internal/typeutils/internaltofrontend.go index 2af479125..cdc250f98 100644 --- a/internal/typeutils/internaltofrontend.go +++ b/internal/typeutils/internaltofrontend.go @@ -2115,7 +2115,7 @@ func (c *Converter) DomainPermToAPIDomainPerm( } domainPerm.ID = d.GetID() - domainPerm.Obfuscate = *d.GetObfuscate() + domainPerm.Obfuscate = util.PtrOrZero(d.GetObfuscate()) domainPerm.PrivateComment = d.GetPrivateComment() domainPerm.SubscriptionID = d.GetSubscriptionID() domainPerm.CreatedBy = d.GetCreatedByAccountID()