From 69f0f48dc084c1ac4d5d441094b15cf01790ac98 Mon Sep 17 00:00:00 2001 From: Dan Ungureanu Date: Wed, 27 Oct 2021 11:39:28 +0300 Subject: [PATCH] DEV: Fix rubocop issues (#14715) --- app/models/locale_site_setting.rb | 2 +- app/models/topic_embed.rb | 10 ++---- app/services/site_settings_task.rb | 2 +- lib/cache.rb | 4 +-- lib/discourse_updates.rb | 22 ++++++++---- lib/i18n/locale_file_checker.rb | 2 +- lib/middleware/anonymous_cache.rb | 2 +- lib/onebox/engine/github_actions_onebox.rb | 2 +- lib/onebox/engine/json.rb | 2 +- lib/onebox/engine/pubmed_onebox.rb | 2 +- lib/onebox/mixins/git_blob_onebox.rb | 2 +- lib/onebox/status_check.rb | 2 +- lib/site_settings/type_supervisor.rb | 2 +- lib/tasks/cdn.rake | 2 +- lib/tasks/emoji.rake | 4 +-- lib/tasks/themes.rake | 2 +- script/bench.rb | 2 +- script/benchmarks/cache/bench.rb | 2 +- script/import_scripts/jforum.rb | 2 +- script/import_scripts/mbox/support/indexer.rb | 2 +- script/import_scripts/nodebb/nodebb.rb | 4 +-- script/import_scripts/smf2.rb | 2 +- script/import_scripts/zendesk_api.rb | 2 +- script/memstats.rb | 2 +- spec/components/discourse_updates_spec.rb | 16 +++++---- spec/models/topic_embed_spec.rb | 34 ++++++------------- spec/serializers/upload_serializer_spec.rb | 2 +- 27 files changed, 65 insertions(+), 69 deletions(-) diff --git a/app/models/locale_site_setting.rb b/app/models/locale_site_setting.rb index f541a27f488..68431871648 100644 --- a/app/models/locale_site_setting.rb +++ b/app/models/locale_site_setting.rb @@ -23,7 +23,7 @@ class LocaleSiteSetting < EnumSiteSetting @lock.synchronize do @language_names ||= begin - names = YAML.load(File.read(File.join(Rails.root, 'config', 'locales', 'names.yml'))) + names = YAML.safe_load(File.read(File.join(Rails.root, 'config', 'locales', 'names.yml'))) DiscoursePluginRegistry.locales.each do |locale, options| if !names.key?(locale) && options[:name] && options[:nativeName] diff --git a/app/models/topic_embed.rb b/app/models/topic_embed.rb index 502409d3c19..22739134d1e 100644 --- a/app/models/topic_embed.rb +++ b/app/models/topic_embed.rb @@ -117,8 +117,8 @@ class TopicEmbed < ActiveRecord::Base follow_canonical: true, ) - url = fd.resolve - return if url.blank? + uri = fd.resolve + return if uri.blank? opts = { tags: %w[div p code pre h1 h2 h3 b em i strong a img ul li ol blockquote], @@ -132,7 +132,7 @@ class TopicEmbed < ActiveRecord::Base response = FetchResponse.new begin - html = open(url, allow_redirections: :safe).read + html = uri.read(allow_redirections: :safe) rescue OpenURI::HTTPError, Net::OpenTimeout return end @@ -256,10 +256,6 @@ class TopicEmbed < ActiveRecord::Base body end end - - def self.open(uri, **kwargs) - URI.open(uri, **kwargs) - end end # == Schema Information diff --git a/app/services/site_settings_task.rb b/app/services/site_settings_task.rb index 64c80486ffe..356d8eccf14 100644 --- a/app/services/site_settings_task.rb +++ b/app/services/site_settings_task.rb @@ -16,7 +16,7 @@ class SiteSettingsTask counts = { updated: 0, not_found: 0, errors: 0 } log = [] - site_settings = YAML::load(yml) + site_settings = YAML::safe_load(yml) site_settings.each do |site_setting| key = site_setting[0] val = site_setting[1] diff --git a/lib/cache.rb b/lib/cache.rb index 7d69361915b..ca814c9e2bb 100644 --- a/lib/cache.rb +++ b/lib/cache.rb @@ -86,7 +86,7 @@ class Cache if raw begin - Marshal.load(raw) + Marshal.load(raw) # rubocop:disable Security/MarshalLoad rescue => e log_first_exception(e) end @@ -113,7 +113,7 @@ class Cache def read_entry(key) if data = redis.get(key) - Marshal.load(data) + Marshal.load(data) # rubocop:disable Security/MarshalLoad end rescue => e # corrupt cache, this can happen if Marshal version diff --git a/lib/discourse_updates.rb b/lib/discourse_updates.rb index fbc81aac9f7..32afc2f6d2d 100644 --- a/lib/discourse_updates.rb +++ b/lib/discourse_updates.rb @@ -61,18 +61,34 @@ module DiscourseUpdates Discourse.redis.get last_installed_version_key end + def last_installed_version=(arg) + Discourse.redis.set(last_installed_version, arg) + end + def latest_version Discourse.redis.get latest_version_key end + def latest_version=(arg) + Discourse.redis.set(latest_version, arg) + end + def missing_versions_count Discourse.redis.get(missing_versions_count_key).try(:to_i) end + def missing_versions_count=(arg) + Discourse.redis.set(missing_versions_count, arg) + end + def critical_updates_available? (Discourse.redis.get(critical_updates_available_key) || false) == 'true' end + def critical_updates_available=(arg) + Discourse.redis.set(critical_updates_available, arg) + end + def updated_at t = Discourse.redis.get(updated_at_key) t ? Time.zone.parse(t) : nil @@ -82,12 +98,6 @@ module DiscourseUpdates Discourse.redis.set updated_at_key, time_with_zone.as_json end - ['last_installed_version', 'latest_version', 'missing_versions_count', 'critical_updates_available'].each do |name| - eval "define_method :#{name}= do |arg| - Discourse.redis.set #{name}_key, arg - end" - end - def missing_versions=(versions) # delete previous list from redis prev_keys = Discourse.redis.lrange(missing_versions_list_key, 0, 4) diff --git a/lib/i18n/locale_file_checker.rb b/lib/i18n/locale_file_checker.rb index e818823fd8d..757e9b969a2 100644 --- a/lib/i18n/locale_file_checker.rb +++ b/lib/i18n/locale_file_checker.rb @@ -165,7 +165,7 @@ class LocaleFileChecker def plural_keys @plural_keys ||= begin - eval(File.read("#{Rails.root}/#{PLURALS_FILE}")).map do |locale, value| + eval(File.read("#{Rails.root}/#{PLURALS_FILE}")).map do |locale, value| # rubocop:disable Security/Eval [locale.to_s, value[:i18n][:plural][:keys].map(&:to_s)] end.to_h end diff --git a/lib/middleware/anonymous_cache.rb b/lib/middleware/anonymous_cache.rb index a62fb01836c..8d605ae537c 100644 --- a/lib/middleware/anonymous_cache.rb +++ b/lib/middleware/anonymous_cache.rb @@ -29,7 +29,7 @@ module Middleware method << "|#{k}=#\{h.#{v}}" end method << "\"\nend" - eval(method) + eval(method) # rubocop:disable Security/Eval @@compiled = true end diff --git a/lib/onebox/engine/github_actions_onebox.rb b/lib/onebox/engine/github_actions_onebox.rb index cdb7e16378f..182fff49d58 100644 --- a/lib/onebox/engine/github_actions_onebox.rb +++ b/lib/onebox/engine/github_actions_onebox.rb @@ -71,7 +71,7 @@ module Onebox raw["head_commit"]["message"].lines.first elsif type == :pr_run pr_url = "https://api.github.com/repos/#{match[:org]}/#{match[:repo]}/pulls/#{match[:pr_id]}" - ::MultiJson.load(URI.open(pr_url, read_timeout: timeout))["title"] + ::MultiJson.load(URI.parse(pr_url).open(read_timeout: timeout))["title"] end { diff --git a/lib/onebox/engine/json.rb b/lib/onebox/engine/json.rb index 261dc0309cf..204b09c720e 100644 --- a/lib/onebox/engine/json.rb +++ b/lib/onebox/engine/json.rb @@ -6,7 +6,7 @@ module Onebox private def raw - @raw ||= ::MultiJson.load(URI.open(url, read_timeout: timeout)) + @raw ||= ::MultiJson.load(URI.parse(url).open(read_timeout: timeout)) end end end diff --git a/lib/onebox/engine/pubmed_onebox.rb b/lib/onebox/engine/pubmed_onebox.rb index 1cf8a0ac9bb..366d5d50299 100644 --- a/lib/onebox/engine/pubmed_onebox.rb +++ b/lib/onebox/engine/pubmed_onebox.rb @@ -12,7 +12,7 @@ module Onebox def xml return @xml if defined?(@xml) - doc = Nokogiri::XML(URI.open(URI.join(@url, "?report=xml&format=text"))) + doc = Nokogiri::XML(URI.join(@url, "?report=xml&format=text").open) pre = doc.xpath("//pre") @xml = Nokogiri::XML("" + pre.text + "") end diff --git a/lib/onebox/mixins/git_blob_onebox.rb b/lib/onebox/mixins/git_blob_onebox.rb index cac7b2a7644..0c9b2bda128 100644 --- a/lib/onebox/mixins/git_blob_onebox.rb +++ b/lib/onebox/mixins/git_blob_onebox.rb @@ -167,7 +167,7 @@ module Onebox @model_file = @lang.dup @raw = "https://render.githubusercontent.com/view/solid?url=" + self.raw_template(m) else - contents = URI.open(self.raw_template(m), read_timeout: timeout).read + contents = URI.parse(self.raw_template(m)).open(read_timeout: timeout).read contents_lines = contents.lines #get contents lines contents_lines_size = contents_lines.size #get number of lines diff --git a/lib/onebox/status_check.rb b/lib/onebox/status_check.rb index 73b3bca24e6..0980b8d1355 100644 --- a/lib/onebox/status_check.rb +++ b/lib/onebox/status_check.rb @@ -35,7 +35,7 @@ module Onebox private def check - res = URI.open(@url, read_timeout: (@options.timeout || Onebox.options.timeout)) + res = URI.parse(@url).open(read_timeout: (@options.timeout || Onebox.options.timeout)) @status = res.status.first.to_i rescue OpenURI::HTTPError => e @status = e.io.status.first.to_i diff --git a/lib/site_settings/type_supervisor.rb b/lib/site_settings/type_supervisor.rb index 99170c3dc0d..428aadfe4c3 100644 --- a/lib/site_settings/type_supervisor.rb +++ b/lib/site_settings/type_supervisor.rb @@ -92,7 +92,7 @@ class SiteSettings::TypeSupervisor end if (new_choices = opts[:choices]) - new_choices = eval(new_choices) if new_choices.is_a?(String) + new_choices = eval(new_choices) if new_choices.is_a?(String) # rubocop:disable Security/Eval if @choices.has_key?(name) @choices[name].concat(new_choices) diff --git a/lib/tasks/cdn.rake b/lib/tasks/cdn.rake index 438e1020fce..b9c6c27501e 100644 --- a/lib/tasks/cdn.rake +++ b/lib/tasks/cdn.rake @@ -20,7 +20,7 @@ task 'assets:prestage' => :environment do |t| puts "pre staging: #{assets.join(' ')}" # makes testing simpler leaving this here - config = YAML::load(File.open("#{Rails.root}/config/cdn.yml")) + config = YAML::safe_load(File.open("#{Rails.root}/config/cdn.yml")) start = Time.now diff --git a/lib/tasks/emoji.rake b/lib/tasks/emoji.rake index 18ec5fc034a..3cb7f5ff3ad 100644 --- a/lib/tasks/emoji.rake +++ b/lib/tasks/emoji.rake @@ -284,7 +284,7 @@ desc "update emoji images" task "emoji:update" do copy_emoji_db - json_db = open(File.join(GENERATED_PATH, "db.json")).read + json_db = File.read(File.join(GENERATED_PATH, "db.json")) db = JSON.parse(json_db) write_db_json(db["emojis"], db["translations"]) @@ -352,7 +352,7 @@ end def generate_emoji_groups(keywords, sections) puts "Generating groups..." - list = open(EMOJI_ORDERING_URL).read + list = URI.parse(EMOJI_ORDERING_URL).read doc = Nokogiri::HTML5(list) table = doc.css("table")[0] diff --git a/lib/tasks/themes.rake b/lib/tasks/themes.rake index 1e2fdfacc03..cd9eda143f1 100644 --- a/lib/tasks/themes.rake +++ b/lib/tasks/themes.rake @@ -30,7 +30,7 @@ task "themes:install" => :environment do |task, args| use_json = theme_args == '' theme_args = begin - use_json ? JSON.parse(ARGV.last.gsub('--', '')) : YAML::load(theme_args) + use_json ? JSON.parse(ARGV.last.gsub('--', '')) : YAML::safe_load(theme_args) rescue puts use_json ? "Invalid JSON input. \n#{ARGV.last}" : "Invalid YML: \n#{theme_args}" exit 1 diff --git a/script/bench.rb b/script/bench.rb index ce88f408b4b..d1ab78d791a 100644 --- a/script/bench.rb +++ b/script/bench.rb @@ -297,7 +297,7 @@ begin run("RAILS_ENV=profile bundle exec rake assets:clean") def get_mem(pid) - YAML.load `ruby script/memstats.rb #{pid} --yaml` + YAML.safe_load `ruby script/memstats.rb #{pid} --yaml` end mem = get_mem(pid) diff --git a/script/benchmarks/cache/bench.rb b/script/benchmarks/cache/bench.rb index ff6092e8595..fad1b73607a 100644 --- a/script/benchmarks/cache/bench.rb +++ b/script/benchmarks/cache/bench.rb @@ -46,7 +46,7 @@ Benchmark.ips do |x| x.report("redis get string marshal") do |times| while times > 0 - Marshal.load(Discourse.redis.get("test_keym")) + Marshal.load(Discourse.redis.get("test_keym")) # rubocop:disable Security/MarshalLoad times -= 1 end end diff --git a/script/import_scripts/jforum.rb b/script/import_scripts/jforum.rb index e5d56718b62..1c1136f2084 100644 --- a/script/import_scripts/jforum.rb +++ b/script/import_scripts/jforum.rb @@ -10,7 +10,7 @@ class ImportScripts::JForum < ImportScripts::Base def initialize super - @settings = YAML.load(File.read(ARGV.first), symbolize_names: true) + @settings = YAML.safe_load(File.read(ARGV.first), symbolize_names: true) @database_client = Mysql2::Client.new( host: @settings[:database][:host], diff --git a/script/import_scripts/mbox/support/indexer.rb b/script/import_scripts/mbox/support/indexer.rb index 47ce3ed1334..c8164534d77 100644 --- a/script/import_scripts/mbox/support/indexer.rb +++ b/script/import_scripts/mbox/support/indexer.rb @@ -48,7 +48,7 @@ module ImportScripts::Mbox if File.exist?(metadata_file) # workaround for YML files that contain classname in file header yaml = File.read(metadata_file).sub(/^--- !.*$/, '---') - metadata = YAML.load(yaml) + metadata = YAML.safe_load(yaml) else metadata = {} end diff --git a/script/import_scripts/nodebb/nodebb.rb b/script/import_scripts/nodebb/nodebb.rb index 45fe69c7c8c..5fed857301d 100644 --- a/script/import_scripts/nodebb/nodebb.rb +++ b/script/import_scripts/nodebb/nodebb.rb @@ -180,7 +180,7 @@ class ImportScripts::NodeBB < ImportScripts::Base if is_external # download external image begin - string_io = open(picture, read_timeout: 5) + string_io = uri.open(read_timeout: 5) rescue Net::ReadTimeout puts "timeout downloading avatar for user #{imported_user.id}" return nil @@ -246,7 +246,7 @@ class ImportScripts::NodeBB < ImportScripts::Base if is_external begin - string_io = open(picture, read_timeout: 5) + string_io = uri.open(read_timeout: 5) rescue Net::ReadTimeout return nil end diff --git a/script/import_scripts/smf2.rb b/script/import_scripts/smf2.rb index 74b63189cd8..9885f1edc32 100644 --- a/script/import_scripts/smf2.rb +++ b/script/import_scripts/smf2.rb @@ -558,7 +558,7 @@ class ImportScripts::Smf2 < ImportScripts::Base def read_smf_settings settings = File.join(self.smfroot, 'Settings.php') - IO.readlines(settings).each do |line| + File.readlines(settings).each do |line| next unless m = /\$([a-z_]+)\s*=\s*['"](.+?)['"]\s*;\s*((#|\/\/).*)?$/.match(line) case m[1] when 'db_server' then self.host ||= m[2] diff --git a/script/import_scripts/zendesk_api.rb b/script/import_scripts/zendesk_api.rb index 4b5a2b56801..9237a764423 100644 --- a/script/import_scripts/zendesk_api.rb +++ b/script/import_scripts/zendesk_api.rb @@ -333,7 +333,7 @@ class ImportScripts::ZendeskApi < ImportScripts::Base attempts = 0 begin - open("#{$1}") do |image| + URI.parse(image_url).open do |image| # IMAGE_DOWNLOAD_PATH is whatever image, it will be replaced with the downloaded image File.open(IMAGE_DOWNLOAD_PATH, "wb") do |file| file.write(image.read) diff --git a/script/memstats.rb b/script/memstats.rb index f387a883370..998d8d525f7 100755 --- a/script/memstats.rb +++ b/script/memstats.rb @@ -131,7 +131,7 @@ def format_number(n) end def get_commandline(pid) - commandline = IO.read("/proc/#{pid}/cmdline").split("\0") + commandline = File.read("/proc/#{pid}/cmdline").split("\0") if commandline.first =~ /java$/ then loop { break if commandline.shift == "-jar" } return "[java] #{commandline.shift}" diff --git a/spec/components/discourse_updates_spec.rb b/spec/components/discourse_updates_spec.rb index 6bb8fd27400..8cb95dd5fdf 100644 --- a/spec/components/discourse_updates_spec.rb +++ b/spec/components/discourse_updates_spec.rb @@ -3,12 +3,16 @@ require 'rails_helper' describe DiscourseUpdates do + let(:latest_version_key) { DiscourseUpdates.send(:latest_version_key) } + let(:missing_versions_count_key) { DiscourseUpdates.send(:missing_versions_count_key) } + let(:critical_updates_available_key) { DiscourseUpdates.send(:critical_updates_available_key) } + let(:updated_at_key) { DiscourseUpdates.send(:updated_at_key) } def stub_data(latest, missing, critical, updated_at) - DiscourseUpdates.stubs(:latest_version).returns(latest) - DiscourseUpdates.stubs(:missing_versions_count).returns(missing) - DiscourseUpdates.stubs(:critical_updates_available?).returns(critical) - DiscourseUpdates.stubs(:updated_at).returns(updated_at) + Discourse.redis.set(latest_version_key, latest) + Discourse.redis.set(missing_versions_count_key, missing) + Discourse.redis.set(critical_updates_available_key, critical) + Discourse.redis.set(updated_at_key, updated_at) end before do @@ -36,7 +40,7 @@ describe DiscourseUpdates do end it 'returns the timestamp of the last version check' do - expect(subject.updated_at).to eq_time(time) + expect(subject.updated_at).to be_within_one_second_of(time) end end @@ -52,7 +56,7 @@ describe DiscourseUpdates do end it 'returns the timestamp of the last version check' do - expect(subject.updated_at).to eq_time(time) + expect(subject.updated_at).to be_within_one_second_of(time) end end end diff --git a/spec/models/topic_embed_spec.rb b/spec/models/topic_embed_spec.rb index 8659eb7d569..4ce37dc37a1 100644 --- a/spec/models/topic_embed_spec.rb +++ b/spec/models/topic_embed_spec.rb @@ -191,19 +191,13 @@ describe TopicEmbed do describe '.find_remote' do fab!(:embeddable_host) { Fabricate(:embeddable_host) } - let(:file) { StringIO.new } - - before do - TopicEmbed.stubs(:open).returns file - end context ".title_scrub" do let(:url) { 'http://eviltrout.com/123' } let(:contents) { "Through the Looking Glass - Classic Bookssome content here" } before do - file.stubs(:read).returns contents - stub_request(:get, url) + stub_request(:get, url).to_return(status: 200, body: contents) end it "doesn't scrub the title by default" do @@ -225,8 +219,7 @@ describe TopicEmbed do before do SiteSetting.allowed_embed_classnames = 'emoji, foo' - file.stubs(:read).returns contents - stub_request(:get, url) + stub_request(:get, url).to_return(status: 200, body: contents) @response = TopicEmbed.find_remote(url) end @@ -257,9 +250,7 @@ describe TopicEmbed do let(:contents) { 'rich and morty' } before(:each) do - file.stubs(:read).returns contents - TopicEmbed.stubs(:open).returns file - stub_request(:get, url) + stub_request(:get, url).to_return(status: 200, body: contents) end it "has no author tag" do @@ -276,8 +267,7 @@ describe TopicEmbed do before(:each) do SiteSetting.allowed_embed_classnames = '' - file.stubs(:read).returns contents - stub_request(:get, url) + stub_request(:get, url).to_return(status: 200, body: contents) @response = TopicEmbed.find_remote(url) end @@ -303,8 +293,7 @@ describe TopicEmbed do let(:contents) { "سلاماین یک پاراگراف آزمون است." } before do - stub_request(:get, url) - file.stubs(:read).returns contents + stub_request(:get, url).to_return(status: 200, body: contents) end it "doesn't throw an error" do @@ -318,8 +307,7 @@ describe TopicEmbed do let(:contents) { "Hello World!" } before do - stub_request(:get, url) - file.stubs(:read).returns contents + stub_request(:get, url).to_return(status: 200, body: contents) end it "doesn't throw an error" do @@ -341,8 +329,7 @@ describe TopicEmbed do let(:contents) { '

URL encoded @ symbol

normal mailto link

' } before do - file.stubs(:read).returns contents - stub_request(:get, url) + stub_request(:get, url).to_return(status: 200, body: contents) end it "handles mailto links" do @@ -358,8 +345,7 @@ describe TopicEmbed do let(:contents) { '

Baz

' } before do - file.stubs(:read).returns contents - stub_request(:get, url) + stub_request(:get, url).to_return(status: 200, body: contents) end it "doesn’t raise an exception" do @@ -374,9 +360,9 @@ describe TopicEmbed do let(:canonical_content) { "Canonical" } before do - file.stubs(:read).returns canonical_content - stub_request(:get, url) + stub_request(:get, url).to_return(status: 200, body: content) stub_request(:head, canonical_url) + stub_request(:get, canonical_url).to_return(status: 200, body: canonical_content) end it 'a' do diff --git a/spec/serializers/upload_serializer_spec.rb b/spec/serializers/upload_serializer_spec.rb index c2a1cf6e048..01be44b5521 100644 --- a/spec/serializers/upload_serializer_spec.rb +++ b/spec/serializers/upload_serializer_spec.rb @@ -7,7 +7,7 @@ RSpec.describe UploadSerializer do let(:subject) { UploadSerializer.new(upload, root: false) } it 'should render without errors' do - json_data = JSON.load(subject.to_json) + json_data = JSON.parse(subject.to_json) expect(json_data['id']).to eql upload.id expect(json_data['width']).to eql upload.width