Skip to content

Commit

Permalink
DEV: Prefer \A and \z over ^ and $ in regexes (discourse#19936)
Browse files Browse the repository at this point in the history
  • Loading branch information
danielwaterworth authored Jan 20, 2023
1 parent f7907a3 commit 666536c
Show file tree
Hide file tree
Showing 115 changed files with 294 additions and 291 deletions.
4 changes: 2 additions & 2 deletions app/controllers/admin/backups_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -238,11 +238,11 @@ def ensure_backups_enabled
end

def valid_extension?(filename)
/\.(tar\.gz|t?gz)$/i =~ filename
/\.(tar\.gz|t?gz)\z/i =~ filename
end

def valid_filename?(filename)
!!(/^[a-zA-Z0-9\._\-]+$/ =~ filename)
!!(/\A[a-zA-Z0-9\._\-]+\z/ =~ filename)
end

def render_error(message_key)
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/admin/reports_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ def index
ApplicationRequest
.req_types
.keys
.select { |r| r =~ /^page_view_/ && r !~ /mobile/ }
.select { |r| r =~ /\Apage_view_/ && r !~ /mobile/ }
.map { |r| r + "_reqs" } +
Report.singleton_methods.grep(/^report_(?!about|storage_stats)/)
Report.singleton_methods.grep(/\Areport_(?!about|storage_stats)/)

reports =
reports_methods.map do |name|
Expand Down Expand Up @@ -61,7 +61,7 @@ def bulk
def show
report_type = params[:type]

raise Discourse::NotFound unless report_type =~ /^[a-z0-9\_]+$/
raise Discourse::NotFound unless report_type =~ /\A[a-z0-9\_]+\z/

args = parse_params(params)

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/admin/site_texts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ def record_for(key:, value: nil, locale:)
{ id: key, value: value, locale: locale }
end

PLURALIZED_REGEX = /(.*)\.(zero|one|two|few|many|other)$/
PLURALIZED_REGEX = /(.*)\.(zero|one|two|few|many|other)\z/

def find_site_text(locale)
if self.class.restricted_keys.include?(params[:id])
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/admin/themes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def import
render json: @theme, status: :created
rescue RemoteTheme::ImportError => e
if params[:force]
theme_name = params[:remote].gsub(/.git$/, "").split("/").last
theme_name = params[:remote].gsub(/.git\z/, "").split("/").last

remote_theme = RemoteTheme.new
remote_theme.private_key = private_key
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ def custom_html_json

DiscoursePluginRegistry.html_builders.each do |name, _|
if name.start_with?("client:")
data[name.sub(/^client:/, "")] = DiscoursePluginRegistry.build_html(name, self)
data[name.sub(/\Aclient:/, "")] = DiscoursePluginRegistry.build_html(name, self)
end
end

Expand Down
6 changes: 3 additions & 3 deletions app/controllers/embed_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ def topics
end

if @embed_id = params[:discourse_embed_id]
raise Discourse::InvalidParameters.new(:embed_id) unless @embed_id =~ /^de\-[a-zA-Z0-9]+$/
raise Discourse::InvalidParameters.new(:embed_id) unless @embed_id =~ /\Ade\-[a-zA-Z0-9]+\z/
end

if @embed_class = params[:embed_class]
unless @embed_class =~ /^[a-zA-Z0-9\-_]+$/
unless @embed_class =~ /\A[a-zA-Z0-9\-_]+\z/
raise Discourse::InvalidParameters.new(:embed_class)
end
end
Expand Down Expand Up @@ -139,7 +139,7 @@ def count
by_url = {}

if embed_urls.present?
urls = embed_urls.map { |u| u.sub(/#discourse-comments$/, "").sub(%r{/$}, "") }
urls = embed_urls.map { |u| u.sub(/#discourse-comments\z/, "").sub(%r{/\z}, "") }
topic_embeds = TopicEmbed.where(embed_url: urls).includes(:topic).references(:topic)

topic_embeds.each do |te|
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/extra_locales_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,6 @@ def self.clear_cache!
private

def valid_bundle?(bundle)
bundle == OVERRIDES_BUNDLE || (bundle =~ /^(admin|wizard)$/ && current_user&.staff?)
bundle == OVERRIDES_BUNDLE || (bundle =~ /\A(admin|wizard)\z/ && current_user&.staff?)
end
end
2 changes: 1 addition & 1 deletion app/controllers/session_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ def sso_login
end

# If it's not a relative URL check the host
if return_path !~ %r{^/[^/]}
if return_path !~ %r{\A/[^/]}
begin
uri = URI(return_path)
if (uri.hostname == Discourse.current_hostname)
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/theme_javascripts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def show_map

def show_tests
digest = params[:digest]
raise Discourse::NotFound if !digest.match?(/^\h{40}$/)
raise Discourse::NotFound if !digest.match?(/\A\h{40}\z/)

theme = Theme.find_by(id: params[:theme_id])
raise Discourse::NotFound if theme.blank?
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/topics_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def show

# Special case: a slug with a number in front should look by slug first before looking
# up that particular number
if params[:id] && params[:id] =~ /^\d+[^\d\\]+$/
if params[:id] && params[:id] =~ /\A\d+[^\d\\]+\z/
topic = Topic.find_by_slug(params[:id])
return redirect_to_correct_topic(topic, opts[:post_number]) if topic
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/user_avatars_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def refresh_gravatar
def show_proxy_letter
is_asset_path

if SiteSetting.external_system_avatars_url !~ %r{^/letter_avatar_proxy}
if SiteSetting.external_system_avatars_url !~ %r{\A/letter_avatar_proxy}
raise Discourse::NotFound
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ def preferences
end

def my_redirect
raise Discourse::NotFound if params[:path] !~ %r{^[a-z_\-/]+$}
raise Discourse::NotFound if params[:path] !~ %r{\A[a-z_\-/]+\z}

if current_user.blank?
cookies[:destination_url] = path("/my/#{params[:path]}")
Expand Down
6 changes: 3 additions & 3 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def discourse_config_environment(testing: false)

def google_universal_analytics_json(ua_domain_name = nil)
result = {}
result[:cookieDomain] = ua_domain_name.gsub(%r{^http(s)?://}, "") if ua_domain_name
result[:cookieDomain] = ua_domain_name.gsub(%r{\Ahttp(s)?://}, "") if ua_domain_name
result[:userId] = current_user.id if current_user.present?
result[:allowLinker] = true if SiteSetting.ga_universal_auto_link_domains.present?
result.to_json
Expand Down Expand Up @@ -117,9 +117,9 @@ def script_asset_path(script)
# seconds.
if !script.start_with?("discourse/tests/")
if is_brotli_req?
path = path.gsub(/\.([^.]+)$/, '.br.\1')
path = path.gsub(/\.([^.]+)\z/, '.br.\1')
elsif is_gzip_req?
path = path.gsub(/\.([^.]+)$/, '.gz.\1')
path = path.gsub(/\.([^.]+)\z/, '.gz.\1')
end
end
elsif GlobalSetting.cdn_url&.start_with?("https") && is_brotli_req? &&
Expand Down
4 changes: 2 additions & 2 deletions app/helpers/user_notifications_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ def correct_top_margin(html, desired)

def logo_url
logo_url = SiteSetting.site_digest_logo_url
logo_url = SiteSetting.site_logo_url if logo_url.blank? || logo_url =~ /\.svg$/i
return nil if logo_url.blank? || logo_url =~ /\.svg$/i
logo_url = SiteSetting.site_logo_url if logo_url.blank? || logo_url =~ /\.svg\z/i
return nil if logo_url.blank? || logo_url =~ /\.svg\z/i
logo_url
end

Expand Down
2 changes: 1 addition & 1 deletion app/jobs/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def self.last_job_performed_at
end

def self.num_email_retry_jobs
Sidekiq::RetrySet.new.count { |job| job.klass =~ /Email$/ }
Sidekiq::RetrySet.new.count { |job| job.klass =~ /Email\z/ }
end

class Base
Expand Down
2 changes: 1 addition & 1 deletion app/jobs/onceoff/onceoff.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ class Jobs::Onceoff < ::Jobs::Base
sidekiq_options retry: false

def self.name_for(klass)
klass.name.sub(/^Jobs\:\:/, "")
klass.name.sub(/\AJobs\:\:/, "")
end

def running_key_name
Expand Down
4 changes: 2 additions & 2 deletions app/jobs/regular/update_username.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ def execute(args)
@raw_quote_regex = /(\[quote\s*=\s*["'']?)#{@old_username}(\,?[^\]]*\])/i

cooked_username = PrettyText::Helpers.format_username(@old_username)
@cooked_mention_username_regex = /^@#{cooked_username}$/i
@cooked_mention_username_regex = /\A@#{cooked_username}\z/i
@cooked_mention_user_path_regex =
%r{^/u(?:sers)?/#{UrlHelper.encode_component(cooked_username)}$}i
%r{\A/u(?:sers)?/#{UrlHelper.encode_component(cooked_username)}\z}i
@cooked_quote_username_regex = /(?<=\s)#{cooked_username}(?=:)/i

update_posts
Expand Down
2 changes: 1 addition & 1 deletion app/models/admin_dashboard_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ def failing_emails_check
end

def subfolder_ends_in_slash_check
I18n.t("dashboard.subfolder_ends_in_slash") if Discourse.base_path =~ %r{/$}
I18n.t("dashboard.subfolder_ends_in_slash") if Discourse.base_path =~ %r{/\z}
end

def email_polling_errored_recently
Expand Down
4 changes: 2 additions & 2 deletions app/models/category.rb
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,7 @@ def ensure_slug
end

# only allow to use category itself id.
match_id = /^(\d+)-category/.match(self.slug)
match_id = /\A(\d+)-category/.match(self.slug)
if match_id.present?
errors.add(:slug, :invalid) if new_record? || (match_id[1] != self.id.to_s)
end
Expand Down Expand Up @@ -897,7 +897,7 @@ def self.find_by_slug_path(slug_path)
slug_path.inject(nil) do |parent_id, slug|
category = Category.where(slug: slug, parent_category_id: parent_id)

if match_id = /^(\d+)-category/.match(slug).presence
if match_id = /\A(\d+)-category/.match(slug).presence
category = category.or(Category.where(id: match_id[1], parent_category_id: parent_id))
end

Expand Down
2 changes: 1 addition & 1 deletion app/models/concerns/has_custom_fields.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def self.get_custom_field_type(types, key)

sorted_types = types.keys.select { |k| k.end_with?("*") }.sort_by(&:length).reverse

sorted_types.each { |t| return types[t] if key =~ /^#{t}/i }
sorted_types.each { |t| return types[t] if key =~ /\A#{t}/i }

types[key]
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/concerns/reports/top_uploads.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ def report_top_uploads(report)
builder.where("up.created_at < :end_date", end_date: report.end_date)

if extension_filter
builder.where("up.extension = :extension", extension: extension_filter.sub(/^\./, ""))
builder.where("up.extension = :extension", extension: extension_filter.sub(/\A\./, ""))
end

builder.query.each do |row|
Expand Down
4 changes: 2 additions & 2 deletions app/models/embeddable_host.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ class EmbeddableHost < ActiveRecord::Base
after_destroy :reset_embedding_settings

before_validation do
self.host.sub!(%r{^https?://}, "")
self.host.sub!(%r{/.*$}, "")
self.host.sub!(%r{\Ahttps?://}, "")
self.host.sub!(%r{/.*\z}, "")
end

# TODO(2021-07-23): Remove
Expand Down
2 changes: 1 addition & 1 deletion app/models/emoji.rb
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def self.load_custom
emojis.each do |name, url|
result << Emoji.new.tap do |e|
e.name = name
url = (Discourse.base_path + url) if url[%r{^/[^/]}]
url = (Discourse.base_path + url) if url[%r{\A/[^/]}]
e.url = url
e.group = group || DEFAULT_GROUP
end
Expand Down
8 changes: 4 additions & 4 deletions app/models/global_setting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ def self.register(key, default)
define_singleton_method(key) { provider.lookup(key, default) }
end

VALID_SECRET_KEY ||= /^[0-9a-f]{128}$/
VALID_SECRET_KEY ||= /\A[0-9a-f]{128}\z/
# this is named SECRET_TOKEN as opposed to SECRET_KEY_BASE
# for legacy reasons
REDIS_SECRET_KEY ||= "SECRET_TOKEN"
Expand Down Expand Up @@ -251,7 +251,7 @@ def self.add_default(name, default)
class BaseProvider
def self.coerce(setting)
return setting == "true" if setting == "true" || setting == "false"
return $1.to_i if setting.to_s.strip =~ /^([0-9]+)$/
return $1.to_i if setting.to_s.strip =~ /\A([0-9]+)\z/
setting
end

Expand Down Expand Up @@ -283,7 +283,7 @@ def read
.result()
.split("\n")
.each do |line|
if line =~ /^\s*([a-z_]+[a-z0-9_]*)\s*=\s*(\"([^\"]*)\"|\'([^\']*)\'|[^#]*)/
if line =~ /\A\s*([a-z_]+[a-z0-9_]*)\s*=\s*(\"([^\"]*)\"|\'([^\']*)\'|[^#]*)/
@data[$1.strip.to_sym] = ($4 || $3 || $2).strip
end
end
Expand Down Expand Up @@ -314,7 +314,7 @@ def lookup(key, default)
end

def keys
ENV.keys.select { |k| k =~ /^DISCOURSE_/ }.map { |k| k[10..-1].downcase.to_sym }
ENV.keys.select { |k| k =~ /\ADISCOURSE_/ }.map { |k| k[10..-1].downcase.to_sym }
end
end

Expand Down
6 changes: 3 additions & 3 deletions app/models/group.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,7 @@ def email_username_regex
user = email_username_user
domain = email_username_domain
if user.present? && domain.present?
/^#{Regexp.escape(user)}(\+[^@]*)?@#{Regexp.escape(domain)}$/i
/\A#{Regexp.escape(user)}(\+[^@]*)?@#{Regexp.escape(domain)}\z/i
end
end

Expand Down Expand Up @@ -1160,8 +1160,8 @@ def self.get_valid_email_domains(value)
value
.split("|")
.each do |domain|
domain.sub!(%r{^https?://}, "")
domain.sub!(%r{/.*$}, "")
domain.sub!(%r{\Ahttps?://}, "")
domain.sub!(%r{/.*\z}, "")

if domain =~ Group::VALID_DOMAIN_REGEX
valid_domains << domain
Expand Down
4 changes: 2 additions & 2 deletions app/models/optimized_image.rb
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def destroy
end

def local?
!(url =~ %r{^(https?:)?//})
!(url =~ %r{\A(https?:)?//})
end

def calculate_filesize
Expand Down Expand Up @@ -337,7 +337,7 @@ def self.convert_with(instructions, to, opts = {})
else
error = +"Failed to optimize image:"

if e.message =~ /^convert:([^`]+)/
if e.message =~ /\Aconvert:([^`]+)/
error << $1
else
error << " unknown reason"
Expand Down
4 changes: 2 additions & 2 deletions app/models/post_action_type.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ class PostActionType < ActiveRecord::Base
include AnonCacheInvalidator

def expire_cache
ApplicationSerializer.expire_cache_fragment!(/^post_action_types_/)
ApplicationSerializer.expire_cache_fragment!(/^post_action_flag_types_/)
ApplicationSerializer.expire_cache_fragment!(/\Apost_action_types_/)
ApplicationSerializer.expire_cache_fragment!(/\Apost_action_flag_types_/)
end

class << self
Expand Down
2 changes: 1 addition & 1 deletion app/models/published_page.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class PublishedPage < ActiveRecord::Base

validate :slug_format
def slug_format
if slug !~ /^[a-zA-Z\-\_0-9]+$/
if slug !~ /\A[a-zA-Z\-\_0-9]+\z/
errors.add(:slug, I18n.t("publish_page.slug_errors.invalid"))
elsif %w[check-slug by-topic].include?(slug)
errors.add(:slug, I18n.t("publish_page.slug_errors.unavailable"))
Expand Down
6 changes: 3 additions & 3 deletions app/models/remote_theme.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ class ImportError < StandardError

ALLOWED_FIELDS = %w[scss embedded_scss head_tag header after_header body_tag footer]

GITHUB_REGEXP = %r{^https?://github\.com/}
GITHUB_SSH_REGEXP = %r{^ssh://git@github\.com:}
GITHUB_REGEXP = %r{\Ahttps?://github\.com/}
GITHUB_SSH_REGEXP = %r{\Assh://git@github\.com:}

has_one :theme, autosave: false
scope :joined_remotes,
Expand Down Expand Up @@ -329,7 +329,7 @@ def update_theme_color_schemes(theme, schemes)

def github_diff_link
if github_repo_url.present? && local_version != remote_version
"#{github_repo_url.gsub(/\.git$/, "")}/compare/#{local_version}...#{remote_version}"
"#{github_repo_url.gsub(/\.git\z/, "")}/compare/#{local_version}...#{remote_version}"
end
end

Expand Down
4 changes: 2 additions & 2 deletions app/models/report.rb
Original file line number Diff line number Diff line change
Expand Up @@ -268,8 +268,8 @@ def self.find(type, opts = nil)
wrap_slow_query do
if respond_to?(report_method)
public_send(report_method, report)
elsif type =~ /_reqs$/
req_report(report, type.split(/_reqs$/)[0].to_sym)
elsif type =~ /_reqs\z/
req_report(report, type.split(/_reqs\z/)[0].to_sym)
else
return nil
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/reviewable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def self.default_visible
end

def self.valid_type?(type)
return false unless type =~ /^Reviewable[A-Za-z]+$/
return false unless type =~ /\AReviewable[A-Za-z]+\z/
type.constantize <= Reviewable
rescue NameError
false
Expand Down
Loading

0 comments on commit 666536c

Please sign in to comment.