From 5614e6724e5131f33197ecbc1998058e9794aae9 Mon Sep 17 00:00:00 2001 From: Claire Date: Thu, 4 Mar 2021 00:12:26 +0100 Subject: Fix URL scanning in note length validator and preview card fetching (#15827) * Add tests * Fix URL scanning in note length validator and preview card fetching --- app/lib/language_detector.rb | 2 +- app/services/fetch_link_card_service.rb | 15 ++++++++------- app/validators/note_length_validator.rb | 2 +- app/validators/status_length_validator.rb | 8 +------- 4 files changed, 11 insertions(+), 16 deletions(-) (limited to 'app') diff --git a/app/lib/language_detector.rb b/app/lib/language_detector.rb index 2cc8ac615..40452eddc 100644 --- a/app/lib/language_detector.rb +++ b/app/lib/language_detector.rb @@ -69,7 +69,7 @@ class LanguageDetector def simplify_text(text) new_text = remove_html(text) - new_text.gsub!(FetchLinkCardService::URL_PATTERN, '') + new_text.gsub!(FetchLinkCardService::URL_PATTERN, '\1') new_text.gsub!(Account::MENTION_RE, '') new_text.gsub!(Tag::HASHTAG_RE) { |string| string.gsub(/[#_]/, '#' => '', '_' => ' ').gsub(/[a-z][A-Z]|[a-zA-Z][\d]/) { |s| s.insert(1, ' ') }.downcase } new_text.gsub!(/:#{CustomEmoji::SHORTCODE_RE_FRAGMENT}:/, '') diff --git a/app/services/fetch_link_card_service.rb b/app/services/fetch_link_card_service.rb index d4e4931e6..fa1636e41 100644 --- a/app/services/fetch_link_card_service.rb +++ b/app/services/fetch_link_card_service.rb @@ -2,12 +2,13 @@ class FetchLinkCardService < BaseService URL_PATTERN = %r{ - ( # $1 URL - (https?:\/\/) # $2 Protocol (required) - (#{Twitter::TwitterText::Regex[:valid_domain]}) # $3 Domain(s) - (?::(#{Twitter::TwitterText::Regex[:valid_port_number]}))? # $4 Port number (optional) - (/#{Twitter::TwitterText::Regex[:valid_url_path]}*)? # $5 URL Path and anchor - (\?#{Twitter::TwitterText::Regex[:valid_url_query_chars]}*#{Twitter::TwitterText::Regex[:valid_url_query_ending_chars]})? # $6 Query String + (#{Twitter::TwitterText::Regex[:valid_url_preceding_chars]}) # $1 preceeding chars + ( # $2 URL + (https?:\/\/) # $3 Protocol (required) + (#{Twitter::TwitterText::Regex[:valid_domain]}) # $4 Domain(s) + (?::(#{Twitter::TwitterText::Regex[:valid_port_number]}))? # $5 Port number (optional) + (/#{Twitter::TwitterText::Regex[:valid_url_path]}*)? # $6 URL Path and anchor + (\?#{Twitter::TwitterText::Regex[:valid_url_query_chars]}*#{Twitter::TwitterText::Regex[:valid_url_query_ending_chars]})? # $7 Query String ) }iox @@ -63,7 +64,7 @@ class FetchLinkCardService < BaseService def parse_urls if @status.local? - urls = @status.text.scan(URL_PATTERN).map { |array| Addressable::URI.parse(array[0]).normalize } + urls = @status.text.scan(URL_PATTERN).map { |array| Addressable::URI.parse(array[1]).normalize } else html = Nokogiri::HTML(@status.text) links = html.css('a') diff --git a/app/validators/note_length_validator.rb b/app/validators/note_length_validator.rb index 7ea2bb3e5..554ad49ce 100644 --- a/app/validators/note_length_validator.rb +++ b/app/validators/note_length_validator.rb @@ -15,7 +15,7 @@ class NoteLengthValidator < ActiveModel::EachValidator return '' if value.nil? value.dup.tap do |new_text| - new_text.gsub!(FetchLinkCardService::URL_PATTERN, 'x' * 23) + new_text.gsub!(FetchLinkCardService::URL_PATTERN, StatusLengthValidator::URL_PLACEHOLDER) new_text.gsub!(Account::MENTION_RE, '@\2') end end diff --git a/app/validators/status_length_validator.rb b/app/validators/status_length_validator.rb index b56c5a321..d036f1925 100644 --- a/app/validators/status_length_validator.rb +++ b/app/validators/status_length_validator.rb @@ -2,12 +2,6 @@ class StatusLengthValidator < ActiveModel::Validator MAX_CHARS = 500 - URL_PATTERN = %r{ - (?: - (#{Twitter::TwitterText::Regex[:valid_url_preceding_chars]}) - (#{FetchLinkCardService::URL_PATTERN}) - ) - }iox URL_PLACEHOLDER = "\1#{'x' * 23}" def validate(status) @@ -35,7 +29,7 @@ class StatusLengthValidator < ActiveModel::Validator return '' if @status.text.nil? @status.text.dup.tap do |new_text| - new_text.gsub!(URL_PATTERN, URL_PLACEHOLDER) + new_text.gsub!(FetchLinkCardService::URL_PATTERN, URL_PLACEHOLDER) new_text.gsub!(Account::MENTION_RE, '@\2') end end -- cgit From 67c5cdea4016405483d493d1efb3765a58587d1c Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 7 Mar 2021 07:06:56 +0100 Subject: Fix some ignored brakeman warnings (#15829) --- app/views/admin/accounts/index.html.haml | 2 +- app/views/admin/action_logs/index.html.haml | 2 +- .../admin/email_domain_blocks/index.html.haml | 2 +- app/views/admin/instances/index.html.haml | 2 +- config/brakeman.ignore | 124 --------------------- 5 files changed, 4 insertions(+), 128 deletions(-) (limited to 'app') diff --git a/app/views/admin/accounts/index.html.haml b/app/views/admin/accounts/index.html.haml index 8eac226e0..398ab4bb4 100644 --- a/app/views/admin/accounts/index.html.haml +++ b/app/views/admin/accounts/index.html.haml @@ -51,6 +51,6 @@ %th= t('admin.accounts.most_recent_activity') %th %tbody - = render @accounts + = render partial: 'account', collection: @accounts = paginate @accounts diff --git a/app/views/admin/action_logs/index.html.haml b/app/views/admin/action_logs/index.html.haml index e7d9054d9..f7f73150b 100644 --- a/app/views/admin/action_logs/index.html.haml +++ b/app/views/admin/action_logs/index.html.haml @@ -23,6 +23,6 @@ = t 'admin.action_logs.empty' - else .announcements-list - = render @action_logs + = render partial: 'action_log', collection: @action_logs = paginate @action_logs diff --git a/app/views/admin/email_domain_blocks/index.html.haml b/app/views/admin/email_domain_blocks/index.html.haml index 6015cfac0..fa5d86b67 100644 --- a/app/views/admin/email_domain_blocks/index.html.haml +++ b/app/views/admin/email_domain_blocks/index.html.haml @@ -14,6 +14,6 @@ %th= t('admin.email_domain_blocks.domain') %th %tbody - = render @email_domain_blocks + = render partial: 'email_domain_block', collection: @email_domain_blocks = paginate @email_domain_blocks diff --git a/app/views/admin/instances/index.html.haml b/app/views/admin/instances/index.html.haml index 5f20e7ec0..7c7958786 100644 --- a/app/views/admin/instances/index.html.haml +++ b/app/views/admin/instances/index.html.haml @@ -36,6 +36,6 @@ %div.muted-hint.center-text = t 'admin.instances.empty' - else - = render @instances + = render partial: 'instance', collection: @instances = paginate @instances diff --git a/config/brakeman.ignore b/config/brakeman.ignore index dcbfd02b4..2d47a9aaf 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -40,99 +40,6 @@ "confidence": "Weak", "note": "" }, - { - "warning_type": "Dynamic Render Path", - "warning_code": 15, - "fingerprint": "20a660939f2bbf8c665e69f2844031c0564524689a9570a0091ed94846212020", - "check_name": "Render", - "message": "Render path contains parameter value", - "file": "app/views/admin/action_logs/index.html.haml", - "line": 26, - "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/", - "code": "render(action => Admin::ActionLogFilter.new(filter_params).results.page(params[:page]), {})", - "render_path": [ - { - "type": "controller", - "class": "Admin::ActionLogsController", - "method": "index", - "line": 8, - "file": "app/controllers/admin/action_logs_controller.rb", - "rendered": { - "name": "admin/action_logs/index", - "file": "app/views/admin/action_logs/index.html.haml" - } - } - ], - "location": { - "type": "template", - "template": "admin/action_logs/index" - }, - "user_input": "params[:page]", - "confidence": "Weak", - "note": "" - }, - { - "warning_type": "Dynamic Render Path", - "warning_code": 15, - "fingerprint": "371fe16dc4c9d6ab08a20437d65be4825776107a67c38f6d4780a9c703cd44a5", - "check_name": "Render", - "message": "Render path contains parameter value", - "file": "app/views/admin/email_domain_blocks/index.html.haml", - "line": 17, - "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/", - "code": "render(action => EmailDomainBlock.where(:parent_id => nil).includes(:children).order(:id => :desc).page(params[:page]), {})", - "render_path": [ - { - "type": "controller", - "class": "Admin::EmailDomainBlocksController", - "method": "index", - "line": 10, - "file": "app/controllers/admin/email_domain_blocks_controller.rb", - "rendered": { - "name": "admin/email_domain_blocks/index", - "file": "app/views/admin/email_domain_blocks/index.html.haml" - } - } - ], - "location": { - "type": "template", - "template": "admin/email_domain_blocks/index" - }, - "user_input": "params[:page]", - "confidence": "Weak", - "note": "" - }, - { - "warning_type": "Dynamic Render Path", - "warning_code": 15, - "fingerprint": "4704e8093e3e0561bf705f892e8fc6780419f8255f4440b1c0afd09339bd6446", - "check_name": "Render", - "message": "Render path contains parameter value", - "file": "app/views/admin/instances/index.html.haml", - "line": 39, - "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/", - "code": "render(action => filtered_instances.page(params[:page]), {})", - "render_path": [ - { - "type": "controller", - "class": "Admin::InstancesController", - "method": "index", - "line": 10, - "file": "app/controllers/admin/instances_controller.rb", - "rendered": { - "name": "admin/instances/index", - "file": "app/views/admin/instances/index.html.haml" - } - } - ], - "location": { - "type": "template", - "template": "admin/instances/index" - }, - "user_input": "params[:page]", - "confidence": "Weak", - "note": "" - }, { "warning_type": "Redirect", "warning_code": 18, @@ -253,37 +160,6 @@ "confidence": "Medium", "note": "" }, - { - "warning_type": "Dynamic Render Path", - "warning_code": 15, - "fingerprint": "9f31d941f3910dba2e9bfcd81aef4513249bd24c02d0f98e13ad44fdeeccd0e8", - "check_name": "Render", - "message": "Render path contains parameter value", - "file": "app/views/admin/accounts/index.html.haml", - "line": 54, - "link": "https://brakemanscanner.org/docs/warning_types/dynamic_render_path/", - "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", - "rendered": { - "name": "admin/accounts/index", - "file": "app/views/admin/accounts/index.html.haml" - } - } - ], - "location": { - "type": "template", - "template": "admin/accounts/index" - }, - "user_input": "params[:page]", - "confidence": "Weak", - "note": "" - }, { "warning_type": "Redirect", "warning_code": 18, -- cgit