Merge pull request #932 from ThibG/glitch-soc/merge-upstream

Merge upstream changes
master
ThibG 6 years ago committed by GitHub
commit b969b150e8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 2
      Gemfile
  2. 6
      Gemfile.lock
  3. 53
      app/controllers/statuses_controller.rb
  4. 7
      app/javascript/mastodon/actions/conversations.js
  5. 6
      app/javascript/mastodon/reducers/conversations.js
  6. 1
      app/javascript/styles/mastodon/components.scss
  7. 12
      app/lib/activitypub/activity/create.rb
  8. 6
      app/lib/activitypub/tag_manager.rb
  9. 4
      app/models/concerns/status_threading_concern.rb
  10. 2
      app/presenters/activitypub/collection_presenter.rb
  11. 10
      app/serializers/activitypub/activity_serializer.rb
  12. 5
      app/serializers/activitypub/collection_serializer.rb
  13. 17
      app/serializers/activitypub/note_serializer.rb
  14. 60
      app/services/activitypub/fetch_replies_service.rb
  15. 12
      app/workers/activitypub/fetch_replies_worker.rb
  16. 11
      app/workers/concerns/exponential_backoff.rb
  17. 12
      app/workers/fetch_reply_worker.rb
  18. 5
      app/workers/thread_resolve_worker.rb
  19. 1
      config/routes.rb
  20. 44
      spec/serializers/activitypub/note_spec.rb
  21. 122
      spec/services/activitypub/fetch_replies_service_spec.rb
  22. 40
      spec/workers/activitypub/fetch_replies_worker_spec.rb

@ -108,7 +108,7 @@ group :production, :test do
end
group :test do
gem 'capybara', '~> 3.13'
gem 'capybara', '~> 3.14'
gem 'climate_control', '~> 0.2'
gem 'faker', '~> 1.9'
gem 'microformats', '~> 4.1'

@ -126,7 +126,7 @@ GEM
sshkit (~> 1.3)
capistrano-yarn (2.0.2)
capistrano (~> 3.0)
capybara (3.13.2)
capybara (3.14.0)
addressable
mini_mime (>= 0.1.3)
nokogiri (~> 1.8)
@ -243,7 +243,7 @@ GEM
temple (>= 0.8.0)
thor
tilt
hamlit-rails (0.2.1)
hamlit-rails (0.2.2)
actionpack (>= 4.0.1)
activesupport (>= 4.0.1)
hamlit (>= 1.2.0)
@ -673,7 +673,7 @@ DEPENDENCIES
capistrano-rails (~> 1.4)
capistrano-rbenv (~> 2.1)
capistrano-yarn (~> 2.0)
capybara (~> 3.13)
capybara (~> 3.14)
charlock_holmes (~> 0.7.6)
chewy (~> 5.0)
cld3 (~> 3.2.3)

@ -18,6 +18,7 @@ class StatusesController < ApplicationController
before_action :redirect_to_original, only: [:show]
before_action :set_referrer_policy_header, only: [:show]
before_action :set_cache_headers
before_action :set_replies, only: [:replies]
content_security_policy only: :embed do |p|
p.frame_ancestors(false)
@ -65,8 +66,37 @@ class StatusesController < ApplicationController
render 'stream_entries/embed', layout: 'embedded'
end
def replies
skip_session!
render json: replies_collection_presenter,
serializer: ActivityPub::CollectionSerializer,
adapter: ActivityPub::Adapter,
content_type: 'application/activity+json',
skip_activities: true
end
private
def replies_collection_presenter
page = ActivityPub::CollectionPresenter.new(
id: replies_account_status_url(@account, @status, page_params),
type: :unordered,
part_of: replies_account_status_url(@account, @status),
next: next_page,
items: @replies.map { |status| status.local ? status : status.id }
)
if page_requested?
page
else
ActivityPub::CollectionPresenter.new(
id: replies_account_status_url(@account, @status),
type: :unordered,
first: page
)
end
end
def create_descendant_thread(starting_depth, statuses)
depth = starting_depth + statuses.size
if depth < DESCENDANTS_DEPTH_LIMIT
@ -176,4 +206,27 @@ class StatusesController < ApplicationController
return if @status.public_visibility? || @status.unlisted_visibility?
response.headers['Referrer-Policy'] = 'origin'
end
def page_requested?
params[:page] == 'true'
end
def set_replies
@replies = page_params[:other_accounts] ? Status.where.not(account_id: @account.id) : @account.statuses
@replies = @replies.where(in_reply_to_id: @status.id, visibility: [:public, :unlisted])
@replies = @replies.paginate_by_min_id(DESCENDANTS_LIMIT, params[:min_id])
end
def next_page
last_reply = @replies.last
return if last_reply.nil?
same_account = last_reply.account_id == @account.id
return unless same_account || @replies.size == DESCENDANTS_LIMIT
same_account = false unless @replies.size == DESCENDANTS_LIMIT
replies_account_status_url(@account, @status, page: true, min_id: last_reply.id, other_accounts: !same_account)
end
def page_params
{ page: true, other_accounts: params[:other_accounts], min_id: params[:min_id] }.compact
end
end

@ -41,13 +41,15 @@ export const expandConversations = ({ maxId } = {}) => (dispatch, getState) => {
params.since_id = getState().getIn(['conversations', 'items', 0, 'last_status']);
}
const isLoadingRecent = !!params.since_id;
api(getState).get('/api/v1/conversations', { params })
.then(response => {
const next = getLinks(response).refs.find(link => link.rel === 'next');
dispatch(importFetchedAccounts(response.data.reduce((aggr, item) => aggr.concat(item.accounts), [])));
dispatch(importFetchedStatuses(response.data.map(item => item.last_status).filter(x => !!x)));
dispatch(expandConversationsSuccess(response.data, next ? next.uri : null));
dispatch(expandConversationsSuccess(response.data, next ? next.uri : null, isLoadingRecent));
})
.catch(err => dispatch(expandConversationsFail(err)));
};
@ -56,10 +58,11 @@ export const expandConversationsRequest = () => ({
type: CONVERSATIONS_FETCH_REQUEST,
});
export const expandConversationsSuccess = (conversations, next) => ({
export const expandConversationsSuccess = (conversations, next, isLoadingRecent) => ({
type: CONVERSATIONS_FETCH_SUCCESS,
conversations,
next,
isLoadingRecent,
});
export const expandConversationsFail = error => ({

@ -35,7 +35,7 @@ const updateConversation = (state, item) => state.update('items', list => {
}
});
const expandNormalizedConversations = (state, conversations, next) => {
const expandNormalizedConversations = (state, conversations, next, isLoadingRecent) => {
let items = ImmutableList(conversations.map(conversationToMap));
return state.withMutations(mutable => {
@ -66,7 +66,7 @@ const expandNormalizedConversations = (state, conversations, next) => {
});
}
if (!next) {
if (!next && !isLoadingRecent) {
mutable.set('hasMore', false);
}
@ -81,7 +81,7 @@ export default function conversations(state = initialState, action) {
case CONVERSATIONS_FETCH_FAIL:
return state.set('isLoading', false);
case CONVERSATIONS_FETCH_SUCCESS:
return expandNormalizedConversations(state, action.conversations, action.next);
return expandNormalizedConversations(state, action.conversations, action.next, action.isLoadingRecent);
case CONVERSATIONS_UPDATE:
return updateConversation(state, action.conversation);
case CONVERSATIONS_MOUNT:

@ -2336,6 +2336,7 @@ a.account__display-name {
.getting-started {
color: $dark-text-color;
overflow: auto;
&__footer {
flex: 0 0 auto;

@ -40,6 +40,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
end
resolve_thread(@status)
fetch_replies(@status)
distribute(@status)
forward_for_reply if @status.public_visibility? || @status.unlisted_visibility?
end
@ -159,7 +160,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
return if tag['href'].blank?
account = account_from_uri(tag['href'])
account = ::FetchRemoteAccountService.new.call(tag['href'], id: false) if account.nil?
account = ::FetchRemoteAccountService.new.call(tag['href']) if account.nil?
return if account.nil?
@ -213,6 +214,15 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
ThreadResolveWorker.perform_async(status.id, in_reply_to_uri)
end
def fetch_replies(status)
collection = @object['replies']
return if collection.nil?
replies = ActivityPub::FetchRepliesService.new.call(status, collection, false)
return if replies.present?
uri = value_or_id(collection)
ActivityPub::FetchRepliesWorker.perform_async(status.id, uri) unless uri.nil?
end
def conversation_from_uri(uri)
return nil if uri.nil?
return Conversation.find_by(id: OStatus::TagManager.instance.unique_tag_to_local_id(uri, 'Conversation')) if OStatus::TagManager.instance.local_id?(uri)

@ -48,6 +48,12 @@ class ActivityPub::TagManager
activity_account_status_url(target.account, target)
end
def replies_uri_for(target, page_params = nil)
raise ArgumentError, 'target must be a local activity' unless %i(note comment activity).include?(target.object_type) && target.local?
replies_account_status_url(target.account, target, page_params)
end
# Primary audience of a status
# Public statuses go out to primarily the public collection
# Unlisted and private statuses go out primarily to the followers collection

@ -11,6 +11,10 @@ module StatusThreadingConcern
find_statuses_from_tree_path(descendant_ids(limit, max_child_id, since_child_id, depth), account, promote: true)
end
def self_replies(limit)
account.statuses.where(in_reply_to_id: id, visibility: [:public, :unlisted]).reorder(id: :asc).limit(limit)
end
private
def ancestor_ids(limit)

@ -1,5 +1,5 @@
# frozen_string_literal: true
class ActivityPub::CollectionPresenter < ActiveModelSerializers::Model
attributes :id, :type, :size, :items, :part_of, :first, :last, :next, :prev
attributes :id, :type, :size, :items, :page, :part_of, :first, :last, :next, :prev
end

@ -3,8 +3,8 @@
class ActivityPub::ActivitySerializer < ActiveModel::Serializer
attributes :id, :type, :actor, :published, :to, :cc
has_one :proper, key: :object, serializer: ActivityPub::NoteSerializer, unless: :owned_announce?
attribute :proper_uri, key: :object, if: :owned_announce?
has_one :proper, key: :object, serializer: ActivityPub::NoteSerializer, if: :serialize_object?
attribute :proper_uri, key: :object, unless: :serialize_object?
attribute :atom_uri, if: :announce?
def id
@ -43,7 +43,9 @@ class ActivityPub::ActivitySerializer < ActiveModel::Serializer
object.reblog?
end
def owned_announce?
announce? && object.account == object.proper.account && object.proper.private_visibility?
def serialize_object?
return true unless announce?
# Serialize private self-boosts of local toots
object.account == object.proper.account && object.proper.private_visibility? && object.local?
end
end

@ -7,7 +7,8 @@ class ActivityPub::CollectionSerializer < ActiveModel::Serializer
super
end
attributes :id, :type
attribute :id, if: -> { object.id.present? }
attribute :type
attribute :total_items, if: -> { object.size.present? }
attribute :next, if: -> { object.next.present? }
attribute :prev, if: -> { object.prev.present? }
@ -37,6 +38,6 @@ class ActivityPub::CollectionSerializer < ActiveModel::Serializer
end
def page?
object.part_of.present?
object.part_of.present? || object.page.present?
end
end

@ -13,6 +13,8 @@ class ActivityPub::NoteSerializer < ActiveModel::Serializer
has_many :media_attachments, key: :attachment
has_many :virtual_tags, key: :tag
has_one :replies, serializer: ActivityPub::CollectionSerializer, if: :local?
def id
ActivityPub::TagManager.instance.uri_for(object)
end
@ -33,6 +35,21 @@ class ActivityPub::NoteSerializer < ActiveModel::Serializer
{ object.language => Formatter.instance.format(object) }
end
def replies
replies = object.self_replies(5).pluck(:id, :uri)
last_id = replies.last&.first
ActivityPub::CollectionPresenter.new(
type: :unordered,
id: ActivityPub::TagManager.instance.replies_uri_for(object),
first: ActivityPub::CollectionPresenter.new(
type: :unordered,
part_of: ActivityPub::TagManager.instance.replies_uri_for(object),
items: replies.map(&:second),
next: last_id ? ActivityPub::TagManager.instance.replies_uri_for(object, page: true, min_id: last_id) : nil
)
)
end
def language?
object.language.present?
end

@ -0,0 +1,60 @@
# frozen_string_literal: true
class ActivityPub::FetchRepliesService < BaseService
include JsonLdHelper
def call(parent_status, collection_or_uri, allow_synchronous_requests = true)
@account = parent_status.account
@allow_synchronous_requests = allow_synchronous_requests
@items = collection_items(collection_or_uri)
return if @items.nil?
FetchReplyWorker.push_bulk(filtered_replies)
@items
end
private
def collection_items(collection_or_uri)
collection = fetch_collection(collection_or_uri)
return unless collection.is_a?(Hash)
collection = fetch_collection(collection['first']) if collection['first'].present?
return unless collection.is_a?(Hash)
case collection['type']
when 'Collection', 'CollectionPage'
collection['items']
when 'OrderedCollection', 'OrderedCollectionPage'
collection['orderedItems']
end
end
def fetch_collection(collection_or_uri)
return collection_or_uri if collection_or_uri.is_a?(Hash)
return unless @allow_synchronous_requests
return if invalid_origin?(collection_or_uri)
collection = fetch_resource_without_id_validation(collection_or_uri)
raise Mastodon::UnexpectedResponseError if collection.nil?
collection
end
def filtered_replies
# Only fetch replies to the same server as the original status to avoid
# amplification attacks.
# Also limit to 5 fetched replies to limit potential for DoS.
@items.map { |item| value_or_id(item) }.reject { |uri| invalid_origin?(uri) }.take(5)
end
def invalid_origin?(url)
return true if unsupported_uri_scheme?(url)
needle = Addressable::URI.parse(url).host
haystack = Addressable::URI.parse(@account.uri).host
!haystack.casecmp(needle).zero?
end
end

@ -0,0 +1,12 @@
# frozen_string_literal: true
class ActivityPub::FetchRepliesWorker
include Sidekiq::Worker
include ExponentialBackoff
sidekiq_options queue: 'pull', retry: 3
def perform(parent_status_id, replies_uri)
ActivityPub::FetchRepliesService.new.call(Status.find(parent_status_id), replies_uri)
end
end

@ -0,0 +1,11 @@
# frozen_string_literal: true
module ExponentialBackoff
extend ActiveSupport::Concern
included do
sidekiq_retry_in do |count|
15 + 10 * (count**4) + rand(10 * (count**4))
end
end
end

@ -0,0 +1,12 @@
# frozen_string_literal: true
class FetchReplyWorker
include Sidekiq::Worker
include ExponentialBackoff
sidekiq_options queue: 'pull', retry: 3
def perform(child_url)
FetchRemoteStatusService.new.call(child_url)
end
end

@ -2,13 +2,10 @@
class ThreadResolveWorker
include Sidekiq::Worker
include ExponentialBackoff
sidekiq_options queue: 'pull', retry: 3
sidekiq_retry_in do |count|
15 + 10 * (count**4) + rand(10 * (count**4))
end
def perform(child_status_id, parent_url)
child_status = Status.find(child_status_id)
parent_status = FetchRemoteStatusService.new.call(parent_url)

@ -56,6 +56,7 @@ Rails.application.routes.draw do
member do
get :activity
get :embed
get :replies
end
end

@ -0,0 +1,44 @@
# frozen_string_literal: true
require 'rails_helper'
describe ActivityPub::NoteSerializer do
let!(:account) { Fabricate(:account) }
let!(:other) { Fabricate(:account) }
let!(:parent) { Fabricate(:status, account: account, visibility: :public) }
let!(:reply1) { Fabricate(:status, account: account, thread: parent, visibility: :public) }
let!(:reply2) { Fabricate(:status, account: account, thread: parent, visibility: :public) }
let!(:reply3) { Fabricate(:status, account: other, thread: parent, visibility: :public) }
let!(:reply4) { Fabricate(:status, account: account, thread: parent, visibility: :public) }
let!(:reply5) { Fabricate(:status, account: account, thread: parent, visibility: :direct) }
before(:each) do
@serialization = ActiveModelSerializers::SerializableResource.new(parent, serializer: ActivityPub::NoteSerializer, adapter: ActivityPub::Adapter)
end
subject { JSON.parse(@serialization.to_json) }
it 'has a Note type' do
expect(subject['type']).to eql('Note')
end
it 'has a replies collection' do
expect(subject['replies']['type']).to eql('Collection')
end
it 'has a replies collection with a first Page' do
expect(subject['replies']['first']['type']).to eql('CollectionPage')
end
it 'includes public self-replies in its replies collection' do
expect(subject['replies']['first']['items']).to include(reply1.uri, reply2.uri, reply4.uri)
end
it 'does not include replies from others in its replies collection' do
expect(subject['replies']['first']['items']).to_not include(reply3.uri)
end
it 'does not include replies with direct visibility in its replies collection' do
expect(subject['replies']['first']['items']).to_not include(reply5.uri)
end
end

@ -0,0 +1,122 @@
require 'rails_helper'
RSpec.describe ActivityPub::FetchRepliesService, type: :service do
let(:actor) { Fabricate(:account, domain: 'example.com', uri: 'http://example.com/account') }
let(:status) { Fabricate(:status, account: actor) }
let(:collection_uri) { 'http://example.com/replies/1' }
let(:items) do
[
'http://example.com/self-reply-1',
'http://example.com/self-reply-2',
'http://example.com/self-reply-3',
'http://other.com/other-reply-1',
'http://other.com/other-reply-2',
'http://other.com/other-reply-3',
'http://example.com/self-reply-4',
'http://example.com/self-reply-5',
'http://example.com/self-reply-6',
]
end
let(:payload) do
{
'@context': 'https://www.w3.org/ns/activitystreams',
type: 'Collection',
id: collection_uri,
items: items,
}.with_indifferent_access
end
subject { described_class.new }
describe '#call' do
context 'when the payload is a Collection with inlined replies' do
context 'when passing the collection itself' do
it 'spawns workers for up to 5 replies on the same server' do
allow(FetchReplyWorker).to receive(:push_bulk)
subject.call(status, payload)
expect(FetchReplyWorker).to have_received(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
end
end
context 'when passing the URL to the collection' do
before do
stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload))
end
it 'spawns workers for up to 5 replies on the same server' do
allow(FetchReplyWorker).to receive(:push_bulk)
subject.call(status, collection_uri)
expect(FetchReplyWorker).to have_received(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
end
end
end
context 'when the payload is an OrderedCollection with inlined replies' do
let(:payload) do
{
'@context': 'https://www.w3.org/ns/activitystreams',
type: 'OrderedCollection',
id: collection_uri,
orderedItems: items,
}.with_indifferent_access
end
context 'when passing the collection itself' do
it 'spawns workers for up to 5 replies on the same server' do
allow(FetchReplyWorker).to receive(:push_bulk)
subject.call(status, payload)
expect(FetchReplyWorker).to have_received(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
end
end
context 'when passing the URL to the collection' do
before do
stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload))
end
it 'spawns workers for up to 5 replies on the same server' do
allow(FetchReplyWorker).to receive(:push_bulk)
subject.call(status, collection_uri)
expect(FetchReplyWorker).to have_received(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
end
end
end
context 'when the payload is a paginated Collection with inlined replies' do
let(:payload) do
{
'@context': 'https://www.w3.org/ns/activitystreams',
type: 'Collection',
id: collection_uri,
first: {
type: 'CollectionPage',
partOf: collection_uri,
items: items,
}
}.with_indifferent_access
end
context 'when passing the collection itself' do
it 'spawns workers for up to 5 replies on the same server' do
allow(FetchReplyWorker).to receive(:push_bulk)
subject.call(status, payload)
expect(FetchReplyWorker).to have_received(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
end
end
context 'when passing the URL to the collection' do
before do
stub_request(:get, collection_uri).to_return(status: 200, body: Oj.dump(payload))
end
it 'spawns workers for up to 5 replies on the same server' do
allow(FetchReplyWorker).to receive(:push_bulk)
subject.call(status, collection_uri)
expect(FetchReplyWorker).to have_received(:push_bulk).with(['http://example.com/self-reply-1', 'http://example.com/self-reply-2', 'http://example.com/self-reply-3', 'http://example.com/self-reply-4', 'http://example.com/self-reply-5'])
end
end
end
end
end

@ -0,0 +1,40 @@
# frozen_string_literal: true
require 'rails_helper'
describe ActivityPub::FetchRepliesWorker do
subject { described_class.new }
let(:account) { Fabricate(:account, uri: 'https://example.com/user/1') }
let(:status) { Fabricate(:status, account: account) }
let(:payload) do
{
'@context': 'https://www.w3.org/ns/activitystreams',
id: 'https://example.com/statuses_replies/1',
type: 'Collection',
items: [],
}
end
let(:json) { Oj.dump(payload) }
describe 'perform' do
it 'performs a request if the collection URI is from the same host' do
stub_request(:get, 'https://example.com/statuses_replies/1').to_return(status: 200, body: json)
subject.perform(status.id, 'https://example.com/statuses_replies/1')
expect(a_request(:get, 'https://example.com/statuses_replies/1')).to have_been_made.once
end
it 'does not perform a request if the collection URI is from a different host' do
stub_request(:get, 'https://other.com/statuses_replies/1').to_return(status: 200)
subject.perform(status.id, 'https://other.com/statuses_replies/1')
expect(a_request(:get, 'https://other.com/statuses_replies/1')).to_not have_been_made
end
it 'raises when request fails' do
stub_request(:get, 'https://example.com/statuses_replies/1').to_return(status: 500)
expect { subject.perform(status.id, 'https://example.com/statuses_replies/1') }.to raise_error Mastodon::UnexpectedResponseError
end
end
end
Loading…
Cancel
Save