~cytrogen/masto-fe

0a5f0a8b200e419df9f58bd8ffb581a0c0632c0f — Matt Jankowski 2 years ago e1b4eeb
Remove instance variables from helper usage (#24203)

M .rubocop_todo.yml => .rubocop_todo.yml +0 -8
@@ 1400,14 1400,6 @@ Rails/HasManyOrHasOneDependent:
    - 'app/models/user.rb'
    - 'app/models/web/push_subscription.rb'

# Configuration parameters: Include.
# Include: app/helpers/**/*.rb
Rails/HelperInstanceVariable:
  Exclude:
    - 'app/helpers/application_helper.rb'
    - 'app/helpers/instance_helper.rb'
    - 'app/helpers/jsonld_helper.rb'

# This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: Include.
# Include: spec/**/*, test/**/*

M app/controllers/application_controller.rb => app/controllers/application_controller.rb +5 -0
@@ 19,6 19,7 @@ class ApplicationController < ActionController::Base
  helper_method :omniauth_only?
  helper_method :sso_account_settings
  helper_method :whitelist_mode?
  helper_method :body_class_string

  rescue_from ActionController::ParameterMissing, Paperclip::AdapterRegistry::NoHandlerError, with: :bad_request
  rescue_from Mastodon::NotPermittedError, with: :forbidden


@@ 148,6 149,10 @@ class ApplicationController < ActionController::Base
    current_user.setting_theme
  end

  def body_class_string
    @body_classes || ''
  end

  def respond_with_error(code)
    respond_to do |format|
      format.any  { render "errors/#{code}", layout: 'error', status: code, formats: [:html] }

M app/helpers/application_helper.rb => app/helpers/application_helper.rb +1 -1
@@ 168,7 168,7 @@ module ApplicationHelper
  end

  def body_classes
    output = (@body_classes || '').split
    output = body_class_string.split
    output << "theme-#{current_theme.parameterize}"
    output << 'system-font' if current_account&.user&.setting_system_font_ui
    output << (current_account&.user&.setting_reduce_motion ? 'reduce-motion' : 'no-reduce-motion')

M app/helpers/instance_helper.rb => app/helpers/instance_helper.rb +11 -7
@@ 9,13 9,17 @@ module InstanceHelper
    @site_hostname ||= Addressable::URI.parse("//#{Rails.configuration.x.local_domain}").display_uri.host
  end

  def description_for_sign_up
    prefix = if @invite.present?
               I18n.t('auth.description.prefix_invited_by_user', name: @invite.user.account.username)
             else
               I18n.t('auth.description.prefix_sign_up')
             end
  def description_for_sign_up(invite = nil)
    safe_join([description_prefix(invite), I18n.t('auth.description.suffix')], ' ')
  end

  private

    safe_join([prefix, I18n.t('auth.description.suffix')], ' ')
  def description_prefix(invite)
    if invite.present?
      I18n.t('auth.description.prefix_invited_by_user', name: invite.user.account.username)
    else
      I18n.t('auth.description.prefix_sign_up')
    end
  end
end

M app/helpers/jsonld_helper.rb => app/helpers/jsonld_helper.rb +4 -4
@@ 63,11 63,11 @@ module JsonLdHelper
    uri.nil? || !uri.start_with?('http://', 'https://')
  end

  def invalid_origin?(url)
    return true if unsupported_uri_scheme?(url)
  def non_matching_uri_hosts?(base_url, comparison_url)
    return true if unsupported_uri_scheme?(comparison_url)

    needle   = Addressable::URI.parse(url).host
    haystack = Addressable::URI.parse(@account.uri).host
    needle = Addressable::URI.parse(comparison_url).host
    haystack = Addressable::URI.parse(base_url).host

    !haystack.casecmp(needle).zero?
  end

M app/lib/activitypub/activity/create.rb => app/lib/activitypub/activity/create.rb +2 -2
@@ 17,7 17,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
  private

  def create_encrypted_message
    return reject_payload! if invalid_origin?(object_uri) || @options[:delivered_to_account_id].blank?
    return reject_payload! if non_matching_uri_hosts?(@account.uri, object_uri) || @options[:delivered_to_account_id].blank?

    target_account = Account.find(@options[:delivered_to_account_id])
    target_device  = target_account.devices.find_by(device_id: @object.dig('to', 'deviceId'))


@@ 45,7 45,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
  end

  def create_status
    return reject_payload! if unsupported_object_type? || invalid_origin?(object_uri) || tombstone_exists? || !related_to_local_activity?
    return reject_payload! if unsupported_object_type? || non_matching_uri_hosts?(@account.uri, object_uri) || tombstone_exists? || !related_to_local_activity?

    with_lock("create:#{object_uri}") do
      return if delete_arrived_first?(object_uri) || poll_vote?

M app/lib/activitypub/activity/delete.rb => app/lib/activitypub/activity/delete.rb +1 -1
@@ 21,7 21,7 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity
    return if object_uri.nil?

    with_lock("delete_status_in_progress:#{object_uri}", raise_on_failure: false) do
      unless invalid_origin?(object_uri)
      unless non_matching_uri_hosts?(@account.uri, object_uri)
        # This lock ensures a concurrent `ActivityPub::Activity::Create` either
        # does not create a status at all, or has finished saving it to the
        # database before we try to load it.

M app/lib/activitypub/activity/flag.rb => app/lib/activitypub/activity/flag.rb +1 -1
@@ 33,6 33,6 @@ class ActivityPub::Activity::Flag < ActivityPub::Activity
  end

  def report_uri
    @json['id'] unless @json['id'].nil? || invalid_origin?(@json['id'])
    @json['id'] unless @json['id'].nil? || non_matching_uri_hosts?(@account.uri, @json['id'])
  end
end

M app/lib/activitypub/activity/update.rb => app/lib/activitypub/activity/update.rb +1 -1
@@ 22,7 22,7 @@ class ActivityPub::Activity::Update < ActivityPub::Activity
  end

  def update_status
    return reject_payload! if invalid_origin?(object_uri)
    return reject_payload! if non_matching_uri_hosts?(@account.uri, object_uri)

    @status = Status.find_by(uri: object_uri, account_id: @account.id)


M app/lib/activitypub/dereferencer.rb => app/lib/activitypub/dereferencer.rb +1 -10
@@ 40,7 40,7 @@ class ActivityPub::Dereferencer
  end

  def perform_request(uri, headers: nil)
    return if invalid_origin?(uri)
    return if non_matching_uri_hosts?(@permitted_origin, uri)

    req = Request.new(:get, uri)



@@ 57,13 57,4 @@ class ActivityPub::Dereferencer
      end
    end
  end

  def invalid_origin?(uri)
    return true if unsupported_uri_scheme?(uri)

    needle   = Addressable::URI.parse(uri).host
    haystack = Addressable::URI.parse(@permitted_origin).host

    !haystack.casecmp(needle).zero?
  end
end

M app/services/activitypub/fetch_featured_collection_service.rb => app/services/activitypub/fetch_featured_collection_service.rb +2 -2
@@ 31,7 31,7 @@ class ActivityPub::FetchFeaturedCollectionService < BaseService

  def fetch_collection(collection_or_uri)
    return collection_or_uri if collection_or_uri.is_a?(Hash)
    return if invalid_origin?(collection_or_uri)
    return if non_matching_uri_hosts?(@account.uri, collection_or_uri)

    fetch_resource_without_id_validation(collection_or_uri, local_follower, true)
  end


@@ 46,7 46,7 @@ class ActivityPub::FetchFeaturedCollectionService < BaseService
      next unless item.is_a?(String) || item['type'] == 'Note'

      uri = value_or_id(item)
      next if ActivityPub::TagManager.instance.local_uri?(uri) || invalid_origin?(uri)
      next if ActivityPub::TagManager.instance.local_uri?(uri) || non_matching_uri_hosts?(@account.uri, uri)

      status = ActivityPub::FetchRemoteStatusService.new.call(uri, on_behalf_of: local_follower, expected_actor_uri: @account.uri, request_id: @options[:request_id])
      next unless status&.account_id == @account.id

M app/services/activitypub/fetch_featured_tags_collection_service.rb => app/services/activitypub/fetch_featured_tags_collection_service.rb +1 -1
@@ 43,7 43,7 @@ class ActivityPub::FetchFeaturedTagsCollectionService < BaseService

  def fetch_collection(collection_or_uri)
    return collection_or_uri if collection_or_uri.is_a?(Hash)
    return if invalid_origin?(collection_or_uri)
    return if non_matching_uri_hosts?(@account.uri, collection_or_uri)

    fetch_resource_without_id_validation(collection_or_uri, local_follower, true)
  end

M app/services/activitypub/fetch_replies_service.rb => app/services/activitypub/fetch_replies_service.rb +2 -2
@@ 35,7 35,7 @@ class ActivityPub::FetchRepliesService < BaseService
  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)
    return if non_matching_uri_hosts?(@account.uri, collection_or_uri)

    fetch_resource_without_id_validation(collection_or_uri, nil, true)
  end


@@ 45,6 45,6 @@ class ActivityPub::FetchRepliesService < BaseService
    # 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)
    @items.map { |item| value_or_id(item) }.reject { |uri| non_matching_uri_hosts?(@account.uri, uri) }.take(5)
  end
end

M app/services/activitypub/prepare_followers_synchronization_service.rb => app/services/activitypub/prepare_followers_synchronization_service.rb +1 -1
@@ 6,7 6,7 @@ class ActivityPub::PrepareFollowersSynchronizationService < BaseService
  def call(account, params)
    @account = account

    return if params['collectionId'] != @account.followers_url || invalid_origin?(params['url']) || @account.local_followers_hash == params['digest']
    return if params['collectionId'] != @account.followers_url || non_matching_uri_hosts?(@account.uri, params['url']) || @account.local_followers_hash == params['digest']

    ActivityPub::FollowersSynchronizationWorker.perform_async(@account.id, params['url'])
  end

M app/services/activitypub/synchronize_followers_service.rb => app/services/activitypub/synchronize_followers_service.rb +1 -1
@@ 67,7 67,7 @@ class ActivityPub::SynchronizeFollowersService < BaseService

  def fetch_collection(collection_or_uri)
    return collection_or_uri if collection_or_uri.is_a?(Hash)
    return if invalid_origin?(collection_or_uri)
    return if non_matching_uri_hosts?(@account.uri, collection_or_uri)

    fetch_resource_without_id_validation(collection_or_uri, nil, true)
  end

M app/views/auth/registrations/new.html.haml => app/views/auth/registrations/new.html.haml +1 -1
@@ 2,7 2,7 @@
  = t('auth.register')

- content_for :header_tags do
  = render partial: 'shared/og', locals: { description: description_for_sign_up }
  = render partial: 'shared/og', locals: { description: description_for_sign_up(@invite) }

= simple_form_for(resource, as: resource_name, url: registration_path(resource_name), html: { novalidate: false }) do |f|
  = render 'auth/shared/progress', stage: 'details'

M app/views/auth/registrations/rules.html.haml => app/views/auth/registrations/rules.html.haml +1 -1
@@ 2,7 2,7 @@
  = t('auth.register')

- content_for :header_tags do
  = render partial: 'shared/og', locals: { description: description_for_sign_up }
  = render partial: 'shared/og', locals: { description: description_for_sign_up(@invite) }

.simple_form
  = render 'auth/shared/progress', stage: 'rules'

M spec/controllers/shares_controller_spec.rb => spec/controllers/shares_controller_spec.rb +1 -1
@@ 9,7 9,7 @@ describe SharesController do

  before { sign_in user }

  describe 'GTE #show' do
  describe 'GET #show' do
    subject(:body_classes) { assigns(:body_classes) }

    before { get :show, params: { title: 'test title', text: 'test text', url: 'url1 url2' } }

M spec/helpers/application_helper_spec.rb => spec/helpers/application_helper_spec.rb +16 -0
@@ 27,6 27,22 @@ describe ApplicationHelper do
    end
  end

  describe 'body_classes' do
    context 'with a body class string from a controller' do
      before do
        without_partial_double_verification do
          allow(helper).to receive(:body_class_string).and_return('modal-layout compose-standalone')
          allow(helper).to receive(:current_theme).and_return('default')
          allow(helper).to receive(:current_account).and_return(Fabricate(:account))
        end
      end

      it 'uses the controller body classes in the result' do
        expect(helper.body_classes).to match(/modal-layout compose-standalone/)
      end
    end
  end

  describe 'locale_direction' do
    around do |example|
      current_locale = I18n.locale