From 60300145d8a7fd89af25c8754e36d3151413c70d Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Tue, 19 Jun 2018 17:19:42 -0300 Subject: [PATCH 1/7] Enable rubocop Lint cops --- .rubocop.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index 4631b4a..b7575cc 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -13,6 +13,9 @@ Gemspec: Layout: Enabled: true +Lint: + Enabled: true + Performance: Enabled: true From 972a19006aced95efefd6c1ecf9dc8ebadb26198 Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Tue, 19 Jun 2018 17:25:36 -0300 Subject: [PATCH 2/7] Enable a couple of rubocop Style cops --- .rubocop.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index b7575cc..ed37697 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -25,6 +25,12 @@ Security: Lint: Enabled: true +Style/FrozenStringLiteralComment: + Enabled: true + +Style/RedundantFreeze: + Enabled: true + # TODO # Remove cop disabling and fix offenses Lint/HandleExceptions: From e6854bcb023169030cdbf3c6d6ad89e68dcdad3e Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Tue, 19 Jun 2018 17:36:21 -0300 Subject: [PATCH 3/7] Enable rubocop Naming cops --- .rubocop.yml | 5 +++++ lib/rack/attack.rb | 8 ++++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index ed37697..5ca4365 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -16,6 +16,11 @@ Layout: Lint: Enabled: true +Naming: + Enabled: true + Exclude: + - "lib/rack/attack/path_normalizer.rb" + Performance: Enabled: true diff --git a/lib/rack/attack.rb b/lib/rack/attack.rb index 1140f91..4a96778 100644 --- a/lib/rack/attack.rb +++ b/lib/rack/attack.rb @@ -38,15 +38,15 @@ class Rack::Attack self.blocklists[name] = Blocklist.new(name, block) end - def blocklist_ip(ip) + def blocklist_ip(ip_address) @ip_blocklists ||= [] - ip_blocklist_proc = lambda { |request| IPAddr.new(ip).include?(IPAddr.new(request.ip)) } + ip_blocklist_proc = lambda { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) } @ip_blocklists << Blocklist.new(nil, ip_blocklist_proc) end - def safelist_ip(ip) + def safelist_ip(ip_address) @ip_safelists ||= [] - ip_safelist_proc = lambda { |request| IPAddr.new(ip).include?(IPAddr.new(request.ip)) } + ip_safelist_proc = lambda { |request| IPAddr.new(ip_address).include?(IPAddr.new(request.ip)) } @ip_safelists << Safelist.new(nil, ip_safelist_proc) end From 1e9d601483791b90d8ef6a3c21b14c8bb31eda1c Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Tue, 19 Jun 2018 17:52:16 -0300 Subject: [PATCH 4/7] Run rubocop checks when running rake default task --- Rakefile | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Rakefile b/Rakefile index 52b39f2..961943b 100644 --- a/Rakefile +++ b/Rakefile @@ -2,6 +2,9 @@ require "rubygems" require "bundler/setup" require 'bundler/gem_tasks' require 'rake/testtask' +require "rubocop/rake_task" + +RuboCop::RakeTask.new namespace :test do Rake::TestTask.new(:units) do |t| @@ -20,4 +23,4 @@ end desc 'Run tests' task :test => %w[test:units test:integration test:acceptance] -task :default => :test +task :default => [:rubocop, :test] From 86eb9f9e0a6ed4a265a16946f56a7f452fc7c9f5 Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Tue, 19 Jun 2018 17:55:24 -0300 Subject: [PATCH 5/7] Enable Style/BracesAroundHashParameters rubocop cop --- .rubocop.yml | 3 +++ lib/rack/attack/store_proxy/redis_cache_store_proxy.rb | 4 ++-- spec/rack_attack_throttle_spec.rb | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 5ca4365..1d679e3 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -30,6 +30,9 @@ Security: Lint: Enabled: true +Style/BracesAroundHashParameters: + Enabled: true + Style/FrozenStringLiteralComment: Enabled: true diff --git a/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb b/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb index 319445e..8bb3f46 100644 --- a/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb +++ b/lib/rack/attack/store_proxy/redis_cache_store_proxy.rb @@ -18,11 +18,11 @@ module Rack end def read(name, options = {}) - super(name, options.merge!({ raw: true })) + super(name, options.merge!(raw: true)) end def write(name, value, options = {}) - super(name, value, options.merge!({ raw: true })) + super(name, value, options.merge!(raw: true)) end end end diff --git a/spec/rack_attack_throttle_spec.rb b/spec/rack_attack_throttle_spec.rb index 0361c7c..b32891a 100644 --- a/spec/rack_attack_throttle_spec.rb +++ b/spec/rack_attack_throttle_spec.rb @@ -37,7 +37,7 @@ describe 'Rack::Attack.throttle' do it 'should tag the env' do last_request.env['rack.attack.matched'].must_equal 'ip/sec' last_request.env['rack.attack.match_type'].must_equal :throttle - last_request.env['rack.attack.match_data'].must_equal({ :count => 2, :limit => 1, :period => @period }) + last_request.env['rack.attack.match_data'].must_equal(:count => 2, :limit => 1, :period => @period) last_request.env['rack.attack.match_discriminator'].must_equal('1.2.3.4') end From d5e585680fb04401150197192de0edaa070cb7b2 Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Wed, 20 Jun 2018 10:29:13 -0300 Subject: [PATCH 6/7] Fix CircleCI rubocop runs by excluding vendor/ folder rubocop default configuration exclude vendor/ folder, but in order to get the default we need to merge arrays, given that we are also setting Exclude --- .rubocop.yml | 4 ++++ rack-attack.gemspec | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.rubocop.yml b/.rubocop.yml index 1d679e3..84e396f 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,3 +1,7 @@ +inherit_mode: + merge: + - Exclude + AllCops: TargetRubyVersion: 2.2 DisabledByDefault: true diff --git a/rack-attack.gemspec b/rack-attack.gemspec index f63c35c..233e9ea 100644 --- a/rack-attack.gemspec +++ b/rack-attack.gemspec @@ -38,7 +38,7 @@ Gem::Specification.new do |s| s.add_development_dependency 'rack-test' s.add_development_dependency 'rake' s.add_development_dependency 'redis-activesupport' - s.add_development_dependency "rubocop", "0.55.0" + s.add_development_dependency "rubocop", "0.57.2" s.add_development_dependency "timecop" # Need to explicitly depend on guard because guard-minitest doesn't declare From 326ab8e098d453ff90157c002d975f6cd8578ad8 Mon Sep 17 00:00:00 2001 From: Gonzalo Rodriguez Date: Wed, 20 Jun 2018 14:25:49 -0300 Subject: [PATCH 7/7] Temporary fix for rubocop during CI builds --- .rubocop.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.rubocop.yml b/.rubocop.yml index 84e396f..cc0e049 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -7,6 +7,12 @@ AllCops: DisabledByDefault: true Exclude: - "examples/instrumentation.rb" + # Remove the following line once we are able to make bundler install gems to /vendor/bundle instead + # of /gemfiles/vendor/bundle during TravisCI builds. The reason that happens for now is because + # bundler 1.x only installs relative to the Gemfile (which during CI builds is always one inside gemfiles/ folder) + # instead of the CWD. Bundler 2.x will add support to install relative to CWD + # (see https://github.com/bundler/bundler/pull/5803). + - "gemfiles/vendor/**/*" Bundler: Enabled: true