Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: judge domain block and silence by endsWith (federation/instances) #12293

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

atsu1125
Copy link
Contributor

@atsu1125 atsu1125 commented Nov 9, 2023

What

ドメインブロックとサーバーサイレンスをendsWithで評価しているが
現状完全一致したものしか一覧には表示されないので整合性が取れてないため
federation/instancesendsWithに対応させる

Why

Fix: #9263
Fix: #12031

Additional info (optional)

tested on our developing environment

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

@atsu1125 atsu1125 force-pushed the fix-federation-instance-blocked branch from ac618cf to 4ff88d7 Compare November 9, 2023 12:31
@atsu1125 atsu1125 marked this pull request as ready for review November 9, 2023 12:31
@github-actions github-actions bot added the packages/backend Server side specific issue/PR label Nov 9, 2023
Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 39.87%. Comparing base (043ab1f) to head (5a559a0).

Files Patch % Lines
...d/src/server/api/endpoints/federation/instances.ts 0.00% 6 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #12293      +/-   ##
===========================================
- Coverage    39.89%   39.87%   -0.03%     
===========================================
  Files         1547     1547              
  Lines       190970   190970              
  Branches      3519     2655     -864     
===========================================
- Hits         76190    76140      -50     
- Misses      114187   114268      +81     
+ Partials       593      562      -31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Nov 9, 2023

このPRによるapi.jsonの差分

差分はこちら

Get diff files from Workflow Page

@syuilo
Copy link
Member

syuilo commented Nov 13, 2023

パフォーマンスへの影響が気になるわね

@atsu1125
Copy link
Contributor Author

一応チャート生成時に同様のクエリが走っているのでどうだろう

const [sub, pub, pubsub, subActive, pubActive] = await Promise.all([
this.followingsRepository.createQueryBuilder('following')
.select('COUNT(DISTINCT following.followeeHost)')
.where('following.followeeHost IS NOT NULL')
.andWhere(meta.blockedHosts.length === 0 ? '1=1' : 'following.followeeHost NOT ILIKE ANY(ARRAY[:...blocked])', { blocked: meta.blockedHosts.flatMap(x => [x, `%.${x}`]) })
.andWhere(`following.followeeHost NOT IN (${ suspendedInstancesQuery.getQuery() })`)
.getRawOne()
.then(x => parseInt(x.count, 10)),
this.followingsRepository.createQueryBuilder('following')
.select('COUNT(DISTINCT following.followerHost)')
.where('following.followerHost IS NOT NULL')
.andWhere(meta.blockedHosts.length === 0 ? '1=1' : 'following.followerHost NOT ILIKE ANY(ARRAY[:...blocked])', { blocked: meta.blockedHosts.flatMap(x => [x, `%.${x}`]) })
.andWhere(`following.followerHost NOT IN (${ suspendedInstancesQuery.getQuery() })`)
.getRawOne()
.then(x => parseInt(x.count, 10)),
this.followingsRepository.createQueryBuilder('following')
.select('COUNT(DISTINCT following.followeeHost)')
.where('following.followeeHost IS NOT NULL')
.andWhere(meta.blockedHosts.length === 0 ? '1=1' : 'following.followeeHost NOT ILIKE ANY(ARRAY[:...blocked])', { blocked: meta.blockedHosts.flatMap(x => [x, `%.${x}`]) })
.andWhere(`following.followeeHost NOT IN (${ suspendedInstancesQuery.getQuery() })`)
.andWhere(`following.followeeHost IN (${ pubsubSubQuery.getQuery() })`)
.setParameters(pubsubSubQuery.getParameters())
.getRawOne()
.then(x => parseInt(x.count, 10)),
this.instancesRepository.createQueryBuilder('instance')
.select('COUNT(instance.id)')
.where(`instance.host IN (${ subInstancesQuery.getQuery() })`)
.andWhere(meta.blockedHosts.length === 0 ? '1=1' : 'instance.host NOT ILIKE ANY(ARRAY[:...blocked])', { blocked: meta.blockedHosts.flatMap(x => [x, `%.${x}`]) })
.andWhere('instance.isSuspended = false')
.andWhere('instance.isNotResponding = false')
.getRawOne()
.then(x => parseInt(x.count, 10)),
this.instancesRepository.createQueryBuilder('instance')
.select('COUNT(instance.id)')
.where(`instance.host IN (${ pubInstancesQuery.getQuery() })`)
.andWhere(meta.blockedHosts.length === 0 ? '1=1' : 'instance.host NOT ILIKE ANY(ARRAY[:...blocked])', { blocked: meta.blockedHosts.flatMap(x => [x, `%.${x}`]) })
.andWhere('instance.isSuspended = false')
.andWhere('instance.isNotResponding = false')
.getRawOne()
.then(x => parseInt(x.count, 10)),
]);

@syuilo
Copy link
Member

syuilo commented Nov 13, 2023

ふーむ
そこも含めて実装を見直したくなってきた

@atsu1125
Copy link
Contributor Author

atsu1125 commented Aug 21, 2024

#14441 の対応とPR作成から9ヶ月経ってコンフリクト出てるのもあるので、今のdevelopベースで書き直しておきますね
パフォーマンスについては

allowGet: true,
cacheSec: 3600,

があり、すでにチャートのコードで毎時で呼ばれているクエリであり、60分というキャッシュ時間とクエリの実行頻度は変わらないので、問題はないと結論付けています

@atsu1125 atsu1125 marked this pull request as draft August 21, 2024 05:07
@atsu1125 atsu1125 force-pushed the fix-federation-instance-blocked branch from 73316f7 to e59d64f Compare August 21, 2024 12:31
@atsu1125 atsu1125 force-pushed the fix-federation-instance-blocked branch from e59d64f to 5a559a0 Compare August 21, 2024 12:33
@atsu1125 atsu1125 marked this pull request as ready for review August 21, 2024 12:44
@atsu1125
Copy link
Contributor Author

今のDevelopベースで書き直しました

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/backend Server side specific issue/PR
Projects
Development

Successfully merging this pull request may close these issues.

2 participants