Remove instance variables from helper usage (#24203)

This commit is contained in:
Matt Jankowski 2023-04-23 16:35:54 -04:00 committed by GitHub
parent 84adcaa474
commit f01a643897
19 changed files with 53 additions and 45 deletions

View file

@ -1400,14 +1400,6 @@ Rails/HasManyOrHasOneDependent:
- 'app/models/user.rb' - 'app/models/user.rb'
- 'app/models/web/push_subscription.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). # This cop supports safe autocorrection (--autocorrect).
# Configuration parameters: Include. # Configuration parameters: Include.
# Include: spec/**/*, test/**/* # Include: spec/**/*, test/**/*

View file

@ -19,6 +19,7 @@ class ApplicationController < ActionController::Base
helper_method :omniauth_only? helper_method :omniauth_only?
helper_method :sso_account_settings helper_method :sso_account_settings
helper_method :whitelist_mode? helper_method :whitelist_mode?
helper_method :body_class_string
rescue_from ActionController::ParameterMissing, Paperclip::AdapterRegistry::NoHandlerError, with: :bad_request rescue_from ActionController::ParameterMissing, Paperclip::AdapterRegistry::NoHandlerError, with: :bad_request
rescue_from Mastodon::NotPermittedError, with: :forbidden rescue_from Mastodon::NotPermittedError, with: :forbidden
@ -148,6 +149,10 @@ class ApplicationController < ActionController::Base
current_user.setting_theme current_user.setting_theme
end end
def body_class_string
@body_classes || ''
end
def respond_with_error(code) def respond_with_error(code)
respond_to do |format| respond_to do |format|
format.any { render "errors/#{code}", layout: 'error', status: code, formats: [:html] } format.any { render "errors/#{code}", layout: 'error', status: code, formats: [:html] }

View file

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

View file

@ -9,13 +9,17 @@ module InstanceHelper
@site_hostname ||= Addressable::URI.parse("//#{Rails.configuration.x.local_domain}").display_uri.host @site_hostname ||= Addressable::URI.parse("//#{Rails.configuration.x.local_domain}").display_uri.host
end end
def description_for_sign_up def description_for_sign_up(invite = nil)
prefix = if @invite.present? safe_join([description_prefix(invite), I18n.t('auth.description.suffix')], ' ')
I18n.t('auth.description.prefix_invited_by_user', name: @invite.user.account.username) end
else
I18n.t('auth.description.prefix_sign_up')
end
safe_join([prefix, I18n.t('auth.description.suffix')], ' ') private
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
end end

View file

@ -63,11 +63,11 @@ module JsonLdHelper
uri.nil? || !uri.start_with?('http://', 'https://') uri.nil? || !uri.start_with?('http://', 'https://')
end end
def invalid_origin?(url) def non_matching_uri_hosts?(base_url, comparison_url)
return true if unsupported_uri_scheme?(url) return true if unsupported_uri_scheme?(comparison_url)
needle = Addressable::URI.parse(url).host needle = Addressable::URI.parse(comparison_url).host
haystack = Addressable::URI.parse(@account.uri).host haystack = Addressable::URI.parse(base_url).host
!haystack.casecmp(needle).zero? !haystack.casecmp(needle).zero?
end end

View file

@ -17,7 +17,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
private private
def create_encrypted_message 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_account = Account.find(@options[:delivered_to_account_id])
target_device = target_account.devices.find_by(device_id: @object.dig('to', 'deviceId')) target_device = target_account.devices.find_by(device_id: @object.dig('to', 'deviceId'))
@ -45,7 +45,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
end end
def create_status 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 with_lock("create:#{object_uri}") do
return if delete_arrived_first?(object_uri) || poll_vote? return if delete_arrived_first?(object_uri) || poll_vote?

View file

@ -21,7 +21,7 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity
return if object_uri.nil? return if object_uri.nil?
with_lock("delete_status_in_progress:#{object_uri}", raise_on_failure: false) do 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 # This lock ensures a concurrent `ActivityPub::Activity::Create` either
# does not create a status at all, or has finished saving it to the # does not create a status at all, or has finished saving it to the
# database before we try to load it. # database before we try to load it.

View file

@ -33,6 +33,6 @@ class ActivityPub::Activity::Flag < ActivityPub::Activity
end end
def report_uri 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
end end

View file

@ -22,7 +22,7 @@ class ActivityPub::Activity::Update < ActivityPub::Activity
end end
def update_status 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) @status = Status.find_by(uri: object_uri, account_id: @account.id)

View file

@ -40,7 +40,7 @@ class ActivityPub::Dereferencer
end end
def perform_request(uri, headers: nil) 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) req = Request.new(:get, uri)
@ -57,13 +57,4 @@ class ActivityPub::Dereferencer
end end
end 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 end

View file

@ -31,7 +31,7 @@ class ActivityPub::FetchFeaturedCollectionService < BaseService
def fetch_collection(collection_or_uri) def fetch_collection(collection_or_uri)
return collection_or_uri if collection_or_uri.is_a?(Hash) 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) fetch_resource_without_id_validation(collection_or_uri, local_follower, true)
end end
@ -46,7 +46,7 @@ class ActivityPub::FetchFeaturedCollectionService < BaseService
next unless item.is_a?(String) || item['type'] == 'Note' next unless item.is_a?(String) || item['type'] == 'Note'
uri = value_or_id(item) 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]) 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 next unless status&.account_id == @account.id

View file

@ -43,7 +43,7 @@ class ActivityPub::FetchFeaturedTagsCollectionService < BaseService
def fetch_collection(collection_or_uri) def fetch_collection(collection_or_uri)
return collection_or_uri if collection_or_uri.is_a?(Hash) 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) fetch_resource_without_id_validation(collection_or_uri, local_follower, true)
end end

View file

@ -35,7 +35,7 @@ class ActivityPub::FetchRepliesService < BaseService
def fetch_collection(collection_or_uri) def fetch_collection(collection_or_uri)
return collection_or_uri if collection_or_uri.is_a?(Hash) return collection_or_uri if collection_or_uri.is_a?(Hash)
return unless @allow_synchronous_requests 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) fetch_resource_without_id_validation(collection_or_uri, nil, true)
end end
@ -45,6 +45,6 @@ class ActivityPub::FetchRepliesService < BaseService
# amplification attacks. # amplification attacks.
# Also limit to 5 fetched replies to limit potential for DoS. # 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
end end

View file

@ -6,7 +6,7 @@ class ActivityPub::PrepareFollowersSynchronizationService < BaseService
def call(account, params) def call(account, params)
@account = account @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']) ActivityPub::FollowersSynchronizationWorker.perform_async(@account.id, params['url'])
end end

View file

@ -67,7 +67,7 @@ class ActivityPub::SynchronizeFollowersService < BaseService
def fetch_collection(collection_or_uri) def fetch_collection(collection_or_uri)
return collection_or_uri if collection_or_uri.is_a?(Hash) 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) fetch_resource_without_id_validation(collection_or_uri, nil, true)
end end

View file

@ -2,7 +2,7 @@
= t('auth.register') = t('auth.register')
- content_for :header_tags do - 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| = simple_form_for(resource, as: resource_name, url: registration_path(resource_name), html: { novalidate: false }) do |f|
= render 'auth/shared/progress', stage: 'details' = render 'auth/shared/progress', stage: 'details'

View file

@ -2,7 +2,7 @@
= t('auth.register') = t('auth.register')
- content_for :header_tags do - 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 .simple_form
= render 'auth/shared/progress', stage: 'rules' = render 'auth/shared/progress', stage: 'rules'

View file

@ -9,7 +9,7 @@ describe SharesController do
before { sign_in user } before { sign_in user }
describe 'GTE #show' do describe 'GET #show' do
subject(:body_classes) { assigns(:body_classes) } subject(:body_classes) { assigns(:body_classes) }
before { get :show, params: { title: 'test title', text: 'test text', url: 'url1 url2' } } before { get :show, params: { title: 'test title', text: 'test text', url: 'url1 url2' } }

View file

@ -27,6 +27,22 @@ describe ApplicationHelper do
end end
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 describe 'locale_direction' do
around do |example| around do |example|
current_locale = I18n.locale current_locale = I18n.locale