From 23376cb6913caca4c36db77cecfe8d24cd92946f Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 4 Dec 2024 08:41:21 +0100 Subject: [PATCH 01/14] Fix `NameError` in status update processing (#33161) --- app/services/activitypub/process_status_update_service.rb | 2 +- .../activitypub/process_status_update_service_spec.rb | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index e2b752227d..32f563013b 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -204,7 +204,7 @@ class ActivityPub::ProcessStatusUpdateService < BaseService mention ||= account.mentions.new(status: @status) current_mentions << mention - rescue Mastodon::UnexpectedResponseError, *Mastodon::HTTP_CONNECTION_ERRORS + rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError # Since previous mentions are about already-known accounts, # they don't try to resolve again and won't fall into this case. # In other words, this failure case is only for new mentions and won't diff --git a/spec/services/activitypub/process_status_update_service_spec.rb b/spec/services/activitypub/process_status_update_service_spec.rb index b6ceba374f..498b3c8aa0 100644 --- a/spec/services/activitypub/process_status_update_service_spec.rb +++ b/spec/services/activitypub/process_status_update_service_spec.rb @@ -6,6 +6,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do subject { described_class.new } let!(:status) { Fabricate(:status, text: 'Hello world', account: Fabricate(:account, domain: 'example.com')) } + let(:bogus_mention) { 'https://example.com/users/erroringuser' } let(:payload) do { '@context': 'https://www.w3.org/ns/activitystreams', @@ -17,6 +18,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do tag: [ { type: 'Hashtag', name: 'hoge' }, { type: 'Mention', href: ActivityPub::TagManager.instance.uri_for(alice) }, + { type: 'Mention', href: bogus_mention }, ], } end @@ -33,16 +35,18 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do mentions.each { |a| Fabricate(:mention, status: status, account: a) } tags.each { |t| status.tags << t } media_attachments.each { |m| status.media_attachments << m } + stub_request(:get, bogus_mention).to_raise(HTTP::ConnectionError) end describe '#call' do - it 'updates text and content warning' do + it 'updates text and content warning, and schedules re-fetching broken mention' do subject.call(status, json, json) expect(status.reload) .to have_attributes( text: eq('Hello universe'), spoiler_text: eq('Show more') ) + expect(MentionResolveWorker).to have_enqueued_sidekiq_job(status.id, bogus_mention, anything) end context 'when the changes are only in sanitized-out HTML' do From 3a4242ce016692d990bb8fdb2c91646f92996f27 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 16 Jan 2025 11:10:08 +0100 Subject: [PATCH 02/14] Merge commit from fork --- app/lib/delivery_failure_tracker.rb | 2 ++ .../activitypub/process_account_service.rb | 27 ++++++++++++++----- spec/lib/delivery_failure_tracker_spec.rb | 4 +-- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/app/lib/delivery_failure_tracker.rb b/app/lib/delivery_failure_tracker.rb index e17b45d667..96292923f4 100644 --- a/app/lib/delivery_failure_tracker.rb +++ b/app/lib/delivery_failure_tracker.rb @@ -46,6 +46,8 @@ class DeliveryFailureTracker urls.reject do |url| host = Addressable::URI.parse(url).normalized_host unavailable_domains_map[host] + rescue Addressable::URI::InvalidURIError, IDN::Idna::IdnaError + true end end diff --git a/app/services/activitypub/process_account_service.rb b/app/services/activitypub/process_account_service.rb index a7422b5d02..f9b7789aec 100644 --- a/app/services/activitypub/process_account_service.rb +++ b/app/services/activitypub/process_account_service.rb @@ -9,6 +9,8 @@ class ActivityPub::ProcessAccountService < BaseService SUBDOMAINS_RATELIMIT = 10 DISCOVERIES_PER_REQUEST = 400 + VALID_URI_SCHEMES = %w(http https).freeze + # Should be called with confirmed valid JSON # and WebFinger-resolved username and domain def call(username, domain, json, options = {}) @@ -96,16 +98,28 @@ class ActivityPub::ProcessAccountService < BaseService end def set_immediate_protocol_attributes! - @account.inbox_url = @json['inbox'] || '' - @account.outbox_url = @json['outbox'] || '' - @account.shared_inbox_url = (@json['endpoints'].is_a?(Hash) ? @json['endpoints']['sharedInbox'] : @json['sharedInbox']) || '' - @account.followers_url = @json['followers'] || '' + @account.inbox_url = valid_collection_uri(@json['inbox']) + @account.outbox_url = valid_collection_uri(@json['outbox']) + @account.shared_inbox_url = valid_collection_uri(@json['endpoints'].is_a?(Hash) ? @json['endpoints']['sharedInbox'] : @json['sharedInbox']) + @account.followers_url = valid_collection_uri(@json['followers']) @account.url = url || @uri @account.uri = @uri @account.actor_type = actor_type @account.created_at = @json['published'] if @json['published'].present? end + def valid_collection_uri(uri) + uri = uri.first if uri.is_a?(Array) + uri = uri['id'] if uri.is_a?(Hash) + return '' unless uri.is_a?(String) + + parsed_uri = Addressable::URI.parse(uri) + + VALID_URI_SCHEMES.include?(parsed_uri.scheme) && parsed_uri.host.present? ? parsed_uri : '' + rescue Addressable::URI::InvalidURIError + '' + end + def set_immediate_attributes! @account.featured_collection_url = @json['featured'] || '' @account.display_name = @json['name'] || '' @@ -268,10 +282,11 @@ class ActivityPub::ProcessAccountService < BaseService end def collection_info(type) - return [nil, nil] if @json[type].blank? + collection_uri = valid_collection_uri(@json[type]) + return [nil, nil] if collection_uri.blank? return @collections[type] if @collections.key?(type) - collection = fetch_resource_without_id_validation(@json[type]) + collection = fetch_resource_without_id_validation(collection_uri) total_items = collection.is_a?(Hash) && collection['totalItems'].present? && collection['totalItems'].is_a?(Numeric) ? collection['totalItems'] : nil has_first_page = collection.is_a?(Hash) && collection['first'].present? diff --git a/spec/lib/delivery_failure_tracker_spec.rb b/spec/lib/delivery_failure_tracker_spec.rb index 40c8adc4c8..34912c8133 100644 --- a/spec/lib/delivery_failure_tracker_spec.rb +++ b/spec/lib/delivery_failure_tracker_spec.rb @@ -42,8 +42,8 @@ RSpec.describe DeliveryFailureTracker do Fabricate(:unavailable_domain, domain: 'foo.bar') end - it 'removes URLs that are unavailable' do - results = described_class.without_unavailable(['http://example.com/good/inbox', 'http://foo.bar/unavailable/inbox']) + it 'removes URLs that are bogus or unavailable' do + results = described_class.without_unavailable(['http://example.com/good/inbox', 'http://foo.bar/unavailable/inbox', '{foo:']) expect(results).to include('http://example.com/good/inbox') expect(results).to_not include('http://foo.bar/unavailable/inbox') From bcc798d6a78535ef69700349c9eefc1554ea365d Mon Sep 17 00:00:00 2001 From: Matt Jankowski Date: Wed, 4 Dec 2024 04:05:58 -0500 Subject: [PATCH 03/14] Fix empty authors preview card serialization (#33151) --- app/models/preview_card.rb | 9 +++- .../rest/preview_card_serializer_spec.rb | 41 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 spec/serializers/rest/preview_card_serializer_spec.rb diff --git a/app/models/preview_card.rb b/app/models/preview_card.rb index 7579178f83..f6f37c8c82 100644 --- a/app/models/preview_card.rb +++ b/app/models/preview_card.rb @@ -134,7 +134,7 @@ class PreviewCard < ApplicationRecord end def authors - @authors ||= [PreviewCard::Author.new(self)] + @authors ||= Array(serialized_authors) end class Author < ActiveModelSerializers::Model @@ -169,6 +169,13 @@ class PreviewCard < ApplicationRecord private + def serialized_authors + if author_name? || author_url? + PreviewCard::Author + .new(self) + end + end + def extract_dimensions file = image.queued_for_write[:original] diff --git a/spec/serializers/rest/preview_card_serializer_spec.rb b/spec/serializers/rest/preview_card_serializer_spec.rb new file mode 100644 index 0000000000..6dbc337865 --- /dev/null +++ b/spec/serializers/rest/preview_card_serializer_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe REST::PreviewCardSerializer do + subject do + serialized_record_json( + preview_card, + described_class + ) + end + + context 'when preview card does not have author data' do + let(:preview_card) { Fabricate.build :preview_card } + + it 'includes empty authors array' do + expect(subject.deep_symbolize_keys) + .to include( + authors: be_an(Array).and(be_empty) + ) + end + end + + context 'when preview card has author data' do + let(:preview_card) { Fabricate.build :preview_card, author_name: 'Name', author_url: 'https://host.example/123' } + + it 'includes populated authors array' do + expect(subject.deep_symbolize_keys) + .to include( + authors: be_an(Array).and( + contain_exactly( + include( + name: 'Name', + url: 'https://host.example/123' + ) + ) + ) + ) + end + end +end From 734f0dd182208f351d65b7fceba88c9a53109cc9 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 6 Jan 2025 11:04:25 +0100 Subject: [PATCH 04/14] Fix `fediverse:creator` metadata not showing up in REST API (#33466) --- app/models/preview_card.rb | 2 +- .../rest/preview_card_serializer_spec.rb | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/app/models/preview_card.rb b/app/models/preview_card.rb index f6f37c8c82..56fe483635 100644 --- a/app/models/preview_card.rb +++ b/app/models/preview_card.rb @@ -170,7 +170,7 @@ class PreviewCard < ApplicationRecord private def serialized_authors - if author_name? || author_url? + if author_name? || author_url? || author_account_id? PreviewCard::Author .new(self) end diff --git a/spec/serializers/rest/preview_card_serializer_spec.rb b/spec/serializers/rest/preview_card_serializer_spec.rb index 6dbc337865..41ba305b7c 100644 --- a/spec/serializers/rest/preview_card_serializer_spec.rb +++ b/spec/serializers/rest/preview_card_serializer_spec.rb @@ -21,7 +21,24 @@ RSpec.describe REST::PreviewCardSerializer do end end - context 'when preview card has author data' do + context 'when preview card has fediverse author data' do + let(:preview_card) { Fabricate.build :preview_card, author_account: Fabricate(:account) } + + it 'includes populated authors array' do + expect(subject.deep_symbolize_keys) + .to include( + authors: be_an(Array).and( + contain_exactly( + include( + account: be_present + ) + ) + ) + ) + end + end + + context 'when preview card has non-fediverse author data' do let(:preview_card) { Fabricate.build :preview_card, author_name: 'Name', author_url: 'https://host.example/123' } it 'includes populated authors array' do From afcfc6400759729fb2870b70ff1f854cc3e2736d Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 5 Dec 2024 10:38:48 +0100 Subject: [PATCH 05/14] Fix deletion of unconfirmed users with Webauthn set (#33186) --- app/workers/scheduler/user_cleanup_scheduler.rb | 1 + spec/workers/scheduler/user_cleanup_scheduler_spec.rb | 3 +++ 2 files changed, 4 insertions(+) diff --git a/app/workers/scheduler/user_cleanup_scheduler.rb b/app/workers/scheduler/user_cleanup_scheduler.rb index f755128332..03544e2e98 100644 --- a/app/workers/scheduler/user_cleanup_scheduler.rb +++ b/app/workers/scheduler/user_cleanup_scheduler.rb @@ -19,6 +19,7 @@ class Scheduler::UserCleanupScheduler User.unconfirmed.where(confirmation_sent_at: ..UNCONFIRMED_ACCOUNTS_MAX_AGE_DAYS.days.ago).find_in_batches do |batch| # We have to do it separately because of missing database constraints AccountModerationNote.where(target_account_id: batch.map(&:account_id)).delete_all + WebauthnCredential.where(user_id: batch.map(&:id)).delete_all Account.where(id: batch.map(&:account_id)).delete_all User.where(id: batch.map(&:id)).delete_all end diff --git a/spec/workers/scheduler/user_cleanup_scheduler_spec.rb b/spec/workers/scheduler/user_cleanup_scheduler_spec.rb index b1be7c4611..604f528586 100644 --- a/spec/workers/scheduler/user_cleanup_scheduler_spec.rb +++ b/spec/workers/scheduler/user_cleanup_scheduler_spec.rb @@ -9,6 +9,7 @@ RSpec.describe Scheduler::UserCleanupScheduler do let!(:old_unconfirmed_user) { Fabricate(:user) } let!(:confirmed_user) { Fabricate(:user) } let!(:moderation_note) { Fabricate(:account_moderation_note, account: Fabricate(:account), target_account: old_unconfirmed_user.account) } + let!(:webauthn_credential) { Fabricate(:webauthn_credential, user_id: old_unconfirmed_user.id) } describe '#perform' do before do @@ -26,6 +27,8 @@ RSpec.describe Scheduler::UserCleanupScheduler do .from(true).to(false) expect { moderation_note.reload } .to raise_error(ActiveRecord::RecordNotFound) + expect { webauthn_credential.reload } + .to raise_error(ActiveRecord::RecordNotFound) expect_preservation_of(new_unconfirmed_user) expect_preservation_of(confirmed_user) end From 533477e77cf5255125a565e8404d4249e8f8b86e Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 15 Jan 2025 11:59:28 +0100 Subject: [PATCH 06/14] Fix processing of mentions for post edits with an existing corresponding silent mention (#33227) --- .../process_status_update_service.rb | 20 +++++-------------- app/services/process_mentions_service.rb | 8 +++++--- app/workers/mention_resolve_worker.rb | 2 +- .../process_status_update_service_spec.rb | 16 +++++++++++++-- spec/services/update_status_service_spec.rb | 8 ++++++++ 5 files changed, 33 insertions(+), 21 deletions(-) diff --git a/app/services/activitypub/process_status_update_service.rb b/app/services/activitypub/process_status_update_service.rb index 32f563013b..e68abe9728 100644 --- a/app/services/activitypub/process_status_update_service.rb +++ b/app/services/activitypub/process_status_update_service.rb @@ -188,40 +188,30 @@ class ActivityPub::ProcessStatusUpdateService < BaseService end def update_mentions! - previous_mentions = @status.active_mentions.includes(:account).to_a - current_mentions = [] unresolved_mentions = [] - @raw_mentions.each do |href| + currently_mentioned_account_ids = @raw_mentions.filter_map do |href| next if href.blank? account = ActivityPub::TagManager.instance.uri_to_resource(href, Account) account ||= ActivityPub::FetchRemoteAccountService.new.call(href, request_id: @request_id) - next if account.nil? - - mention = previous_mentions.find { |x| x.account_id == account.id } - mention ||= account.mentions.new(status: @status) - - current_mentions << mention + account&.id rescue Mastodon::UnexpectedResponseError, HTTP::TimeoutError, HTTP::ConnectionError, OpenSSL::SSL::SSLError # Since previous mentions are about already-known accounts, # they don't try to resolve again and won't fall into this case. # In other words, this failure case is only for new mentions and won't # affect `removed_mentions` so they can safely be retried asynchronously unresolved_mentions << href + nil end - current_mentions.each do |mention| - mention.save if mention.new_record? - end + @status.mentions.upsert_all(currently_mentioned_account_ids.map { |id| { account_id: id, silent: false } }, unique_by: %w(status_id account_id)) # If previous mentions are no longer contained in the text, convert them # to silent mentions, since withdrawing access from someone who already # received a notification might be more confusing - removed_mentions = previous_mentions - current_mentions - - Mention.where(id: removed_mentions.map(&:id)).update_all(silent: true) unless removed_mentions.empty? + @status.mentions.where.not(account_id: currently_mentioned_account_ids).update_all(silent: true) # Queue unresolved mentions for later unresolved_mentions.uniq.each do |uri| diff --git a/app/services/process_mentions_service.rb b/app/services/process_mentions_service.rb index 1c4c7805f1..06777bee95 100644 --- a/app/services/process_mentions_service.rb +++ b/app/services/process_mentions_service.rb @@ -13,7 +13,7 @@ class ProcessMentionsService < BaseService return unless @status.local? - @previous_mentions = @status.active_mentions.includes(:account).to_a + @previous_mentions = @status.mentions.includes(:account).to_a @current_mentions = [] Status.transaction do @@ -57,6 +57,8 @@ class ProcessMentionsService < BaseService mention ||= @current_mentions.find { |x| x.account_id == mentioned_account.id } mention ||= @status.mentions.new(account: mentioned_account) + mention.silent = false + @current_mentions << mention "@#{mentioned_account.acct}" @@ -78,7 +80,7 @@ class ProcessMentionsService < BaseService end @current_mentions.each do |mention| - mention.save if mention.new_record? && @save_records + mention.save if (mention.new_record? || mention.silent_changed?) && @save_records end # If previous mentions are no longer contained in the text, convert them @@ -86,7 +88,7 @@ class ProcessMentionsService < BaseService # received a notification might be more confusing removed_mentions = @previous_mentions - @current_mentions - Mention.where(id: removed_mentions.map(&:id)).update_all(silent: true) unless removed_mentions.empty? + Mention.where(id: removed_mentions.map(&:id), silent: false).update_all(silent: true) unless removed_mentions.empty? end def mention_undeliverable?(mentioned_account) diff --git a/app/workers/mention_resolve_worker.rb b/app/workers/mention_resolve_worker.rb index 72dcd9633f..8c5938aeaf 100644 --- a/app/workers/mention_resolve_worker.rb +++ b/app/workers/mention_resolve_worker.rb @@ -16,7 +16,7 @@ class MentionResolveWorker return if account.nil? - status.mentions.create!(account: account, silent: false) + status.mentions.upsert({ account_id: account.id, silent: false }, unique_by: %w(status_id account_id)) rescue ActiveRecord::RecordNotFound # Do nothing rescue Mastodon::UnexpectedResponseError => e diff --git a/spec/services/activitypub/process_status_update_service_spec.rb b/spec/services/activitypub/process_status_update_service_spec.rb index 498b3c8aa0..ed6e8e3167 100644 --- a/spec/services/activitypub/process_status_update_service_spec.rb +++ b/spec/services/activitypub/process_status_update_service_spec.rb @@ -32,7 +32,7 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do let(:media_attachments) { [] } before do - mentions.each { |a| Fabricate(:mention, status: status, account: a) } + mentions.each { |(account, silent)| Fabricate(:mention, status: status, account: account, silent: silent) } tags.each { |t| status.tags << t } media_attachments.each { |m| status.media_attachments << m } stub_request(:get, bogus_mention).to_raise(HTTP::ConnectionError) @@ -280,7 +280,19 @@ RSpec.describe ActivityPub::ProcessStatusUpdateService do end context 'when originally with mentions' do - let(:mentions) { [alice, bob] } + let(:mentions) { [[alice, false], [bob, false]] } + + before do + subject.call(status, json, json) + end + + it 'updates mentions' do + expect(status.active_mentions.reload.map(&:account_id)).to eq [alice.id] + end + end + + context 'when originally with silent mentions' do + let(:mentions) { [[alice, true], [bob, true]] } before do subject.call(status, json, json) diff --git a/spec/services/update_status_service_spec.rb b/spec/services/update_status_service_spec.rb index 7c92adeffd..463605dd22 100644 --- a/spec/services/update_status_service_spec.rb +++ b/spec/services/update_status_service_spec.rb @@ -150,6 +150,14 @@ RSpec.describe UpdateStatusService do .to eq [bob.id] expect(status.mentions.pluck(:account_id)) .to contain_exactly(alice.id, bob.id) + + # Going back when a mention was switched to silence should still be possible + subject.call(status, status.account_id, text: 'Hello @alice') + + expect(status.active_mentions.pluck(:account_id)) + .to eq [alice.id] + expect(status.mentions.pluck(:account_id)) + .to contain_exactly(alice.id, bob.id) end end From 3cff7caffd704521a89a78398fa719cf1609c5bb Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 9 Jan 2025 14:47:12 +0100 Subject: [PATCH 07/14] Fix last paginated notification group only including data on a single notification (#33271) --- .../api/v2/notifications_controller.rb | 23 ++++++++- app/models/notification_group.rb | 26 +++++++--- spec/requests/api/v2/notifications_spec.rb | 49 +++++++++++++++++++ 3 files changed, 89 insertions(+), 9 deletions(-) diff --git a/app/controllers/api/v2/notifications_controller.rb b/app/controllers/api/v2/notifications_controller.rb index c070c0e5e7..cc38b95114 100644 --- a/app/controllers/api/v2/notifications_controller.rb +++ b/app/controllers/api/v2/notifications_controller.rb @@ -80,10 +80,31 @@ class Api::V2::NotificationsController < Api::BaseController return [] if @notifications.empty? MastodonOTELTracer.in_span('Api::V2::NotificationsController#load_grouped_notifications') do - NotificationGroup.from_notifications(@notifications, pagination_range: (@notifications.last.id)..(@notifications.first.id), grouped_types: params[:grouped_types]) + pagination_range = (@notifications.last.id)..@notifications.first.id + + # If the page is incomplete, we know we are on the last page + if incomplete_page? + if paginating_up? + pagination_range = @notifications.last.id...(params[:max_id]&.to_i) + else + range_start = params[:since_id]&.to_i + range_start += 1 unless range_start.nil? + pagination_range = range_start..(@notifications.first.id) + end + end + + NotificationGroup.from_notifications(@notifications, pagination_range: pagination_range, grouped_types: params[:grouped_types]) end end + def incomplete_page? + @notifications.size < limit_param(DEFAULT_NOTIFICATIONS_LIMIT) + end + + def paginating_up? + params[:min_id].present? + end + def browserable_account_notifications current_account.notifications.without_suspended.browserable( types: Array(browserable_params[:types]), diff --git a/app/models/notification_group.rb b/app/models/notification_group.rb index b6aa4d309c..0201657f1b 100644 --- a/app/models/notification_group.rb +++ b/app/models/notification_group.rb @@ -63,21 +63,31 @@ class NotificationGroup < ActiveModelSerializers::Model binds = [ account_id, SAMPLE_ACCOUNTS_SIZE, - pagination_range.begin, - pagination_range.end, ActiveRecord::Relation::QueryAttribute.new('group_keys', group_keys, ActiveRecord::ConnectionAdapters::PostgreSQL::OID::Array.new(ActiveModel::Type::String.new)), + pagination_range.begin || 0, ] + binds << pagination_range.end unless pagination_range.end.nil? + + upper_bound_cond = begin + if pagination_range.end.nil? + '' + elsif pagination_range.exclude_end? + 'AND id < $5' + else + 'AND id <= $5' + end + end ActiveRecord::Base.connection.select_all(<<~SQL.squish, 'grouped_notifications', binds).cast_values.to_h { |k, *values| [k, values] } SELECT groups.group_key, - (SELECT id FROM notifications WHERE notifications.account_id = $1 AND notifications.group_key = groups.group_key AND id <= $4 ORDER BY id DESC LIMIT 1), - array(SELECT from_account_id FROM notifications WHERE notifications.account_id = $1 AND notifications.group_key = groups.group_key AND id <= $4 ORDER BY id DESC LIMIT $2), - (SELECT count(*) FROM notifications WHERE notifications.account_id = $1 AND notifications.group_key = groups.group_key AND id <= $4) AS notifications_count, - (SELECT id FROM notifications WHERE notifications.account_id = $1 AND notifications.group_key = groups.group_key AND id >= $3 ORDER BY id ASC LIMIT 1) AS min_id, - (SELECT created_at FROM notifications WHERE notifications.account_id = $1 AND notifications.group_key = groups.group_key AND id <= $4 ORDER BY id DESC LIMIT 1) + (SELECT id FROM notifications WHERE notifications.account_id = $1 AND notifications.group_key = groups.group_key #{upper_bound_cond} ORDER BY id DESC LIMIT 1), + array(SELECT from_account_id FROM notifications WHERE notifications.account_id = $1 AND notifications.group_key = groups.group_key #{upper_bound_cond} ORDER BY id DESC LIMIT $2), + (SELECT count(*) FROM notifications WHERE notifications.account_id = $1 AND notifications.group_key = groups.group_key #{upper_bound_cond}) AS notifications_count, + (SELECT id FROM notifications WHERE notifications.account_id = $1 AND notifications.group_key = groups.group_key AND id >= $4 ORDER BY id ASC LIMIT 1) AS min_id, + (SELECT created_at FROM notifications WHERE notifications.account_id = $1 AND notifications.group_key = groups.group_key #{upper_bound_cond} ORDER BY id DESC LIMIT 1) FROM - unnest($5::text[]) AS groups(group_key); + unnest($3::text[]) AS groups(group_key); SQL else binds = [ diff --git a/spec/requests/api/v2/notifications_spec.rb b/spec/requests/api/v2/notifications_spec.rb index ffa0a71c77..aa4a861557 100644 --- a/spec/requests/api/v2/notifications_spec.rb +++ b/spec/requests/api/v2/notifications_spec.rb @@ -143,6 +143,55 @@ RSpec.describe 'Notifications' do end end + context 'when there are numerous notifications for the same final group' do + before do + user.account.notifications.destroy_all + 5.times.each { FavouriteService.new.call(Fabricate(:account), user.account.statuses.first) } + end + + context 'with no options' do + it 'returns a notification group covering all notifications' do + subject + + notification_ids = user.account.notifications.reload.pluck(:id) + + expect(response).to have_http_status(200) + expect(response.content_type) + .to start_with('application/json') + expect(response.parsed_body[:notification_groups]).to contain_exactly( + a_hash_including( + type: 'favourite', + sample_account_ids: have_attributes(size: 5), + page_min_id: notification_ids.first.to_s, + page_max_id: notification_ids.last.to_s + ) + ) + end + end + + context 'with min_id param' do + let(:params) { { min_id: user.account.notifications.reload.first.id - 1 } } + + it 'returns a notification group covering all notifications' do + subject + + notification_ids = user.account.notifications.reload.pluck(:id) + + expect(response).to have_http_status(200) + expect(response.content_type) + .to start_with('application/json') + expect(response.parsed_body[:notification_groups]).to contain_exactly( + a_hash_including( + type: 'favourite', + sample_account_ids: have_attributes(size: 5), + page_min_id: notification_ids.first.to_s, + page_max_id: notification_ids.last.to_s + ) + ) + end + end + end + context 'with no options' do it 'returns expected notification types', :aggregate_failures do subject From 757aed32907c5a8744234a61621e54c4858a1712 Mon Sep 17 00:00:00 2001 From: Claire Date: Mon, 16 Dec 2024 13:33:55 +0100 Subject: [PATCH 08/14] Fix error decrementing status count when `FeaturedTags#last_status_at` is `nil` (#33320) --- app/models/featured_tag.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/featured_tag.rb b/app/models/featured_tag.rb index 529056f9c6..dfc700649c 100644 --- a/app/models/featured_tag.rb +++ b/app/models/featured_tag.rb @@ -47,7 +47,7 @@ class FeaturedTag < ApplicationRecord def decrement(deleted_status) if statuses_count <= 1 update(statuses_count: 0, last_status_at: nil) - elsif last_status_at > deleted_status.created_at + elsif last_status_at.present? && last_status_at > deleted_status.created_at update(statuses_count: statuses_count - 1) else # Fetching the latest status creation time can be expensive, so only perform it From d764ae017d5b80c585ea92d2f433eaa0c09ecd7f Mon Sep 17 00:00:00 2001 From: Jesse Karmani Date: Wed, 18 Dec 2024 00:52:47 -0800 Subject: [PATCH 09/14] Fix down clause for notification policy v2 migrations (#33340) --- db/migrate/20240808124338_migrate_notifications_policy_v2.rb | 2 +- ...808124339_post_deployment_migrate_notifications_policy_v2.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/db/migrate/20240808124338_migrate_notifications_policy_v2.rb b/db/migrate/20240808124338_migrate_notifications_policy_v2.rb index 2e0684826a..ba8f809c66 100644 --- a/db/migrate/20240808124338_migrate_notifications_policy_v2.rb +++ b/db/migrate/20240808124338_migrate_notifications_policy_v2.rb @@ -18,7 +18,7 @@ class MigrateNotificationsPolicyV2 < ActiveRecord::Migration[7.1] def down NotificationPolicy.in_batches.update_all(<<~SQL.squish) filter_not_following = CASE for_not_following WHEN 0 THEN false ELSE true END, - filter_not_following = CASE for_not_followers WHEN 0 THEN false ELSE true END, + filter_not_followers = CASE for_not_followers WHEN 0 THEN false ELSE true END, filter_new_accounts = CASE for_new_accounts WHEN 0 THEN false ELSE true END, filter_private_mentions = CASE for_private_mentions WHEN 0 THEN false ELSE true END SQL diff --git a/db/post_migrate/20240808124339_post_deployment_migrate_notifications_policy_v2.rb b/db/post_migrate/20240808124339_post_deployment_migrate_notifications_policy_v2.rb index eb0c909729..11b2b5bd2a 100644 --- a/db/post_migrate/20240808124339_post_deployment_migrate_notifications_policy_v2.rb +++ b/db/post_migrate/20240808124339_post_deployment_migrate_notifications_policy_v2.rb @@ -18,7 +18,7 @@ class PostDeploymentMigrateNotificationsPolicyV2 < ActiveRecord::Migration[7.1] def down NotificationPolicy.in_batches.update_all(<<~SQL.squish) filter_not_following = CASE for_not_following WHEN 0 THEN false ELSE true END, - filter_not_following = CASE for_not_followers WHEN 0 THEN false ELSE true END, + filter_not_followers = CASE for_not_followers WHEN 0 THEN false ELSE true END, filter_new_accounts = CASE for_new_accounts WHEN 0 THEN false ELSE true END, filter_private_mentions = CASE for_private_mentions WHEN 0 THEN false ELSE true END SQL From 512bfc0a54e104bd581d629fb1b9a0eef6e7e7e5 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 18 Dec 2024 13:25:40 +0100 Subject: [PATCH 10/14] Fix incorrect notification settings migration for non-followers (#33348) --- db/migrate/20240808124338_migrate_notifications_policy_v2.rb | 2 +- ...808124339_post_deployment_migrate_notifications_policy_v2.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/db/migrate/20240808124338_migrate_notifications_policy_v2.rb b/db/migrate/20240808124338_migrate_notifications_policy_v2.rb index ba8f809c66..ed1642d6b8 100644 --- a/db/migrate/20240808124338_migrate_notifications_policy_v2.rb +++ b/db/migrate/20240808124338_migrate_notifications_policy_v2.rb @@ -9,7 +9,7 @@ class MigrateNotificationsPolicyV2 < ActiveRecord::Migration[7.1] def up NotificationPolicy.in_batches.update_all(<<~SQL.squish) for_not_following = CASE filter_not_following WHEN true THEN 1 ELSE 0 END, - for_not_followers = CASE filter_not_following WHEN true THEN 1 ELSE 0 END, + for_not_followers = CASE filter_not_followers WHEN true THEN 1 ELSE 0 END, for_new_accounts = CASE filter_new_accounts WHEN true THEN 1 ELSE 0 END, for_private_mentions = CASE filter_private_mentions WHEN true THEN 1 ELSE 0 END SQL diff --git a/db/post_migrate/20240808124339_post_deployment_migrate_notifications_policy_v2.rb b/db/post_migrate/20240808124339_post_deployment_migrate_notifications_policy_v2.rb index 11b2b5bd2a..5daf646643 100644 --- a/db/post_migrate/20240808124339_post_deployment_migrate_notifications_policy_v2.rb +++ b/db/post_migrate/20240808124339_post_deployment_migrate_notifications_policy_v2.rb @@ -9,7 +9,7 @@ class PostDeploymentMigrateNotificationsPolicyV2 < ActiveRecord::Migration[7.1] def up NotificationPolicy.in_batches.update_all(<<~SQL.squish) for_not_following = CASE filter_not_following WHEN true THEN 1 ELSE 0 END, - for_not_followers = CASE filter_not_following WHEN true THEN 1 ELSE 0 END, + for_not_followers = CASE filter_not_followers WHEN true THEN 1 ELSE 0 END, for_new_accounts = CASE filter_new_accounts WHEN true THEN 1 ELSE 0 END, for_private_mentions = CASE filter_private_mentions WHEN true THEN 1 ELSE 0 END SQL From 90072f4367a3c2d6a477119b0e79a09a03eb3fd8 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 3 Jan 2025 14:23:01 +0100 Subject: [PATCH 11/14] Fix incorrect `relationship_severance_event` attribute name in changelog (#33443) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 02a12cfe5b..b940efbaf1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -135,7 +135,7 @@ The following changelog entries focus on changes visible to users, administrator - **Add notifications of severed relationships** (#27511, #29665, #29668, #29670, #29700, #29714, #29712, and #29731 by @ClearlyClaire and @Gargron)\ Notify local users when they lose relationships as a result of a local moderator blocking a remote account or server, allowing the affected user to retrieve the list of broken relationships.\ Note that this does not notify remote users.\ - This adds the `severed_relationships` notification type to the REST API and streaming, with a new [`relationship_severance_event` attribute](https://docs.joinmastodon.org/entities/Notification/#relationship_severance_event). + This adds the `severed_relationships` notification type to the REST API and streaming, with a new [`event` attribute](https://docs.joinmastodon.org/entities/Notification/#relationship_severance_event). - **Add hover cards in web UI** (#30754, #30864, #30850, #30879, #30928, #30949, #30948, #30931, and #31300 by @ClearlyClaire, @Gargron, and @renchap)\ Hovering over an avatar or username will now display a hover card with the first two lines of the user's description and their first two profile fields.\ This can be disabled in the “Animations and accessibility” section of the preferences. From 5c8d2be23b5bf4224878d1503c55b81cc972ec18 Mon Sep 17 00:00:00 2001 From: Michael Stanclift Date: Tue, 14 Jan 2025 10:35:58 -0600 Subject: [PATCH 12/14] Fix libyaml missing from Dockerfile build stage (#33591) --- Dockerfile | 1 + 1 file changed, 1 insertion(+) diff --git a/Dockerfile b/Dockerfile index c7c02d9b46..8d50458132 100644 --- a/Dockerfile +++ b/Dockerfile @@ -150,6 +150,7 @@ RUN \ libpq-dev \ libssl-dev \ libtool \ + libyaml-dev \ meson \ nasm \ pkg-config \ From 10f10844ffa4bed5e0978d151ba5fa9cd8dc87bd Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 15 Jan 2025 12:45:39 +0100 Subject: [PATCH 13/14] Update dependencies rails and rails-html-sanitizer --- Gemfile.lock | 117 +++++++++++++++++++++++++++------------------------ 1 file changed, 61 insertions(+), 56 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 0236bf3770..cd07706f93 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -10,35 +10,35 @@ GIT GEM remote: https://rubygems.org/ specs: - actioncable (7.1.4.1) - actionpack (= 7.1.4.1) - activesupport (= 7.1.4.1) + actioncable (7.1.5.1) + actionpack (= 7.1.5.1) + activesupport (= 7.1.5.1) nio4r (~> 2.0) websocket-driver (>= 0.6.1) zeitwerk (~> 2.6) - actionmailbox (7.1.4.1) - actionpack (= 7.1.4.1) - activejob (= 7.1.4.1) - activerecord (= 7.1.4.1) - activestorage (= 7.1.4.1) - activesupport (= 7.1.4.1) + actionmailbox (7.1.5.1) + actionpack (= 7.1.5.1) + activejob (= 7.1.5.1) + activerecord (= 7.1.5.1) + activestorage (= 7.1.5.1) + activesupport (= 7.1.5.1) mail (>= 2.7.1) net-imap net-pop net-smtp - actionmailer (7.1.4.1) - actionpack (= 7.1.4.1) - actionview (= 7.1.4.1) - activejob (= 7.1.4.1) - activesupport (= 7.1.4.1) + actionmailer (7.1.5.1) + actionpack (= 7.1.5.1) + actionview (= 7.1.5.1) + activejob (= 7.1.5.1) + activesupport (= 7.1.5.1) mail (~> 2.5, >= 2.5.4) net-imap net-pop net-smtp rails-dom-testing (~> 2.2) - actionpack (7.1.4.1) - actionview (= 7.1.4.1) - activesupport (= 7.1.4.1) + actionpack (7.1.5.1) + actionview (= 7.1.5.1) + activesupport (= 7.1.5.1) nokogiri (>= 1.8.5) racc rack (>= 2.2.4) @@ -46,15 +46,15 @@ GEM rack-test (>= 0.6.3) rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) - actiontext (7.1.4.1) - actionpack (= 7.1.4.1) - activerecord (= 7.1.4.1) - activestorage (= 7.1.4.1) - activesupport (= 7.1.4.1) + actiontext (7.1.5.1) + actionpack (= 7.1.5.1) + activerecord (= 7.1.5.1) + activestorage (= 7.1.5.1) + activesupport (= 7.1.5.1) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (7.1.4.1) - activesupport (= 7.1.4.1) + actionview (7.1.5.1) + activesupport (= 7.1.5.1) builder (~> 3.1) erubi (~> 1.11) rails-dom-testing (~> 2.2) @@ -64,30 +64,33 @@ GEM activemodel (>= 4.1) case_transform (>= 0.2) jsonapi-renderer (>= 0.1.1.beta1, < 0.3) - activejob (7.1.4.1) - activesupport (= 7.1.4.1) + activejob (7.1.5.1) + activesupport (= 7.1.5.1) globalid (>= 0.3.6) - activemodel (7.1.4.1) - activesupport (= 7.1.4.1) - activerecord (7.1.4.1) - activemodel (= 7.1.4.1) - activesupport (= 7.1.4.1) + activemodel (7.1.5.1) + activesupport (= 7.1.5.1) + activerecord (7.1.5.1) + activemodel (= 7.1.5.1) + activesupport (= 7.1.5.1) timeout (>= 0.4.0) - activestorage (7.1.4.1) - actionpack (= 7.1.4.1) - activejob (= 7.1.4.1) - activerecord (= 7.1.4.1) - activesupport (= 7.1.4.1) + activestorage (7.1.5.1) + actionpack (= 7.1.5.1) + activejob (= 7.1.5.1) + activerecord (= 7.1.5.1) + activesupport (= 7.1.5.1) marcel (~> 1.0) - activesupport (7.1.4.1) + activesupport (7.1.5.1) base64 + benchmark (>= 0.3) bigdecimal concurrent-ruby (~> 1.0, >= 1.0.2) connection_pool (>= 2.2.5) drb i18n (>= 1.6, < 2) + logger (>= 1.4.2) minitest (>= 5.1) mutex_m + securerandom (>= 0.3) tzinfo (~> 2.0) addressable (2.8.7) public_suffix (>= 2.0.2, < 7.0) @@ -126,6 +129,7 @@ GEM base64 (0.2.0) bcp47_spec (0.2.1) bcrypt (3.1.20) + benchmark (0.4.0) better_errors (2.10.1) erubi (>= 1.0.0) rack (>= 0.9.0) @@ -454,7 +458,7 @@ GEM net-smtp (0.5.0) net-protocol nio4r (2.7.3) - nokogiri (1.16.7) + nokogiri (1.16.8) mini_portile2 (~> 2.8.2) racc (~> 1.4) oj (3.16.6) @@ -638,20 +642,20 @@ GEM rackup (1.0.0) rack (< 3) webrick - rails (7.1.4.1) - actioncable (= 7.1.4.1) - actionmailbox (= 7.1.4.1) - actionmailer (= 7.1.4.1) - actionpack (= 7.1.4.1) - actiontext (= 7.1.4.1) - actionview (= 7.1.4.1) - activejob (= 7.1.4.1) - activemodel (= 7.1.4.1) - activerecord (= 7.1.4.1) - activestorage (= 7.1.4.1) - activesupport (= 7.1.4.1) + rails (7.1.5.1) + actioncable (= 7.1.5.1) + actionmailbox (= 7.1.5.1) + actionmailer (= 7.1.5.1) + actionpack (= 7.1.5.1) + actiontext (= 7.1.5.1) + actionview (= 7.1.5.1) + activejob (= 7.1.5.1) + activemodel (= 7.1.5.1) + activerecord (= 7.1.5.1) + activestorage (= 7.1.5.1) + activesupport (= 7.1.5.1) bundler (>= 1.15.0) - railties (= 7.1.4.1) + railties (= 7.1.5.1) rails-controller-testing (1.0.5) actionpack (>= 5.0.1.rc1) actionview (>= 5.0.1.rc1) @@ -660,15 +664,15 @@ GEM activesupport (>= 5.0.0) minitest nokogiri (>= 1.6) - rails-html-sanitizer (1.6.0) + rails-html-sanitizer (1.6.2) loofah (~> 2.21) - nokogiri (~> 1.14) + nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) rails-i18n (7.0.9) i18n (>= 0.7, < 2) railties (>= 6.0.0, < 8) - railties (7.1.4.1) - actionpack (= 7.1.4.1) - activesupport (= 7.1.4.1) + railties (7.1.5.1) + actionpack (= 7.1.5.1) + activesupport (= 7.1.5.1) irb rackup (>= 1.0.0) rake (>= 12.2) @@ -781,6 +785,7 @@ GEM scenic (1.8.0) activerecord (>= 4.0.0) railties (>= 4.0.0) + securerandom (0.4.1) selenium-webdriver (4.25.0) base64 (~> 0.2) logger (~> 1.4) From faed9bf9f14f077443374f5eb3075b9878e24214 Mon Sep 17 00:00:00 2001 From: Claire Date: Wed, 15 Jan 2025 12:43:54 +0100 Subject: [PATCH 14/14] Bump version to v4.3.3 --- CHANGELOG.md | 18 ++++++++++++++++++ docker-compose.yml | 6 +++--- lib/mastodon/version.rb | 2 +- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b940efbaf1..0b6d52c376 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,24 @@ All notable changes to this project will be documented in this file. +## [4.3.3] - 2025-01-16 + +### Security + +- Fix insufficient validation of account URIs ([GHSA-5wxh-3p65-r4g6](https://github.com/mastodon/mastodon/security/advisories/GHSA-5wxh-3p65-r4g6)) +- Update dependencies + +### Fixed + +- Fix `libyaml` missing from `Dockerfile` build stage (#33591 by @vmstan) +- Fix incorrect notification settings migration for non-followers (#33348 by @ClearlyClaire) +- Fix down clause for notification policy v2 migrations (#33340 by @jesseplusplus) +- Fix error decrementing status count when `FeaturedTags#last_status_at` is `nil` (#33320 by @ClearlyClaire) +- Fix last paginated notification group only including data on a single notification (#33271 by @ClearlyClaire) +- Fix processing of mentions for post edits with an existing corresponding silent mention (#33227 by @ClearlyClaire) +- Fix deletion of unconfirmed users with Webauthn set (#33186 by @ClearlyClaire) +- Fix empty authors preview card serialization (#33151, #33466 by @mjankowski and @ClearlyClaire) + ## [4.3.2] - 2024-12-03 ### Added diff --git a/docker-compose.yml b/docker-compose.yml index 6048129318..636d1b3a40 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -59,7 +59,7 @@ services: web: # You can uncomment the following line if you want to not use the prebuilt image, for example if you have local code changes # build: . - image: ghcr.io/mastodon/mastodon:v4.3.2 + image: ghcr.io/mastodon/mastodon:v4.3.3 restart: always env_file: .env.production command: bundle exec puma -C config/puma.rb @@ -83,7 +83,7 @@ services: # build: # dockerfile: ./streaming/Dockerfile # context: . - image: ghcr.io/mastodon/mastodon-streaming:v4.3.2 + image: ghcr.io/mastodon/mastodon-streaming:v4.3.3 restart: always env_file: .env.production command: node ./streaming/index.js @@ -101,7 +101,7 @@ services: sidekiq: build: . - image: ghcr.io/mastodon/mastodon:v4.3.2 + image: ghcr.io/mastodon/mastodon:v4.3.3 restart: always env_file: .env.production command: bundle exec sidekiq diff --git a/lib/mastodon/version.rb b/lib/mastodon/version.rb index ff7cab800e..05e0532962 100644 --- a/lib/mastodon/version.rb +++ b/lib/mastodon/version.rb @@ -13,7 +13,7 @@ module Mastodon end def patch - 2 + 3 end def default_prerelease