MetuFSS
/
metu.life
2
1
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Add type, limit, offset, min_id, max_id, account_id to search API (#10091)

Browse Source 
* Add type, limit, offset, min_id, max_id, account_id to search API

Fix #8939

* Make the offset work on accounts and hashtags search as well

* Assure brakeman we are not doing mass assignment here

* Do not allow paginating unless a type is chosen

* Fix search query and index id field on statuses instead of created_at
master
Eugen Rochko 6 years ago committed by GitHub
parent ea58e31822
commit e7f20cc43f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 161 additions and 230 deletions
Show all changes Ignore whitespace when comparing lines Ignore changes in amount of whitespace Ignore changes in whitespace at EOL
Show Stats Download Patch File Download Diff File
  1. 2
      app/chewy/statuses_index.rb
  2. 5
      app/controllers/api/v1/accounts/search_controller.rb
  3. 26
      app/controllers/api/v1/search_controller.rb
  4. 2
      app/controllers/api/v2/search_controller.rb
  5. 16
      app/models/account.rb
  6. 8
      app/models/tag.rb
  7. 11
      app/services/account_search_service.rb
  8. 84
      app/services/search_service.rb
  9. 181
      config/brakeman.ignore
  10. 36
      spec/services/account_search_service_spec.rb
  11. 20
      spec/services/search_service_spec.rb

2
app/chewy/statuses_index.rb
Unescape View File

@ -48,6 +48,7 @@ class StatusesIndex < Chewy::Index
end end
root date_detection: false do root date_detection: false do
field :id, type: 'long'
field :account_id, type: 'long' field :account_id, type: 'long'
field :text, type: 'text', value: ->(status) { [status.spoiler_text, Formatter.instance.plaintext(status)].concat(status.media_attachments.map(&:description)).join("\n\n") } do field :text, type: 'text', value: ->(status) { [status.spoiler_text, Formatter.instance.plaintext(status)].concat(status.media_attachments.map(&:description)).join("\n\n") } do
@ -55,7 +56,6 @@ class StatusesIndex < Chewy::Index
end end
field :searchable_by, type: 'long', value: ->(status, crutches) { status.searchable_by(crutches) } field :searchable_by, type: 'long', value: ->(status, crutches) { status.searchable_by(crutches) }
field :created_at, type: 'date'
end end
end end
end end

5
app/controllers/api/v1/accounts/search_controller.rb
Unescape View File

@ -16,10 +16,11 @@ class Api::V1::Accounts::SearchController < Api::BaseController
def account_search def account_search
AccountSearchService.new.call( AccountSearchService.new.call(
params[:q], params[:q],
limit_param(DEFAULT_ACCOUNTS_LIMIT),
current_account, current_account,
limit: limit_param(DEFAULT_ACCOUNTS_LIMIT),
resolve: truthy_param?(:resolve), resolve: truthy_param?(:resolve),
following: truthy_param?(:following) following: truthy_param?(:following),
offset: params[:offset]
) )
end end
end end

26
app/controllers/api/v1/search_controller.rb
Unescape View File

@ -3,7 +3,7 @@
class Api::V1::SearchController < Api::BaseController class Api::V1::SearchController < Api::BaseController
include Authorization include Authorization
RESULTS_LIMIT = 5 RESULTS_LIMIT = 20
before_action -> { doorkeeper_authorize! :read, :'read:search' } before_action -> { doorkeeper_authorize! :read, :'read:search' }
before_action :require_user! before_action :require_user!
@ -11,30 +11,22 @@ class Api::V1::SearchController < Api::BaseController
respond_to :json respond_to :json
def index def index
@search = Search.new(search) @search = Search.new(search_results)
render json: @search, serializer: REST::SearchSerializer render json: @search, serializer: REST::SearchSerializer
end end
private private
def search
search_results.tap do |search|
search[:statuses].keep_if do |status|
begin
authorize status, :show?
rescue Mastodon::NotPermittedError
false
end
end
end
end
def search_results def search_results
SearchService.new.call( SearchService.new.call(
params[:q], params[:q],
RESULTS_LIMIT, current_account,
truthy_param?(:resolve), limit_param(RESULTS_LIMIT),
current_account search_params.merge(resolve: truthy_param?(:resolve))
) )
end end
def search_params
params.permit(:type, :offset, :min_id, :max_id, :account_id)
end
end end

2
app/controllers/api/v2/search_controller.rb
Unescape View File

@ -2,7 +2,7 @@
class Api::V2::SearchController < Api::V1::SearchController class Api::V2::SearchController < Api::V1::SearchController
def index def index
@search = Search.new(search) @search = Search.new(search_results)
render json: @search, serializer: REST::V2::SearchSerializer render json: @search, serializer: REST::V2::SearchSerializer
end end
end end

16
app/models/account.rb
Unescape View File

@ -386,7 +386,7 @@ class Account < ApplicationRecord
DeliveryFailureTracker.filter(urls) DeliveryFailureTracker.filter(urls)
end end
def search_for(terms, limit = 10) def search_for(terms, limit = 10, offset = 0)
textsearch, query = generate_query_for_search(terms) textsearch, query = generate_query_for_search(terms)
sql = <<-SQL.squish sql = <<-SQL.squish
@ -398,15 +398,15 @@ class Account < ApplicationRecord
AND accounts.suspended = false AND accounts.suspended = false
AND accounts.moved_to_account_id IS NULL AND accounts.moved_to_account_id IS NULL
ORDER BY rank DESC ORDER BY rank DESC
LIMIT ? LIMIT ? OFFSET ?
SQL SQL
records = find_by_sql([sql, limit]) records = find_by_sql([sql, limit, offset])
ActiveRecord::Associations::Preloader.new.preload(records, :account_stat) ActiveRecord::Associations::Preloader.new.preload(records, :account_stat)
records records
end end
def advanced_search_for(terms, account, limit = 10, following = false) def advanced_search_for(terms, account, limit = 10, following = false, offset = 0)
textsearch, query = generate_query_for_search(terms) textsearch, query = generate_query_for_search(terms)
if following if following
@ -427,10 +427,10 @@ class Account < ApplicationRecord
AND accounts.moved_to_account_id IS NULL AND accounts.moved_to_account_id IS NULL
GROUP BY accounts.id GROUP BY accounts.id
ORDER BY rank DESC ORDER BY rank DESC
LIMIT ? LIMIT ? OFFSET ?
SQL SQL
records = find_by_sql([sql, account.id, account.id, account.id, limit]) records = find_by_sql([sql, account.id, account.id, account.id, limit, offset])
else else
sql = <<-SQL.squish sql = <<-SQL.squish
SELECT SELECT
@ -443,10 +443,10 @@ class Account < ApplicationRecord
AND accounts.moved_to_account_id IS NULL AND accounts.moved_to_account_id IS NULL
GROUP BY accounts.id GROUP BY accounts.id
ORDER BY rank DESC ORDER BY rank DESC
LIMIT ? LIMIT ? OFFSET ?
SQL SQL
records = find_by_sql([sql, account.id, account.id, limit]) records = find_by_sql([sql, account.id, account.id, limit, offset])
end end
ActiveRecord::Associations::Preloader.new.preload(records, :account_stat) ActiveRecord::Associations::Preloader.new.preload(records, :account_stat)

8
app/models/tag.rb
Unescape View File

@ -64,9 +64,13 @@ class Tag < ApplicationRecord
end end
class << self class << self
def search_for(term, limit = 5) def search_for(term, limit = 5, offset = 0)
pattern = sanitize_sql_like(term.strip) + '%' pattern = sanitize_sql_like(term.strip) + '%'
Tag.where('lower(name) like lower(?)', pattern).order(:name).limit(limit)
Tag.where('lower(name) like lower(?)', pattern)
.order(:name)
.limit(limit)
.offset(offset)
end end
end end

11
app/services/account_search_service.rb
Unescape View File

@ -1,11 +1,12 @@
# frozen_string_literal: true # frozen_string_literal: true
class AccountSearchService < BaseService class AccountSearchService < BaseService
attr_reader :query, :limit, :options, :account attr_reader :query, :limit, :offset, :options, :account
def call(query, limit, account = nil, options = {}) def call(query, account = nil, options = {})
@query = query.strip @query = query.strip
@limit = limit @limit = options[:limit].to_i
@offset = options[:offset].to_i
@options = options @options = options
@account = account @account = account
@ -83,11 +84,11 @@ class AccountSearchService < BaseService
end end
def advanced_search_results def advanced_search_results
Account.advanced_search_for(terms_for_query, account, limit, options[:following]) Account.advanced_search_for(terms_for_query, account, limit, options[:following], offset)
end end
def simple_search_results def simple_search_results
Account.search_for(terms_for_query, limit) Account.search_for(terms_for_query, limit, offset)
end end
def terms_for_query def terms_for_query

84
app/services/search_service.rb
Unescape View File

@ -1,18 +1,18 @@
# frozen_string_literal: true # frozen_string_literal: true
class SearchService < BaseService class SearchService < BaseService
attr_accessor :query, :account, :limit, :resolve def call(query, account, limit, options = {})
def call(query, limit, resolve = false, account = nil)
@query = query.strip @query = query.strip
@account = account @account = account
@limit = limit @options = options
@resolve = resolve @limit = limit.to_i
@offset = options[:type].blank? ? 0 : options[:offset].to_i
@resolve = options[:resolve] || false
default_results.tap do |results| default_results.tap do |results|
if url_query? if url_query?
results.merge!(url_resource_results) unless url_resource.nil? results.merge!(url_resource_results) unless url_resource.nil?
elsif query.present? elsif @query.present?
results[:accounts] = perform_accounts_search! if account_searchable? results[:accounts] = perform_accounts_search! if account_searchable?
results[:statuses] = perform_statuses_search! if full_text_searchable? results[:statuses] = perform_statuses_search! if full_text_searchable?
results[:hashtags] = perform_hashtags_search! if hashtag_searchable? results[:hashtags] = perform_hashtags_search! if hashtag_searchable?
@ -23,23 +23,46 @@ class SearchService < BaseService
private private
def perform_accounts_search! def perform_accounts_search!
AccountSearchService.new.call(query, limit, account, resolve: resolve) AccountSearchService.new.call(
@query,
@account,
limit: @limit,
resolve: @resolve,
offset: @offset
)
end end
def perform_statuses_search! def perform_statuses_search!
statuses = StatusesIndex.filter(term: { searchable_by: account.id }) definition = StatusesIndex.filter(term: { searchable_by: @account.id })
.query(multi_match: { type: 'most_fields', query: query, operator: 'and', fields: %w(text text.stemmed) }) .query(multi_match: { type: 'most_fields', query: @query, operator: 'and', fields: %w(text text.stemmed) })
.limit(limit)
.objects if @options[:account_id].present?
.compact definition = definition.filter(term: { account_id: @options[:account_id] })
end
if @options[:min_id].present? || @options[:max_id].present?
range = {}
range[:gt] = @options[:min_id].to_i if @options[:min_id].present?
range[:lt] = @options[:max_id].to_i if @options[:max_id].present?
definition = definition.filter(range: { id: range })
end
results = definition.limit(@limit).offset(@offset).objects.compact
account_ids = results.map(&:account_id)
account_domains = results.map(&:account_domain)
preloaded_relations = relations_map_for_account(@account, account_ids, account_domains)
statuses.reject { |status| StatusFilter.new(status, account).filtered? } results.reject { |status| StatusFilter.new(status, @account, preloaded_relations).filtered? }
rescue Faraday::ConnectionFailed rescue Faraday::ConnectionFailed
[] []
end end
def perform_hashtags_search! def perform_hashtags_search!
Tag.search_for(query.gsub(/\A#/, ''), limit) Tag.search_for(
@query.gsub(/\A#/, ''),
@limit,
@offset
)
end end
def default_results def default_results
@ -47,7 +70,7 @@ class SearchService < BaseService
end end
def url_query? def url_query?
query =~ /\Ahttps?:\/\// @options[:type].blank? && @query =~ /\Ahttps?:\/\//
end end
def url_resource_results def url_resource_results
@ -55,7 +78,7 @@ class SearchService < BaseService
end end
def url_resource def url_resource
@_url_resource ||= ResolveURLService.new.call(query, on_behalf_of: @account) @_url_resource ||= ResolveURLService.new.call(@query, on_behalf_of: @account)
end end
def url_resource_symbol def url_resource_symbol
@ -64,14 +87,37 @@ class SearchService < BaseService
def full_text_searchable? def full_text_searchable?
return false unless Chewy.enabled? return false unless Chewy.enabled?
!account.nil? && !((query.start_with?('#') || query.include?('@')) && !query.include?(' '))
statuses_search? && !@account.nil? && !((@query.start_with?('#') || @query.include?('@')) && !@query.include?(' '))
end end
def account_searchable? def account_searchable?
!(query.include?('@') && query.include?(' ')) account_search? && !(@query.include?('@') && @query.include?(' '))
end end
def hashtag_searchable? def hashtag_searchable?
!query.include?('@') hashtag_search? && !@query.include?('@')
end
def account_search?
@options[:type].blank? || @options[:type] == 'accounts'
end
def hashtag_search?
@options[:type].blank? || @options[:type] == 'hashtags'
end
def statuses_search?
@options[:type].blank? || @options[:type] == 'statuses'
end
def relations_map_for_account(account, account_ids, domains)
{
blocking: Account.blocking_map(account_ids, account.id),
blocked_by: Account.blocked_by_map(account_ids, account.id),
muting: Account.muting_map(account_ids, account.id),
following: Account.following_map(account_ids, account.id),
domain_blocking_by_domain: Account.domain_blocking_map_by_domain(domains, account.id),
}
end end
end end

181
config/brakeman.ignore
Unescape View File

@ -1,5 +1,25 @@
{ {
"ignored_warnings": [ "ignored_warnings": [
{
"warning_type": "Mass Assignment",
"warning_code": 105,
"fingerprint": "0117d2be5947ea4e4fbed9c15f23c6615b12c6892973411820c83d079808819d",
"check_name": "PermitAttributes",
"message": "Potentially dangerous key allowed for mass assignment",
"file": "app/controllers/api/v1/search_controller.rb",
"line": 30,
"link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
"code": "params.permit(:type, :offset, :min_id, :max_id, :account_id)",
"render_path": null,
"location": {
"type": "method",
"class": "Api::V1::SearchController",
"method": "search_params"
},
"user_input": ":account_id",
"confidence": "High",
"note": ""
},
{ {
"warning_type": "SQL Injection", "warning_type": "SQL Injection",
"warning_code": 0, "warning_code": 0,
@ -20,25 +40,6 @@
"confidence": "High", "confidence": "High",
"note": "" "note": ""
}, },
{
"warning_type": "Cross-Site Scripting",
"warning_code": 4,
"fingerprint": "0adbe361b91afff22ba51e5fc2275ec703cc13255a0cb3eecd8dab223ab9f61e",
"check_name": "LinkToHref",
"message": "Potentially unsafe model attribute in link_to href",
"file": "app/views/admin/accounts/show.html.haml",
"line": 167,
"link": "https://brakemanscanner.org/docs/warning_types/link_to_href",
"code": "link_to(Account.find(params[:id]).inbox_url, Account.find(params[:id]).inbox_url)",
"render_path": [{"type":"controller","class":"Admin::AccountsController","method":"show","line":18,"file":"app/controllers/admin/accounts_controller.rb"}],
"location": {
"type": "template",
"template": "admin/accounts/show"
},
"user_input": "Account.find(params[:id]).inbox_url",
"confidence": "Weak",
"note": ""
},
{ {
"warning_type": "SQL Injection", "warning_type": "SQL Injection",
"warning_code": 0, "warning_code": 0,
@ -46,7 +47,7 @@
"check_name": "SQL", "check_name": "SQL",
"message": "Possible SQL injection", "message": "Possible SQL injection",
"file": "app/models/status.rb", "file": "app/models/status.rb",
"line": 84, "line": 87,
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
"code": "result.joins(\"INNER JOIN statuses_tags t#{id} ON t#{id}.status_id = statuses.id AND t#{id}.tag_id = #{id}\")", "code": "result.joins(\"INNER JOIN statuses_tags t#{id} ON t#{id}.status_id = statuses.id AND t#{id}.tag_id = #{id}\")",
"render_path": null, "render_path": null,
@ -59,44 +60,6 @@
"confidence": "Weak", "confidence": "Weak",
"note": "" "note": ""
}, },
{
"warning_type": "Cross-Site Scripting",
"warning_code": 4,
"fingerprint": "1fc29c578d0c89bf13bd5476829d272d54cd06b92ccf6df18568fa1f2674926e",
"check_name": "LinkToHref",
"message": "Potentially unsafe model attribute in link_to href",
"file": "app/views/admin/accounts/show.html.haml",
"line": 173,
"link": "https://brakemanscanner.org/docs/warning_types/link_to_href",
"code": "link_to(Account.find(params[:id]).shared_inbox_url, Account.find(params[:id]).shared_inbox_url)",
"render_path": [{"type":"controller","class":"Admin::AccountsController","method":"show","line":18,"file":"app/controllers/admin/accounts_controller.rb"}],
"location": {
"type": "template",
"template": "admin/accounts/show"
},
"user_input": "Account.find(params[:id]).shared_inbox_url",
"confidence": "Weak",
"note": ""
},
{
"warning_type": "Cross-Site Scripting",
"warning_code": 4,
"fingerprint": "2129d4c1e63a351d28d8d2937ff0b50237809c3df6725c0c5ef82b881dbb2086",
"check_name": "LinkToHref",
"message": "Potentially unsafe model attribute in link_to href",
"file": "app/views/admin/accounts/show.html.haml",
"line": 75,
"link": "https://brakemanscanner.org/docs/warning_types/link_to_href",
"code": "link_to(Account.find(params[:id]).url, Account.find(params[:id]).url)",
"render_path": [{"type":"controller","class":"Admin::AccountsController","method":"show","line":18,"file":"app/controllers/admin/accounts_controller.rb"}],
"location": {
"type": "template",
"template": "admin/accounts/show"
},
"user_input": "Account.find(params[:id]).url",
"confidence": "Weak",
"note": ""
},
{ {
"warning_type": "Mass Assignment", "warning_type": "Mass Assignment",
"warning_code": 105, "warning_code": 105,
@ -104,7 +67,7 @@
"check_name": "PermitAttributes", "check_name": "PermitAttributes",
"message": "Potentially dangerous key allowed for mass assignment", "message": "Potentially dangerous key allowed for mass assignment",
"file": "app/controllers/admin/reports_controller.rb", "file": "app/controllers/admin/reports_controller.rb",
"line": 80, "line": 56,
"link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/", "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
"code": "params.permit(:account_id, :resolved, :target_account_id)", "code": "params.permit(:account_id, :resolved, :target_account_id)",
"render_path": null, "render_path": null,
@ -127,7 +90,7 @@
"line": 4, "line": 4,
"link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/", "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
"code": "render(action => Admin::ActionLog.page(params[:page]), {})", "code": "render(action => Admin::ActionLog.page(params[:page]), {})",
"render_path": [{"type":"controller","class":"Admin::ActionLogsController","method":"index","line":7,"file":"app/controllers/admin/action_logs_controller.rb"}], "render_path": [{"type":"controller","class":"Admin::ActionLogsController","method":"index","line":7,"file":"app/controllers/admin/action_logs_controller.rb","rendered":{"name":"admin/action_logs/index","file":"/home/eugr/Projects/mastodon/app/views/admin/action_logs/index.html.haml"}}],
"location": { "location": {
"type": "template", "type": "template",
"template": "admin/action_logs/index" "template": "admin/action_logs/index"
@ -143,7 +106,7 @@
"check_name": "Redirect", "check_name": "Redirect",
"message": "Possible unprotected redirect", "message": "Possible unprotected redirect",
"file": "app/controllers/remote_interaction_controller.rb", "file": "app/controllers/remote_interaction_controller.rb",
"line": 20, "line": 21,
"link": "https://brakemanscanner.org/docs/warning_types/redirect/", "link": "https://brakemanscanner.org/docs/warning_types/redirect/",
"code": "redirect_to(RemoteFollow.new(resource_params).interact_address_for(Status.find(params[:id])))", "code": "redirect_to(RemoteFollow.new(resource_params).interact_address_for(Status.find(params[:id])))",
"render_path": null, "render_path": null,
@ -156,25 +119,6 @@
"confidence": "High", "confidence": "High",
"note": "" "note": ""
}, },
{
"warning_type": "Cross-Site Scripting",
"warning_code": 4,
"fingerprint": "64b5b2a02ede9c2b3598881eb5a466d63f7d27fe0946aa00d570111ec7338d2e",
"check_name": "LinkToHref",
"message": "Potentially unsafe model attribute in link_to href",
"file": "app/views/admin/accounts/show.html.haml",
"line": 176,
"link": "https://brakemanscanner.org/docs/warning_types/link_to_href",
"code": "link_to(Account.find(params[:id]).followers_url, Account.find(params[:id]).followers_url)",
"render_path": [{"type":"controller","class":"Admin::AccountsController","method":"show","line":18,"file":"app/controllers/admin/accounts_controller.rb"}],
"location": {
"type": "template",
"template": "admin/accounts/show"
},
"user_input": "Account.find(params[:id]).followers_url",
"confidence": "Weak",
"note": ""
},
{ {
"warning_type": "Dynamic Render Path", "warning_type": "Dynamic Render Path",
"warning_code": 15, "warning_code": 15,
@ -185,7 +129,7 @@
"line": 3, "line": 3,
"link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/", "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
"code": "render(action => \"stream_entries/#{Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase}\", { Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase.to_sym => Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity, :centered => true, :autoplay => ActiveModel::Type::Boolean.new.cast(params[:autoplay]) })", "code": "render(action => \"stream_entries/#{Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase}\", { Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase.to_sym => Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity, :centered => true, :autoplay => ActiveModel::Type::Boolean.new.cast(params[:autoplay]) })",
"render_path": [{"type":"controller","class":"StatusesController","method":"embed","line":59,"file":"app/controllers/statuses_controller.rb"}], "render_path": [{"type":"controller","class":"StatusesController","method":"embed","line":63,"file":"app/controllers/statuses_controller.rb","rendered":{"name":"stream_entries/embed","file":"/home/eugr/Projects/mastodon/app/views/stream_entries/embed.html.haml"}}],
"location": { "location": {
"type": "template", "type": "template",
"template": "stream_entries/embed" "template": "stream_entries/embed"
@ -201,7 +145,7 @@
"check_name": "SQL", "check_name": "SQL",
"message": "Possible SQL injection", "message": "Possible SQL injection",
"file": "app/models/status.rb", "file": "app/models/status.rb",
"line": 89, "line": 92,
"link": "https://brakemanscanner.org/docs/warning_types/sql_injection/", "link": "https://brakemanscanner.org/docs/warning_types/sql_injection/",
"code": "result.joins(\"LEFT OUTER JOIN statuses_tags t#{id} ON t#{id}.status_id = statuses.id AND t#{id}.tag_id = #{id}\")", "code": "result.joins(\"LEFT OUTER JOIN statuses_tags t#{id} ON t#{id}.status_id = statuses.id AND t#{id}.tag_id = #{id}\")",
"render_path": null, "render_path": null,
@ -214,25 +158,6 @@
"confidence": "Weak", "confidence": "Weak",
"note": "" "note": ""
}, },
{
"warning_type": "Cross-Site Scripting",
"warning_code": 4,
"fingerprint": "82f7b0d09beb3ab68e0fa16be63cedf4e820f2490326e9a1cec05761d92446cd",
"check_name": "LinkToHref",
"message": "Potentially unsafe model attribute in link_to href",
"file": "app/views/admin/accounts/show.html.haml",
"line": 149,
"link": "https://brakemanscanner.org/docs/warning_types/link_to_href",
"code": "link_to(Account.find(params[:id]).salmon_url, Account.find(params[:id]).salmon_url)",
"render_path": [{"type":"controller","class":"Admin::AccountsController","method":"show","line":18,"file":"app/controllers/admin/accounts_controller.rb"}],
"location": {
"type": "template",
"template": "admin/accounts/show"
},
"user_input": "Account.find(params[:id]).salmon_url",
"confidence": "Weak",
"note": ""
},
{ {
"warning_type": "Dynamic Render Path", "warning_type": "Dynamic Render Path",
"warning_code": 15, "warning_code": 15,
@ -243,7 +168,7 @@
"line": 45, "line": 45,
"link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/", "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
"code": "render(action => filtered_custom_emojis.eager_load(:local_counterpart).page(params[:page]), {})", "code": "render(action => filtered_custom_emojis.eager_load(:local_counterpart).page(params[:page]), {})",
"render_path": [{"type":"controller","class":"Admin::CustomEmojisController","method":"index","line":11,"file":"app/controllers/admin/custom_emojis_controller.rb"}], "render_path": [{"type":"controller","class":"Admin::CustomEmojisController","method":"index","line":11,"file":"app/controllers/admin/custom_emojis_controller.rb","rendered":{"name":"admin/custom_emojis/index","file":"/home/eugr/Projects/mastodon/app/views/admin/custom_emojis/index.html.haml"}}],
"location": { "location": {
"type": "template", "type": "template",
"template": "admin/custom_emojis/index" "template": "admin/custom_emojis/index"
@ -279,10 +204,10 @@
"check_name": "Render", "check_name": "Render",
"message": "Render path contains parameter value", "message": "Render path contains parameter value",
"file": "app/views/admin/accounts/index.html.haml", "file": "app/views/admin/accounts/index.html.haml",
"line": 67, "line": 47,
"link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/", "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
"code": "render(action => filtered_accounts.page(params[:page]), {})", "code": "render(action => filtered_accounts.page(params[:page]), {})",
"render_path": [{"type":"controller","class":"Admin::AccountsController","method":"index","line":12,"file":"app/controllers/admin/accounts_controller.rb"}], "render_path": [{"type":"controller","class":"Admin::AccountsController","method":"index","line":12,"file":"app/controllers/admin/accounts_controller.rb","rendered":{"name":"admin/accounts/index","file":"/home/eugr/Projects/mastodon/app/views/admin/accounts/index.html.haml"}}],
"location": { "location": {
"type": "template", "type": "template",
"template": "admin/accounts/index" "template": "admin/accounts/index"
@ -298,7 +223,7 @@
"check_name": "Redirect", "check_name": "Redirect",
"message": "Possible unprotected redirect", "message": "Possible unprotected redirect",
"file": "app/controllers/media_controller.rb", "file": "app/controllers/media_controller.rb",
"line": 10, "line": 14,
"link": "https://brakemanscanner.org/docs/warning_types/redirect/", "link": "https://brakemanscanner.org/docs/warning_types/redirect/",
"code": "redirect_to(MediaAttachment.attached.find_by!(:shortcode => ((params[:id] or params[:medium_id]))).file.url(:original))", "code": "redirect_to(MediaAttachment.attached.find_by!(:shortcode => ((params[:id] or params[:medium_id]))).file.url(:original))",
"render_path": null, "render_path": null,
@ -311,25 +236,6 @@
"confidence": "High", "confidence": "High",
"note": "" "note": ""
}, },
{
"warning_type": "Cross-Site Scripting",
"warning_code": 4,
"fingerprint": "bb0ad5c4a42e06e3846c2089ff5269c17f65483a69414f6ce65eecf2bb11fab7",
"check_name": "LinkToHref",
"message": "Potentially unsafe model attribute in link_to href",
"file": "app/views/admin/accounts/show.html.haml",
"line": 138,
"link": "https://brakemanscanner.org/docs/warning_types/link_to_href",
"code": "link_to(Account.find(params[:id]).remote_url, Account.find(params[:id]).remote_url)",
"render_path": [{"type":"controller","class":"Admin::AccountsController","method":"show","line":18,"file":"app/controllers/admin/accounts_controller.rb"}],
"location": {
"type": "template",
"template": "admin/accounts/show"
},
"user_input": "Account.find(params[:id]).remote_url",
"confidence": "Weak",
"note": ""
},
{ {
"warning_type": "Redirect", "warning_type": "Redirect",
"warning_code": 18, "warning_code": 18,
@ -350,25 +256,6 @@
"confidence": "High", "confidence": "High",
"note": "" "note": ""
}, },
{
"warning_type": "Cross-Site Scripting",
"warning_code": 4,
"fingerprint": "e04aafe1e06cf8317fb6ac0a7f35783e45aa1274272ee6eaf28d39adfdad489b",
"check_name": "LinkToHref",
"message": "Potentially unsafe model attribute in link_to href",
"file": "app/views/admin/accounts/show.html.haml",
"line": 170,
"link": "https://brakemanscanner.org/docs/warning_types/link_to_href",
"code": "link_to(Account.find(params[:id]).outbox_url, Account.find(params[:id]).outbox_url)",
"render_path": [{"type":"controller","class":"Admin::AccountsController","method":"show","line":18,"file":"app/controllers/admin/accounts_controller.rb"}],
"location": {
"type": "template",
"template": "admin/accounts/show"
},
"user_input": "Account.find(params[:id]).outbox_url",
"confidence": "Weak",
"note": ""
},
{ {
"warning_type": "Mass Assignment", "warning_type": "Mass Assignment",
"warning_code": 105, "warning_code": 105,
@ -376,7 +263,7 @@
"check_name": "PermitAttributes", "check_name": "PermitAttributes",
"message": "Potentially dangerous key allowed for mass assignment", "message": "Potentially dangerous key allowed for mass assignment",
"file": "app/controllers/api/v1/reports_controller.rb", "file": "app/controllers/api/v1/reports_controller.rb",
"line": 37, "line": 36,
"link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/", "link": "https://brakemanscanner.org/docs/warning_types/mass_assignment/",
"code": "params.permit(:account_id, :comment, :forward, :status_ids => ([]))", "code": "params.permit(:account_id, :comment, :forward, :status_ids => ([]))",
"render_path": null, "render_path": null,
@ -399,7 +286,7 @@
"line": 23, "line": 23,
"link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/", "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/",
"code": "render(partial => \"stream_entries/#{Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase}\", { :locals => ({ Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase.to_sym => Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity, :include_threads => true }) })", "code": "render(partial => \"stream_entries/#{Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase}\", { :locals => ({ Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity_type.downcase.to_sym => Account.find_local!(params[:account_username]).statuses.find(params[:id]).stream_entry.activity, :include_threads => true }) })",
"render_path": [{"type":"controller","class":"StatusesController","method":"show","line":30,"file":"app/controllers/statuses_controller.rb"}], "render_path": [{"type":"controller","class":"StatusesController","method":"show","line":34,"file":"app/controllers/statuses_controller.rb","rendered":{"name":"stream_entries/show","file":"/home/eugr/Projects/mastodon/app/views/stream_entries/show.html.haml"}}],
"location": { "location": {
"type": "template", "type": "template",
"template": "stream_entries/show" "template": "stream_entries/show"
@ -409,6 +296,6 @@
"note": "" "note": ""
} }
], ],
"updated": "2018-10-20 23:24:45 +1300", "updated": "2019-02-21 02:30:29 +0100",
"brakeman_version": "4.2.1" "brakeman_version": "4.4.0"
} }

36
spec/services/account_search_service_spec.rb
Unescape View File

@ -4,18 +4,18 @@ describe AccountSearchService, type: :service do
describe '.call' do describe '.call' do
describe 'with a query to ignore' do describe 'with a query to ignore' do
it 'returns empty array for missing query' do it 'returns empty array for missing query' do
results = subject.call('', 10) results = subject.call('', nil, limit: 10)
expect(results).to eq [] expect(results).to eq []
end end
it 'returns empty array for hashtag query' do it 'returns empty array for hashtag query' do
results = subject.call('#tag', 10) results = subject.call('#tag', nil, limit: 10)
expect(results).to eq [] expect(results).to eq []
end end
it 'returns empty array for limit zero' do it 'returns empty array for limit zero' do
Fabricate(:account, username: 'match') Fabricate(:account, username: 'match')
results = subject.call('match', 0) results = subject.call('match', nil, limit: 0)
expect(results).to eq [] expect(results).to eq []
end end
@ -25,7 +25,7 @@ describe AccountSearchService, type: :service do
it 'does not return a nil entry in the array for the exact match' do it 'does not return a nil entry in the array for the exact match' do
match = Fabricate(:account, username: 'matchingusername') match = Fabricate(:account, username: 'matchingusername')
results = subject.call('match', 5) results = subject.call('match', nil, limit: 5)
expect(results).to eq [match] expect(results).to eq [match]
end end
end end
@ -35,7 +35,7 @@ describe AccountSearchService, type: :service do
before do before do
allow(Account).to receive(:find_local) allow(Account).to receive(:find_local)
allow(Account).to receive(:search_for) allow(Account).to receive(:search_for)
subject.call('@', 10) subject.call('@', nil, limit: 10)
end end
it 'uses find_local with empty query to look for local accounts' do it 'uses find_local with empty query to look for local accounts' do
@ -47,7 +47,7 @@ describe AccountSearchService, type: :service do
before do before do
allow(Account).to receive(:find_local) allow(Account).to receive(:find_local)
allow(Account).to receive(:search_for) allow(Account).to receive(:search_for)
subject.call('one', 10) subject.call('one', nil, limit: 10)
end end
it 'uses find_local to look for local accounts' do it 'uses find_local to look for local accounts' do
@ -55,7 +55,7 @@ describe AccountSearchService, type: :service do
end end
it 'uses search_for to find matches' do it 'uses search_for to find matches' do
expect(Account).to have_received(:search_for).with('one', 10) expect(Account).to have_received(:search_for).with('one', 10, 0)
end end
end end
@ -65,16 +65,16 @@ describe AccountSearchService, type: :service do
end end
it 'uses find_remote to look for remote accounts' do it 'uses find_remote to look for remote accounts' do
subject.call('two@example.com', 10) subject.call('two@example.com', nil, limit: 10)
expect(Account).to have_received(:find_remote).with('two', 'example.com') expect(Account).to have_received(:find_remote).with('two', 'example.com')
end end
describe 'and there is no account provided' do describe 'and there is no account provided' do
it 'uses search_for to find matches' do it 'uses search_for to find matches' do
allow(Account).to receive(:search_for) allow(Account).to receive(:search_for)
subject.call('two@example.com', 10, nil, resolve: false) subject.call('two@example.com', nil, limit: 10, resolve: false)
expect(Account).to have_received(:search_for).with('two example.com', 10) expect(Account).to have_received(:search_for).with('two example.com', 10, 0)
end end
end end
@ -82,9 +82,9 @@ describe AccountSearchService, type: :service do
it 'uses advanced_search_for to find matches' do it 'uses advanced_search_for to find matches' do
account = Fabricate(:account) account = Fabricate(:account)
allow(Account).to receive(:advanced_search_for) allow(Account).to receive(:advanced_search_for)
subject.call('two@example.com', 10, account, resolve: false) subject.call('two@example.com', account, limit: 10, resolve: false)
expect(Account).to have_received(:advanced_search_for).with('two example.com', account, 10, nil) expect(Account).to have_received(:advanced_search_for).with('two example.com', account, 10, nil, 0)
end end
end end
end end
@ -95,7 +95,7 @@ describe AccountSearchService, type: :service do
partial = Fabricate(:account, username: 'exactness') partial = Fabricate(:account, username: 'exactness')
exact = Fabricate(:account, username: 'exact') exact = Fabricate(:account, username: 'exact')
results = subject.call('exact', 10) results = subject.call('exact', nil, limit: 10)
expect(results.size).to eq 2 expect(results.size).to eq 2
expect(results).to eq [exact, partial] expect(results).to eq [exact, partial]
end end
@ -114,7 +114,7 @@ describe AccountSearchService, type: :service do
exact = Fabricate(:account, username: 'e') exact = Fabricate(:account, username: 'e')
Rails.configuration.x.local_domain = 'example.com' Rails.configuration.x.local_domain = 'example.com'
results = subject.call('e@example.com', 2) results = subject.call('e@example.com', nil, limit: 2)
expect(results.size).to eq 2 expect(results.size).to eq 2
expect(results).to eq([exact, remote]).or eq([exact, remote_too]) expect(results).to eq([exact, remote]).or eq([exact, remote_too])
end end
@ -125,7 +125,7 @@ describe AccountSearchService, type: :service do
service = double(call: nil) service = double(call: nil)
allow(ResolveAccountService).to receive(:new).and_return(service) allow(ResolveAccountService).to receive(:new).and_return(service)
results = subject.call('newuser@remote.com', 10, nil, resolve: true) results = subject.call('newuser@remote.com', nil, limit: 10, resolve: true)
expect(service).to have_received(:call).with('newuser@remote.com') expect(service).to have_received(:call).with('newuser@remote.com')
end end
@ -133,7 +133,7 @@ describe AccountSearchService, type: :service do
service = double(call: nil) service = double(call: nil)
allow(ResolveAccountService).to receive(:new).and_return(service) allow(ResolveAccountService).to receive(:new).and_return(service)
results = subject.call('newuser@remote.com', 10, nil, resolve: false) results = subject.call('newuser@remote.com', nil, limit: 10, resolve: false)
expect(service).not_to have_received(:call) expect(service).not_to have_received(:call)
end end
end end
@ -143,7 +143,7 @@ describe AccountSearchService, type: :service do
partial = Fabricate(:account, username: 'exactness') partial = Fabricate(:account, username: 'exactness')
exact = Fabricate(:account, username: 'exact', suspended: true) exact = Fabricate(:account, username: 'exact', suspended: true)
results = subject.call('exact', 10) results = subject.call('exact', nil, limit: 10)
expect(results.size).to eq 1 expect(results.size).to eq 1
expect(results).to eq [partial] expect(results).to eq [partial]
end end
@ -151,7 +151,7 @@ describe AccountSearchService, type: :service do
it "does not return suspended remote accounts" do it "does not return suspended remote accounts" do
remote = Fabricate(:account, username: 'a', domain: 'remote', display_name: 'e', suspended: true) remote = Fabricate(:account, username: 'a', domain: 'remote', display_name: 'e', suspended: true)
results = subject.call('a@example.com', 2) results = subject.call('a@example.com', nil, limit: 2)
expect(results.size).to eq 0 expect(results.size).to eq 0
expect(results).to eq [] expect(results).to eq []
end end

20
spec/services/search_service_spec.rb
Unescape View File

@ -10,7 +10,7 @@ describe SearchService, type: :service do
it 'returns empty results without searching' do it 'returns empty results without searching' do
allow(AccountSearchService).to receive(:new) allow(AccountSearchService).to receive(:new)
allow(Tag).to receive(:search_for) allow(Tag).to receive(:search_for)
results = subject.call('', 10) results = subject.call('', nil, 10)
expect(results).to eq(empty_results) expect(results).to eq(empty_results)
expect(AccountSearchService).not_to have_received(:new) expect(AccountSearchService).not_to have_received(:new)
@ -27,7 +27,7 @@ describe SearchService, type: :service do
it 'returns the empty results' do it 'returns the empty results' do
service = double(call: nil) service = double(call: nil)
allow(ResolveURLService).to receive(:new).and_return(service) allow(ResolveURLService).to receive(:new).and_return(service)
results = subject.call(@query, 10) results = subject.call(@query, nil, 10)
expect(service).to have_received(:call).with(@query, on_behalf_of: nil) expect(service).to have_received(:call).with(@query, on_behalf_of: nil)
expect(results).to eq empty_results expect(results).to eq empty_results
@ -40,7 +40,7 @@ describe SearchService, type: :service do
service = double(call: account) service = double(call: account)
allow(ResolveURLService).to receive(:new).and_return(service) allow(ResolveURLService).to receive(:new).and_return(service)
results = subject.call(@query, 10) results = subject.call(@query, nil, 10)
expect(service).to have_received(:call).with(@query, on_behalf_of: nil) expect(service).to have_received(:call).with(@query, on_behalf_of: nil)
expect(results).to eq empty_results.merge(accounts: [account]) expect(results).to eq empty_results.merge(accounts: [account])
end end
@ -52,7 +52,7 @@ describe SearchService, type: :service do
service = double(call: status) service = double(call: status)
allow(ResolveURLService).to receive(:new).and_return(service) allow(ResolveURLService).to receive(:new).and_return(service)
results = subject.call(@query, 10) results = subject.call(@query, nil, 10)
expect(service).to have_received(:call).with(@query, on_behalf_of: nil) expect(service).to have_received(:call).with(@query, on_behalf_of: nil)
expect(results).to eq empty_results.merge(statuses: [status]) expect(results).to eq empty_results.merge(statuses: [status])
end end
@ -67,8 +67,8 @@ describe SearchService, type: :service do
service = double(call: [account]) service = double(call: [account])
allow(AccountSearchService).to receive(:new).and_return(service) allow(AccountSearchService).to receive(:new).and_return(service)
results = subject.call(query, 10) results = subject.call(query, nil, 10)
expect(service).to have_received(:call).with(query, 10, nil, resolve: false) expect(service).to have_received(:call).with(query, nil, limit: 10, offset: 0, resolve: false)
expect(results).to eq empty_results.merge(accounts: [account]) expect(results).to eq empty_results.merge(accounts: [account])
end end
end end
@ -77,17 +77,17 @@ describe SearchService, type: :service do
it 'includes the tag in the results' do it 'includes the tag in the results' do
query = '#tag' query = '#tag'
tag = Tag.new tag = Tag.new
allow(Tag).to receive(:search_for).with('tag', 10).and_return([tag]) allow(Tag).to receive(:search_for).with('tag', 10, 0).and_return([tag])
results = subject.call(query, 10) results = subject.call(query, nil, 10)
expect(Tag).to have_received(:search_for).with('tag', 10) expect(Tag).to have_received(:search_for).with('tag', 10, 0)
expect(results).to eq empty_results.merge(hashtags: [tag]) expect(results).to eq empty_results.merge(hashtags: [tag])
end end
it 'does not include tag when starts with @ character' do it 'does not include tag when starts with @ character' do
query = '@username' query = '@username'
allow(Tag).to receive(:search_for) allow(Tag).to receive(:search_for)
results = subject.call(query, 10) results = subject.call(query, nil, 10)
expect(Tag).not_to have_received(:search_for) expect(Tag).not_to have_received(:search_for)
expect(results).to eq empty_results expect(results).to eq empty_results
end end

Write Preview
Loading…
Cancel
Save