diff --git a/app/models/gem_typo.rb b/app/models/gem_typo.rb index 0b266056003..d0585e286cc 100644 --- a/app/models/gem_typo.rb +++ b/app/models/gem_typo.rb @@ -1,46 +1,42 @@ -require 'rubygems/text' +require "rubygems/text" class GemTypo - PROTECTED_GEMS = [ - 'rspec-core', - 'diff-lcs', - 'rspec-expectations', - 'rspec-mocks', - 'rspec', - 'bundler', - 'rspec-support', - 'multi_json', - 'rack', - 'rake' - ].freeze - - DISTANCE_THRESHOLD = 1 - - GEM_EXCEPTIONS = [ - 'rspec-coreZ' - # Add exceptions here to manage gems which share a close distance, - # but are manually reviewed and accepted by rubygems team - ].freeze + attr_reader :protected_gem include Gem::Text - def initialize(rubygem_name, opts = {}) - @rubygem_name = rubygem_name - @protected_gems = opts[:protected_gems] || GemTypo::PROTECTED_GEMS - @distance_threshold = opts[:distance_threshold] || GemTypo::DISTANCE_THRESHOLD - @gem_exceptions = opts[:gem_exceptions] || GemTypo::GEM_EXCEPTIONS + DOWNLOADS_THRESHOLD = 10_000_000 + SIZE_THRESHOLD = 4 + + def initialize(rubygem_name) + @rubygem_name = rubygem_name.downcase + @distance_threshold = distance_threshold end def protected_typo? - @protected_gems.each do |protected_gem| - return false if @rubygem_name == protected_gem + return false if @rubygem_name.size < GemTypo::SIZE_THRESHOLD + + protected_gems.each do |protected_gem| distance = levenshtein_distance(@rubygem_name, protected_gem) - if distance <= @distance_threshold && - !@gem_exceptions.include?(@rubygem_name) + if distance <= @distance_threshold + @protected_gem = protected_gem return true end end false end + + private + + def distance_threshold + @rubygem_name.size == GemTypo::SIZE_THRESHOLD ? 1 : 2 + end + + def protected_gems + Rubygem.joins(:gem_download) + .where("gem_downloads.count > ?", GemTypo::DOWNLOADS_THRESHOLD) + .where.not(name: @rubygem_name) + .pluck(:name) + end end diff --git a/app/models/rubygem.rb b/app/models/rubygem.rb index 21ddee42a6e..49f6b9ddce5 100644 --- a/app/models/rubygem.rb +++ b/app/models/rubygem.rb @@ -18,7 +18,7 @@ class Rubygem < ApplicationRecord uniqueness: { case_sensitive: false }, if: :needs_name_validation? validate :blacklist_names_exclusion - validate :protected_gem_typo_protection + validate :protected_gem_typo, on: :create after_create :update_unresolved before_destroy :mark_unresolved @@ -309,10 +309,11 @@ def blacklist_names_exclusion errors.add :name, "'#{name}' is a reserved gem name." end - def protected_gem_typo_protection + def protected_gem_typo gem_typo = GemTypo.new(name) + return unless gem_typo.protected_typo? - errors.add :name, "'#{name}' is too close to a typo-protected gem." + errors.add :name, "'#{name}' is too close to typo-protected gem: #{gem_typo.protected_gem}" end def update_unresolved diff --git a/test/functional/api/v1/rubygems_controller_test.rb b/test/functional/api/v1/rubygems_controller_test.rb index 0a9f3076502..2a2108fb394 100644 --- a/test/functional/api/v1/rubygems_controller_test.rb +++ b/test/functional/api/v1/rubygems_controller_test.rb @@ -308,6 +308,20 @@ def self.should_respond_to(format) end end + context "On POST to create with a protected gem name" do + setup do + above_downloads_thres = GemTypo::DOWNLOADS_THRESHOLD + 1 + create(:rubygem, name: "best", downloads: above_downloads_thres) + post :create, body: gem_file("test-1.0.0.gem").read + end + + should respond_with :forbidden + should "not register new gem" do + assert_equal 1, Rubygem.count + assert_equal "There was a problem saving your gem: Name 'test' is too close to typo-protected gem: best", @response.body + end + end + context "On POST to create for someone else's gem" do setup do @other_user = create(:user) diff --git a/test/unit/gem_typo_test.rb b/test/unit/gem_typo_test.rb index 90ebbc41f9c..7fa51811d95 100644 --- a/test/unit/gem_typo_test.rb +++ b/test/unit/gem_typo_test.rb @@ -1,78 +1,44 @@ -require 'test_helper' -require 'gem_typo' +require "test_helper" class GemTypoTest < ActiveSupport::TestCase - teardown do - Rails.cache.clear - end - - should 'return false for exact match' do - gem_typo = GemTypo.new('rspec-core') - assert_equal false, gem_typo.protected_typo? - end - - should 'return true for 1 char distance match' do - gem_typo = GemTypo.new('rspec-core2') - assert_equal true, gem_typo.protected_typo? - end - - should 'return false for 2 char distance match' do - gem_typo = GemTypo.new('rspec-core12') - assert_equal false, gem_typo.protected_typo? - end - - should 'return false for 3 char distance match' do - gem_typo = GemTypo.new('rspec-core123') - assert_equal false, gem_typo.protected_typo? - end - - should 'return false for 1 char distance match on the exception list' do - gem_typo = GemTypo.new('rspec-coreZ') - assert_equal false, gem_typo.protected_typo? - end - - should 'allow customized protected_gems' do - opts = { - protected_gems: ["hello"] - } - - gem_typo = GemTypo.new('hello', opts) - assert_equal false, gem_typo.protected_typo? - - gem_typo = GemTypo.new('hello1', opts) - assert_equal true, gem_typo.protected_typo? - end - - should 'allow customized distance_threshold' do - opts = { - distance_threshold: 3 - } - - gem_typo = GemTypo.new('rack', opts) - assert_equal false, gem_typo.protected_typo? - - gem_typo = GemTypo.new('rack1', opts) - assert_equal true, gem_typo.protected_typo? - - gem_typo = GemTypo.new('rack12', opts) - assert_equal true, gem_typo.protected_typo? - - gem_typo = GemTypo.new('rack123', opts) - assert_equal true, gem_typo.protected_typo? - - gem_typo = GemTypo.new('rack1234', opts) - assert_equal false, gem_typo.protected_typo? - end - - should 'allow customized protected_gem_exceptions' do - opts = { - gem_exceptions: ["rake1"] - } - - gem_typo = GemTypo.new('rake', opts) - assert_equal false, gem_typo.protected_typo? - - gem_typo = GemTypo.new('rake1', opts) - assert_equal false, gem_typo.protected_typo? + context "with above downloads threshold gem" do + setup do + above_downloads_thres = GemTypo::DOWNLOADS_THRESHOLD + 1 + create(:rubygem, name: "four", downloads: above_downloads_thres) + end + + should "return false for exact match" do + gem_typo = GemTypo.new("four") + assert_equal false, gem_typo.protected_typo? + end + + should "return false for gem name size below protected threshold" do + gem_typo = GemTypo.new("fou") + assert_equal false, gem_typo.protected_typo? + end + + context "size equals protected threshold" do + should "return true for one character distance" do + gem_typo = GemTypo.new("fous") + assert_equal true, gem_typo.protected_typo? + end + + should "return false for two character distance" do + gem_typo = GemTypo.new("foss") + assert_equal false, gem_typo.protected_typo? + end + end + + context "size above protected threshold" do + should "return true for two character distance" do + gem_typo = GemTypo.new("fourss") + assert_equal true, gem_typo.protected_typo? + end + + should "return false for three characher distance" do + gem_typo = GemTypo.new("foursss") + assert_equal false, gem_typo.protected_typo? + end + end end end