From a1438cf9149587606aca68f4b778839099c80fb7 Mon Sep 17 00:00:00 2001 From: Dean Welch Date: Thu, 22 Jun 2023 00:09:58 +0100 Subject: [PATCH 1/2] Add rubocop to CI --- .github/workflows/rubocop.yml | 24 +++++ .rubocop.yml | 41 ++++++++ .rubocop_todo.yml | 19 ++++ .ruby-gemset | 1 + Gemfile | 6 ++ Rakefile | 14 +-- bin/recog_cleanup | 15 +-- bin/recog_export | 42 ++++---- bin/recog_match | 41 ++++---- bin/recog_standardize | 118 +++++++++++----------- bin/recog_verify | 40 ++++---- features/support/aruba.rb | 2 + features/support/env.rb | 4 +- features/support/hooks.rb | 14 +-- misc/convert_mysql_err | 23 ++--- recog-content.gemspec | 24 ++--- spec/lib/fingerprint_self_test_spec.rb | 130 ++++++++++--------------- spec/spec_helper.rb | 3 +- 18 files changed, 314 insertions(+), 247 deletions(-) create mode 100644 .github/workflows/rubocop.yml create mode 100644 .rubocop.yml create mode 100644 .rubocop_todo.yml create mode 100644 .ruby-gemset diff --git a/.github/workflows/rubocop.yml b/.github/workflows/rubocop.yml new file mode 100644 index 00000000..4f23f076 --- /dev/null +++ b/.github/workflows/rubocop.yml @@ -0,0 +1,24 @@ +name: Rubocop + +on: [push, pull_request] + +permissions: + contents: read + +jobs: + rubocop: + runs-on: ubuntu-latest + env: + BUNDLE_ONLY: rubocop + + steps: + - uses: actions/checkout@v3 + + - name: Set up Ruby 2.7.5 + uses: ruby/setup-ruby@v1 + with: + ruby-version: 2.7.5 + bundler-cache: true + + - name: Run Rubocop + run: bundle exec rubocop --parallel --format simple diff --git a/.rubocop.yml b/.rubocop.yml new file mode 100644 index 00000000..ab082b22 --- /dev/null +++ b/.rubocop.yml @@ -0,0 +1,41 @@ +inherit_from: .rubocop_todo.yml + +AllCops: + TargetRubyVersion: 2.5.0 + SuggestExtensions: false + NewCops: disable + +Layout/LineLength: + Enabled: false + +Style/NumericPredicate: + Description: 'This adds no efficiency nor space saving' + Enabled: false + +Style/ZeroLengthPredicate: + Description: 'This adds no efficiency nor space saving' + Enabled: false + +Style/StderrPuts: + Description: 'Replaces $stderr.puts with warn() which can suppress messages' + Enabled: false + +Metrics/AbcSize: + Enabled: false + Description: 'This is often a red-herring' + +Metrics/MethodLength: + Description: 'The style guide suggests 10 lines, bumping it up to the current maximum' + Max: 21 + +Metrics/BlockLength: + Description: 'The style guide suggests 10 lines, bumping it up to the current maximum' + Max: 135 + +Layout/CommentIndentation: + Exclude: + - 'spec/spec_helper.rb' + +Style/BlockComments: + Exclude: + - 'spec/spec_helper.rb' diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml new file mode 100644 index 00000000..ebd932a4 --- /dev/null +++ b/.rubocop_todo.yml @@ -0,0 +1,19 @@ +# This configuration was generated by +# `rubocop --auto-gen-config` +# on 2023-06-21 22:59:09 UTC using RuboCop version 1.52.1. +# The point is for the user to remove these configuration records +# one by one as the offenses are removed from the code base. +# Note that changes in the inspected code, or installation of new +# versions of RuboCop, may require this file to be generated again. + +# Offense count: 3 +Security/Open: + Exclude: + - 'misc/convert_mysql_err' + - 'spec/lib/fingerprint_self_test_spec.rb' + +# Offense count: 6 +# Configuration parameters: AllowedVariables. +Style/GlobalVars: + Exclude: + - 'bin/recog_standardize' diff --git a/.ruby-gemset b/.ruby-gemset new file mode 100644 index 00000000..cf875f90 --- /dev/null +++ b/.ruby-gemset @@ -0,0 +1 @@ +recog diff --git a/Gemfile b/Gemfile index be07f2bc..89e16449 100644 --- a/Gemfile +++ b/Gemfile @@ -1,3 +1,5 @@ +# frozen_string_literal: true + source 'https://rubygems.org' gemspec name: 'recog-content' @@ -8,3 +10,7 @@ group :test do gem 'rake' gem 'regexp_parser' end + +group :rubocop do + gem 'rubocop' +end diff --git a/Rakefile b/Rakefile index 777a08cf..05620523 100644 --- a/Rakefile +++ b/Rakefile @@ -1,22 +1,24 @@ -require "bundler/gem_tasks" +# frozen_string_literal: true + +require 'bundler/gem_tasks' require 'rspec/core/rake_task' RSpec::Core::RakeTask.new do |t| - t.pattern = 'spec/**/*_spec.rb' + t.pattern = 'spec/**/*_spec.rb' end require 'yard' require 'yard/rake/yardoc_task' YARD::Rake::YardocTask.new do |t| - t.files = ['bin/*.rb', '-', 'README.md'] + t.files = ['bin/*.rb', '-', 'README.md'] end require 'cucumber' require 'cucumber/rake/task' Cucumber::Rake::Task.new(:features) do |t| - t.cucumber_opts = %w(features --format pretty) + t.cucumber_opts = %w[features --format pretty] end -task :default => [ :tests, :yard ] -task :tests => [ :spec, :features ] +task default: %i[tests yard] +task tests: %i[spec features] diff --git a/bin/recog_cleanup b/bin/recog_cleanup index 699e0461..234ac978 100755 --- a/bin/recog_cleanup +++ b/bin/recog_cleanup @@ -1,14 +1,15 @@ #!/usr/bin/env ruby +# frozen_string_literal: true require 'optparse' require 'ostruct' # Cleanup trailing whitespace around fingerprints -Dir[ File.expand_path(File.join(File.dirname(__FILE__), "..", "xml")) + "/*.xml" ].each do |f| - data = File.read(f). - gsub(/\s+$/, ''). # Trailing whitespace and empty lines - gsub("", "\n"). # Every fingerprint should have an empty line after it - gsub("-->", "-->\n") # Every comment should have an empty line after it +Dir["#{File.expand_path(File.join(File.dirname(__FILE__), '..', 'xml'))}/*.xml"].each do |f| + data = File.read(f) + .gsub(/\s+$/, '') # Trailing whitespace and empty lines + .gsub('', "\n") # Every fingerprint should have an empty line after it + .gsub('-->', "-->\n") # Every comment should have an empty line after it - File.write(f, data) -end \ No newline at end of file + File.write(f, data) +end diff --git a/bin/recog_export b/bin/recog_export index a151a382..91747d52 100755 --- a/bin/recog_export +++ b/bin/recog_export @@ -1,4 +1,5 @@ #!/usr/bin/env ruby +# frozen_string_literal: true require 'optparse' require 'ostruct' @@ -8,46 +9,44 @@ def squash_lines(str) str.split(/\n/).join(' ').gsub(/\s+/, ' ') end -def export_text(options) -end +def export_text(options); end def export_ruby(options) - $stdout.puts "# Recog fingerprint database export [ #{File.basename(options.xml_file)} ] on #{Time.now.to_s}" + $stdout.puts "# Recog fingerprint database export [ #{File.basename(options.xml_file)} ] on #{Time.now}" $stdout.puts "fp_str = '' # Set this value to the match string" - $stdout.puts "fp_match = {} # Match results are stored here" - $stdout.puts "" - $stdout.puts "case fp_str" + $stdout.puts 'fp_match = {} # Match results are stored here' + $stdout.puts '' + $stdout.puts 'case fp_str' options.db.fingerprints.each do |fp| puts " # #{squash_lines fp.name}" - puts " when /#{fp.regex.to_s}/" + puts " when /#{fp.regex}/" fp.tests.each do |test| puts " # Example: #{squash_lines test}" end - fp.params.each_pair do |k,v| + fp.params.each_pair do |k, v| if v[0] == 0 puts " fp_match[#{k.inspect}] = #{v[1].inspect}" else - puts " fp_match[#{k.inspect}] = $#{v[0].to_s}" + puts " fp_match[#{k.inspect}] = $#{v[0]}" end end - puts "" + puts '' end - $stdout.puts "end" + $stdout.puts 'end' end - options = OpenStruct.new(etype: :ruby) option_parser = OptionParser.new do |opts| - opts.banner = "Usage: #{$0} [options] XML_FINGERPRINTS_FILE" - opts.separator "Exports an XML fingerprint database to another format." - opts.separator "" - opts.separator "Options" + opts.banner = "Usage: #{$PROGRAM_NAME} [options] XML_FINGERPRINTS_FILE" + opts.separator 'Exports an XML fingerprint database to another format.' + opts.separator '' + opts.separator 'Options' - opts.on("-t", "--type type", - "Choose a type of export.", - " [r]uby (default - export a ruby case statement with regular expressions)", - " [t]ext (export a text description of the fingerprints)") do |etype| + opts.on('-t', '--type type', + 'Choose a type of export.', + ' [r]uby (default - export a ruby case statement with regular expressions)', + ' [t]ext (export a text description of the fingerprints)') do |etype| case etype.downcase when /^r/ options.etype = :ruby @@ -56,7 +55,7 @@ option_parser = OptionParser.new do |opts| end end - opts.on("-h", "--help", "Show this message.") do + opts.on('-h', '--help', 'Show this message.') do puts opts exit end @@ -77,4 +76,3 @@ when :ruby when :text export_text(options) end - diff --git a/bin/recog_match b/bin/recog_match index 66a65ec2..ee3965b1 100755 --- a/bin/recog_match +++ b/bin/recog_match @@ -1,4 +1,5 @@ #!/usr/bin/env ruby +# frozen_string_literal: true require 'optparse' require 'ostruct' @@ -8,47 +9,47 @@ require 'recog/matcher_factory' options = OpenStruct.new(color: false, detail: false, json_format: false, fail_fast: false, multi_match: false) option_parser = OptionParser.new do |opts| - opts.banner = "Usage: #{$0} [options] XML_FINGERPRINT_FILE [BANNERS_FILE]" - opts.separator "Identifies the matches and misses between the fingerprints and the banners file or STDIN" - opts.separator "" - opts.separator "Options" + opts.banner = "Usage: #{$PROGRAM_NAME} [options] XML_FINGERPRINT_FILE [BANNERS_FILE]" + opts.separator 'Identifies the matches and misses between the fingerprints and the banners file or STDIN' + opts.separator '' + opts.separator 'Options' - opts.on("-f", "--format FORMATTER", [:summary, :detail, :json], - "Choose a formatter.", - " [s]ummary (default - failure/match msgs)", - " [d]etail (msgs with total counts)", - " [j]son (JSON failure/match msgs)") do |format| - if format == :summary + opts.on('-f', '--format FORMATTER', %i[summary detail json], + 'Choose a formatter.', + ' [s]ummary (default - failure/match msgs)', + ' [d]etail (msgs with total counts)', + ' [j]son (JSON failure/match msgs)') do |format| + case format + when :summary options.detail = false options.json_format = false - elsif format == :detail + when :detail options.detail = true - elsif format == :json + when :json options.json_format = true end end - opts.on("--fail-fast [NUM]", - "Stop after number of failures (default: 10).") do |num| + opts.on('--fail-fast [NUM]', + 'Stop after number of failures (default: 10).') do |num| options.fail_fast = true - options.stop_after = (num.to_i == 0) ? 10 : num.to_i + options.stop_after = num.to_i == 0 ? 10 : num.to_i end - opts.on("-c", "--color", "Enable color in the output.") do + opts.on('-c', '--color', 'Enable color in the output.') do options.color = true end - opts.on("--[no-]multi-match", "Enable or disable multiple matches (defaults to disabled)") do |o| + opts.on('--[no-]multi-match', 'Enable or disable multiple matches (defaults to disabled)') do |o| options.multi_match = o end - opts.on("-h", "--help", "Show this message.") do + opts.on('-h', '--help', 'Show this message.') do puts opts exit end end - begin option_parser.parse!(ARGV) rescue OptionParser::ParseError => e @@ -65,4 +66,4 @@ end ndb = Recog::DB.new(ARGV.shift) options.fingerprints = ndb.fingerprints matcher = Recog::MatcherFactory.build(options) -matcher.match_banners(ARGV.shift || "-") +matcher.match_banners(ARGV.shift || '-') diff --git a/bin/recog_standardize b/bin/recog_standardize index 99582c55..2ef2da7c 100755 --- a/bin/recog_standardize +++ b/bin/recog_standardize @@ -1,29 +1,29 @@ #!/usr/bin/env ruby +# frozen_string_literal: true require 'optparse' require 'ostruct' require 'recog' -bdir = File.expand_path(File.join(File.dirname(__FILE__), "..", "identifiers")) +bdir = File.expand_path(File.join(File.dirname(__FILE__), '..', 'identifiers')) IDENTIFIER_FILES = { - "vendor" => File.join(bdir, "vendor.txt"), - "device" => File.join(bdir, "device.txt"), - "fields" => File.join(bdir, "fields.txt"), - "hw_prod" => File.join(bdir, "hw_product.txt"), - "hw_family" => File.join(bdir, "hw_family.txt"), - "os_arch" => File.join(bdir, "os_architecture.txt"), - "os_prod" => File.join(bdir, "os_product.txt"), - "os_family" => File.join(bdir, "os_family.txt"), - "svc_prod" => File.join(bdir, "service_product.txt"), - "svc_family" => File.join(bdir, "service_family.txt") -} + 'vendor' => File.join(bdir, 'vendor.txt'), + 'device' => File.join(bdir, 'device.txt'), + 'fields' => File.join(bdir, 'fields.txt'), + 'hw_prod' => File.join(bdir, 'hw_product.txt'), + 'hw_family' => File.join(bdir, 'hw_family.txt'), + 'os_arch' => File.join(bdir, 'os_architecture.txt'), + 'os_prod' => File.join(bdir, 'os_product.txt'), + 'os_family' => File.join(bdir, 'os_family.txt'), + 'svc_prod' => File.join(bdir, 'service_product.txt'), + 'svc_family' => File.join(bdir, 'service_family.txt') +}.freeze # @param ident_type [String] Key used to get the identifier file path # @return [Hash] Contents of the indicated identifier file or an empty hash # if the file doesn't exist def load_identifiers(ident_type) - res = {} path = IDENTIFIER_FILES[ident_type] @@ -34,7 +34,7 @@ def load_identifiers(ident_type) return res unless File.file?(path) - File.readlines(path).map{|line| line.strip}.each do |ident| + File.readlines(path).map(&:strip).each do |ident| res[ident] = true end @@ -42,7 +42,6 @@ def load_identifiers(ident_type) end def write_identifiers(vals, ident_type) - path = IDENTIFIER_FILES[ident_type] unless path warn "Unknown identifier file type '#{ident_type}' when writing identifiers, exiting.." @@ -51,10 +50,10 @@ def write_identifiers(vals, ident_type) res = [] vals.each_pair do |k, _| - res = res.push(k) + res.push(k) end - res = res.map{|x| x.strip}.select{|x| x.length > 0}.sort.uniq - File.write(path, res.join("\n") + "\n") + res = res.map(&:strip).select { |x| x.length > 0 }.sort.uniq + File.write(path, "#{res.join("\n")}\n") end # @param current [Hash] Indentifiers extracted from fingerprints @@ -63,7 +62,6 @@ end # @param ident_type [String] Key used to get the identifier file path # @param write [Boolean] Indicate if changes should be written to disk def handle_changes(current, original, msg, ident_type, write) - changes_detected = false # The hashes below are being sorted so as to make it easier for human @@ -78,7 +76,6 @@ def handle_changes(current, original, msg, ident_type, write) changes_detected = true end - current.sort.to_h.each_key do |k| next if original[k] @@ -92,18 +89,18 @@ end options = OpenStruct.new(write: false) option_parser = OptionParser.new do |opts| - opts.banner = "Usage: #{$0} [options] XML_FINGERPRINT_FILE1 ..." - opts.separator "Verifies that each fingerprint asserts known identifiers." - opts.separator "Known identifiers are stored in reference files in the path: " + opts.banner = "Usage: #{$PROGRAM_NAME} [options] XML_FINGERPRINT_FILE1 ..." + opts.separator 'Verifies that each fingerprint asserts known identifiers.' + opts.separator 'Known identifiers are stored in reference files in the path: ' opts.separator bdir - opts.separator "" - opts.separator "Options" + opts.separator '' + opts.separator 'Options' - opts.on("-w", "--write", "Write newly discovered identifiers to the identifiers reference files.") do + opts.on('-w', '--write', 'Write newly discovered identifiers to the identifiers reference files.') do options.write = true end - opts.on("-h", "--help", "Show this message.") do + opts.on('-h', '--help', 'Show this message.') do puts opts exit end @@ -117,16 +114,16 @@ if ARGV.empty? end # Load the unique identifiers -vendors = load_identifiers("vendor") -devices = load_identifiers("device") -fields = load_identifiers("fields") -os_arch = load_identifiers("os_arch") -os_prod = load_identifiers("os_prod") -os_family = load_identifiers("os_family") -hw_prod = load_identifiers("hw_prod") -hw_family = load_identifiers("hw_family") -svc_prod = load_identifiers("svc_prod") -svc_family = load_identifiers("svc_family") +vendors = load_identifiers('vendor') +devices = load_identifiers('device') +fields = load_identifiers('fields') +os_arch = load_identifiers('os_arch') +os_prod = load_identifiers('os_prod') +os_family = load_identifiers('os_family') +hw_prod = load_identifiers('hw_prod') +hw_family = load_identifiers('hw_family') +svc_prod = load_identifiers('svc_prod') +svc_family = load_identifiers('svc_family') $found_removed = false $found_new = false @@ -144,40 +141,39 @@ curr_hw_family = {} curr_svc_prod = {} curr_svc_family = {} - ARGV.each do |arg| Dir.glob(arg).each do |file| ndb = Recog::DB.new(file) ndb.fingerprints.each do |f| f.params.each do |k, v| - # Don't track temporary attributes. - next if k.start_with?("_tmp.") + next if k.start_with?('_tmp.') + curr_fields[k] = true param_index, val = v next if param_index != 0 - next if val.index("{") != nil - next if val.strip == "" + next unless val.index('{').nil? + next if val.strip == '' case k - when "os.vendor", "service.vendor", "service.component.vendor", "hw.vendor" + when 'os.vendor', 'service.vendor', 'service.component.vendor', 'hw.vendor' curr_vendors[val] = true - when "os.device", "service.device", "hw.device" + when 'os.device', 'service.device', 'hw.device' curr_devices[val] = true - when "os.arch" + when 'os.arch' curr_os_arch[val] = true - when "os.product" + when 'os.product' curr_os_prod[val] = true - when "os.family" + when 'os.family' curr_os_family[val] = true - when "hw.product" + when 'hw.product' curr_hw_prod[val] = true - when "hw.family" + when 'hw.family' curr_hw_family[val] = true - when "service.product", "service.component.product" + when 'service.product', 'service.component.product' curr_svc_prod[val] = true - when "service.family" + when 'service.family' curr_svc_family[val] = true end end @@ -185,16 +181,16 @@ ARGV.each do |arg| end end -handle_changes(curr_vendors, vendors, "VENDORS", "vendor", options.write) -handle_changes(curr_devices, devices, "DEVICE", "device", options.write) -handle_changes(curr_fields, fields, "FIELDS", "fields", options.write) -handle_changes(curr_os_arch, os_arch, "OS ARCH", "os_arch", options.write) -handle_changes(curr_os_prod, os_prod, "OS PRODUCT", "os_prod", options.write) -handle_changes(curr_os_family, os_family, "OS FAMILY", "os_family", options.write) -handle_changes(curr_hw_prod, hw_prod, "HW PRODUCT", "hw_prod", options.write) -handle_changes(curr_hw_family, hw_family, "HW FAMILY", "hw_family", options.write) -handle_changes(curr_svc_prod, svc_prod, "SERVICE PRODUCT", "svc_prod", options.write) -handle_changes(curr_svc_family, svc_family, "SERVICE FAMILY", "svc_family", options.write) +handle_changes(curr_vendors, vendors, 'VENDORS', 'vendor', options.write) +handle_changes(curr_devices, devices, 'DEVICE', 'device', options.write) +handle_changes(curr_fields, fields, 'FIELDS', 'fields', options.write) +handle_changes(curr_os_arch, os_arch, 'OS ARCH', 'os_arch', options.write) +handle_changes(curr_os_prod, os_prod, 'OS PRODUCT', 'os_prod', options.write) +handle_changes(curr_os_family, os_family, 'OS FAMILY', 'os_family', options.write) +handle_changes(curr_hw_prod, hw_prod, 'HW PRODUCT', 'hw_prod', options.write) +handle_changes(curr_hw_family, hw_family, 'HW FAMILY', 'hw_family', options.write) +handle_changes(curr_svc_prod, svc_prod, 'SERVICE PRODUCT', 'svc_prod', options.write) +handle_changes(curr_svc_family, svc_family, 'SERVICE FAMILY', 'svc_family', options.write) exit_code = ($found_removed || $found_new ? 1 : 0) exit(exit_code) diff --git a/bin/recog_verify b/bin/recog_verify index bb47a07d..1275270f 100755 --- a/bin/recog_verify +++ b/bin/recog_verify @@ -1,4 +1,5 @@ #!/usr/bin/env ruby +# frozen_string_literal: true require 'nokogiri' require 'optparse' @@ -11,37 +12,34 @@ require 'recog/verify_reporter' options = OpenStruct.new(color: false, detail: false, quiet: false, warnings: true, schema: nil) option_parser = OptionParser.new do |opts| - opts.banner = "Usage: #{$0} [options] XML_FINGERPRINT_FILE1 ..." - opts.separator "Verifies that each fingerprint passes its internal tests." - opts.separator "" - opts.separator "Options" + opts.banner = "Usage: #{$PROGRAM_NAME} [options] XML_FINGERPRINT_FILE1 ..." + opts.separator 'Verifies that each fingerprint passes its internal tests.' + opts.separator '' + opts.separator 'Options' - opts.on("-f", "--format FORMATTER", - "Choose a formatter.", - " [s]ummary (default - failure/warning msgs and summary)", - " [q]uiet (configured failure/warning msgs only)", - " [d]etail (fingerprint name with tests and expanded summary)") do |format| - if format.start_with? 'd' - options.detail = true - end - if format.start_with? 'q' - options.quiet = true - end + opts.on('-f', '--format FORMATTER', + 'Choose a formatter.', + ' [s]ummary (default - failure/warning msgs and summary)', + ' [q]uiet (configured failure/warning msgs only)', + ' [d]etail (fingerprint name with tests and expanded summary)') do |format| + options.detail = true if format.start_with? 'd' + options.quiet = true if format.start_with? 'q' end - opts.on("-c", "--color", "Enable color in the output.") do + opts.on('-c', '--color', 'Enable color in the output.') do options.color = true end - opts.on("--[no-]warnings", "Track warnings") do |o| + opts.on('--[no-]warnings', 'Track warnings') do |o| options.warnings = o end - opts.on("--schema-location SCHEMA_FILE", "Location of the Recog XSD file. If not specified, validation will not be run.") do |schema_file| + opts.on('--schema-location SCHEMA_FILE', + 'Location of the Recog XSD file. If not specified, validation will not be run.') do |schema_file| options.schema = Nokogiri::XML::Schema(File.read(schema_file)) end - opts.on("-h", "--help", "Show this message.") do + opts.on('-h', '--help', 'Show this message.') do puts opts exit end @@ -60,7 +58,7 @@ formatter = Recog::Formatter.new(options, $stdout) ARGV.each do |arg| Dir.glob(arg).each do |file| # Create a new reporter per XML file to hold context on success/warn/fails - reporter = Recog::VerifyReporter.new(options, formatter, file) + reporter = Recog::VerifyReporter.new(options, formatter, file) begin # Validate the XML database against the recog schema first, if requested @@ -85,7 +83,7 @@ ARGV.each do |arg| verifier.verify rescue Recog::FingerprintParseError => e reporter.failure(e.message, e.line_number) - rescue => e + rescue StandardError => e reporter.failure(e.message) ensure failures += reporter.failure_count diff --git a/features/support/aruba.rb b/features/support/aruba.rb index 16abfdc3..061cfb71 100644 --- a/features/support/aruba.rb +++ b/features/support/aruba.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + Aruba.configure do |config| config.working_directory = 'features/data' end diff --git a/features/support/env.rb b/features/support/env.rb index 1a9b554f..afc87c5b 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -1,6 +1,8 @@ +# frozen_string_literal: true + require 'aruba/cucumber' Before do - @dirs = ["features/data"] + @dirs = ['features/data'] @aruba_timeout_seconds = 30 end diff --git a/features/support/hooks.rb b/features/support/hooks.rb index 3867c097..26f7540c 100644 --- a/features/support/hooks.rb +++ b/features/support/hooks.rb @@ -1,9 +1,11 @@ -require "cucumber/platform" +# frozen_string_literal: true -Before "@requires-ruby-platform-java" do - skip_this_scenario unless Cucumber::JRUBY +require 'cucumber/platform' + +Before '@requires-ruby-platform-java' do + skip_this_scenario unless Cucumber::JRUBY end - -Before "@unsupported-on-platform-java" do - skip_this_scenario if Cucumber::JRUBY + +Before '@unsupported-on-platform-java' do + skip_this_scenario if Cucumber::JRUBY end diff --git a/misc/convert_mysql_err b/misc/convert_mysql_err index c7d70619..140bce4c 100755 --- a/misc/convert_mysql_err +++ b/misc/convert_mysql_err @@ -1,4 +1,5 @@ #!/usr/bin/env ruby +# frozen_string_literal: true # Takes the MySQL error messages from sql/share/errmsg-utf8.txt, locates the # provided error message type (for example, ER_HOST_NOT_PRIVILEGED), then @@ -12,7 +13,7 @@ require 'open-uri' require 'securerandom' def generate_recog(error_name, locale, error_message) - xml = Builder::XmlMarkup.new(target: STDOUT, indent: 2) + xml = Builder::XmlMarkup.new(target: $stdout, indent: 2) xml.fingerprint(pattern: error_message) do xml.description "Oracle MySQL error #{error_name} (#{locale})" xml.example(error_message) @@ -22,28 +23,24 @@ def generate_recog(error_name, locale, error_message) end end -unless ARGV.size == 2 - fail "Usage: #{$PROGRAM_NAME} " -end +raise "Usage: #{$PROGRAM_NAME} " unless ARGV.size == 2 path = ARGV.first error_name = ARGV.last lines = IO.readlines(open(path)) -fail "Nothing read from #{path}" if lines.empty? +raise "Nothing read from #{path}" if lines.empty? unless (error_start = lines.find_index { |line| line.strip =~ /^#{error_name}(?:\s+\S+)?$/ }) - fail "Unable to find #{error_name} in #{path}" + raise "Unable to find #{error_name} in #{path}" end locale_map = {} lines.slice(error_start + 1, lines.size).each do |line| - if /^\s+(?\S+)\s+"(?.*)",?$/ =~ line - locale_map[locale] = error_message - else - break - end + break unless /^\s+(?\S+)\s+"(?.*)",?$/ =~ line + + locale_map[locale] = error_message end # Many of the error messages contain format strings. This can be problematic @@ -52,9 +49,7 @@ end # a rough count of the possible format strings and warn the user so that they # can deal with it. format_count = locale_map.values.map { |error_message| error_message.scan(/%/).size }.inject(&:+) -unless format_count == 0 - warn("#{format_count} possible format strings found -- you'll need to deal with this") -end +warn("#{format_count} possible format strings found -- you'll need to deal with this") unless format_count == 0 Hash[locale_map.sort].map do |locale, error_message| generate_recog(error_name, locale, error_message) diff --git a/recog-content.gemspec b/recog-content.gemspec index f88ed01f..7969b7c4 100644 --- a/recog-content.gemspec +++ b/recog-content.gemspec @@ -1,31 +1,31 @@ -# -*- encoding: utf-8 -*- +# frozen_string_literal: true Gem::Specification.new do |s| s.name = 'recog-content' - s.version = "0.0.1" - s.required_ruby_version = '>= 2.1' - s.authors = [ + s.version = '0.0.1' + s.required_ruby_version = '>= 2.5' + s.authors = [ 'Rapid7 Research' ] - s.email = [ + s.email = [ 'research@rapid7.com' ] - s.homepage = "https://www.github.com/rapid7/recog" - s.summary = %q{Network service fingerprint database and utilities} - s.description = %q{ + s.homepage = 'https://www.github.com/rapid7/recog' + s.summary = 'Network service fingerprint database and utilities' + s.description = ' recog-content is the Recog fingerprint database and management utilities. Recog is a framework for identifying products, services, operating systems, and hardware by matching fingerprints against datareturned from various network probes. Recog makes it simply to extract useful information from web server banners, snmp system description fields, and a whole lot more. - }.gsub(/\s+/, ' ').strip + '.gsub(/\s+/, ' ').strip - s.files = %w(Gemfile Rakefile COPYING LICENSE README.md recog-content.gemspec .yardopts) + + s.files = %w[Gemfile Rakefile COPYING LICENSE README.md recog-content.gemspec .yardopts] + Dir.glob('bin/*') + Dir.glob('features**/*') + Dir.glob('xml/*') s.test_files = s.files.grep(%r{^(test|spec|features)/}) - s.executables = s.files.grep(%r{^bin/}).map{ |f| File.basename(f) } + s.executables = s.files.grep(%r{^bin/}).map { |f| File.basename(f) } # ---- Dependencies ---- @@ -38,8 +38,8 @@ Gem::Specification.new do |s| # markdown formatting for yard s.add_development_dependency 'redcarpet' end - s.add_development_dependency 'cucumber' s.add_development_dependency 'aruba' + s.add_development_dependency 'cucumber' s.add_development_dependency 'simplecov' s.add_runtime_dependency 'recog' diff --git a/spec/lib/fingerprint_self_test_spec.rb b/spec/lib/fingerprint_self_test_spec.rb index 757a0e70..55e0d006 100644 --- a/spec/lib/fingerprint_self_test_spec.rb +++ b/spec/lib/fingerprint_self_test_spec.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'recog/db' require 'regexp_parser' require 'nokogiri' @@ -5,10 +7,8 @@ describe Recog::DB do let(:schema) { Nokogiri::XML::Schema(open(File.join(FINGERPRINT_DIR, 'fingerprints.xsd'))) } Dir[File.join(FINGERPRINT_DIR, '*.xml')].each do |xml_file_name| - describe "##{File.basename(xml_file_name)}" do - - it "is valid XML" do + it 'is valid XML' do doc = Nokogiri::XML(open(xml_file_name)) errors = schema.validate(doc) expect(errors).to be_empty, "#{xml_file_name} is invalid recog XML -- #{errors.inspect}" @@ -16,16 +16,16 @@ db = Recog::DB.new(xml_file_name) - it "has a match key" do + it 'has a match key' do expect(db.match_key).not_to be_nil expect(db.match_key).not_to be_empty end it "has valid 'preference' value" do - # Reserve values below 0.10 and above 0.90 for users - # See xml/fingerprints.xsd - expect(db.preference.class).to be ::Float - expect(db.preference).to be_between(0.10, 0.90) + # Reserve values below 0.10 and above 0.90 for users + # See xml/fingerprints.xsd + expect(db.preference.class).to be ::Float + expect(db.preference).to be_between(0.10, 0.90) end fp_descriptions = [] @@ -33,72 +33,53 @@ fp = db.fingerprints[i] it "doesn't have a duplicate description" do - if fp_descriptions.include?(fp.name) - fail "'#{fp.name}'s description is not unique" - else - fp_descriptions << fp.name - end + raise "'#{fp.name}'s description is not unique" if fp_descriptions.include?(fp.name) + + fp_descriptions << fp.name end - context "#{fp.name}" do + context fp.name.to_s do param_names = [] - it "has consistent os.device and hw.device" do - if fp.params['os.device'] && fp.params['hw.device'] && (fp.params['os.device'] != fp.params['hw.device']) - fail "#{fp.name} has both hw.device and os.device but with differing values" - end + it 'has consistent os.device and hw.device' do + raise "#{fp.name} has both hw.device and os.device but with differing values" if fp.params['os.device'] && fp.params['hw.device'] && (fp.params['os.device'] != fp.params['hw.device']) end fp.params.each do |param_name, pos_value| pos, value = pos_value - it "has valid looking fingerprint parameter names" do - unless param_name =~ /^(?:cookie|[^\.]+\..*)$/ - fail "'#{param_name}' is invalid" - end + it 'has valid looking fingerprint parameter names' do + raise "'#{param_name}' is invalid" unless param_name =~ /^(?:cookie|[^.]+\..*)$/ end it "doesn't have param values for capture params" do - if pos > 0 && !value.to_s.empty? - fail "'#{fp.name}'s #{param_name} is a non-zero pos but specifies a value of '#{value}'" - end + raise "'#{fp.name}'s #{param_name} is a non-zero pos but specifies a value of '#{value}'" if pos > 0 && !value.to_s.empty? end - it "has *.device parameter values other than General, Server or Unknown, which are not helpful" do - if pos == 0 && param_name =~ /^(?:[^\.]+\.device*)$/ && value =~ /^(?i:general|server|unknown)$/ - fail "'#{param_name}' has unhelpful value '#{value}'" - end + it 'has *.device parameter values other than General, Server or Unknown, which are not helpful' do + raise "'#{param_name}' has unhelpful value '#{value}'" if pos == 0 && param_name =~ /^(?:[^.]+\.device*)$/ && value =~ /^(?i:general|server|unknown)$/ end it "doesn't omit values for non-capture params" do - if pos == 0 && value.to_s.empty? - fail "'#{fp.name}'s #{param_name} is not a capture (pos=0) but doesn't specify a value" - end + raise "'#{fp.name}'s #{param_name} is not a capture (pos=0) but doesn't specify a value" if pos == 0 && value.to_s.empty? end it "doesn't have duplicate params" do - if param_names.include?(param_name) - fail "'#{fp.name}'s has duplicate #{param_name}" - else - param_names << param_name - end + raise "'#{fp.name}'s has duplicate #{param_name}" if param_names.include?(param_name) + + param_names << param_name end - it "uses interpolation correctly" do - if pos == 0 && /\{(?[^\s{}]+)\}/ =~ value - unless fp.params.key?(interpolated) - fail "'#{fp.name}' uses interpolated value '#{interpolated}' that does not exist" - end - end + it 'uses interpolation correctly' do + raise "'#{fp.name}' uses interpolated value '#{interpolated}' that does not exist" if pos == 0 && /\{(?[^\s{}]+)\}/ =~ value && !fp.params.key?(interpolated) end end end - context "#{fp.regex}" do - - it "has a valid looking name" do + context fp.regex.to_s do + it 'has a valid looking name' do expect(fp.name).not_to be_nil expect(fp.name).not_to be_empty end - it "has a regex" do + it 'has a regex' do expect(fp.regex).not_to be_nil expect(fp.regex.class).to be ::Regexp end @@ -106,25 +87,23 @@ it 'uses capturing regular expressions properly' do # the list of index-based captures that the fingerprint is expecting expected_capture_positions = fp.params.values.map(&:first).map(&:to_i).select { |position| position > 0 } - if fp.params.empty? && expected_capture_positions.size > 0 - fail "Non-asserting fingerprint with regex #{fp.regex} captures #{expected_capture_positions.size} time(s); 0 are needed" - else - # parse the regex and count the number of captures - actual_capture_positions = [] - capture_number = 1 - Regexp::Scanner.scan(fp.regex).each do |token_parts| - if token_parts.first == :group && ![:close, :passive, :options, :options_switch].include?(token_parts[1]) - actual_capture_positions << capture_number - capture_number += 1 - end + raise "Non-asserting fingerprint with regex #{fp.regex} captures #{expected_capture_positions.size} time(s); 0 are needed" if fp.params.empty? && expected_capture_positions.size > 0 + + # parse the regex and count the number of captures + actual_capture_positions = [] + capture_number = 1 + Regexp::Scanner.scan(fp.regex).each do |token_parts| + if token_parts.first == :group && !%i[close passive options options_switch].include?(token_parts[1]) + actual_capture_positions << capture_number + capture_number += 1 end - # compare the captures actually performed to those being used and ensure that they contain - # the same elements regardless of order, preventing, over-, under- and other forms of mis-capturing. - actual_capture_positions = actual_capture_positions.sort.uniq - expected_capture_positions = expected_capture_positions.sort.uniq - expect(actual_capture_positions).to eq(expected_capture_positions), - "Regex has #{actual_capture_positions.size} capture groups, but the fingerprint expected #{expected_capture_positions.size} extractions." end + # compare the captures actually performed to those being used and ensure that they contain + # the same elements regardless of order, preventing, over-, under- and other forms of mis-capturing. + actual_capture_positions = actual_capture_positions.sort.uniq + expected_capture_positions = expected_capture_positions.sort.uniq + expect(actual_capture_positions).to eq(expected_capture_positions), + "Regex has #{actual_capture_positions.size} capture groups, but the fingerprint expected #{expected_capture_positions.size} extractions." end # Not yet enforced @@ -132,44 +111,43 @@ # expect(fp.tests.length).not_to equal(0) # end - it "Has a reasonable number (<= 20) of test cases" do + it 'Has a reasonable number (<= 20) of test cases' do expect(fp.tests.length).to be <= 20 end fp_examples = [] fp.tests.each do |example| it "doesn't have a duplicate examples" do - if fp_examples.include?(example.content) - fail "'#{fp.name}' has duplicate example '#{example.content}'" - else - fp_examples << example.content - end + raise "'#{fp.name}' has duplicate example '#{example.content}'" if fp_examples.include?(example.content) + + fp_examples << example.content end it "Example '#{example.content}' matches this regex" do match = fp.match(example.content) expect(match).to_not be_nil, 'Regex did not match' # test any extractions specified in the example - example.attributes.each_pair do |k,v| + example.attributes.each_pair do |k, v| next if k == '_encoding' next if k == '_filename' - expect(match[k]).to eq(v), "Regex didn't extract expected value for fingerprint attribute #{k} -- got #{match[k]} instead of #{v}" + + expect(match[k]).to eq(v), + "Regex didn't extract expected value for fingerprint attribute #{k} -- got #{match[k]} instead of #{v}" end end it "Example '#{example.content}' matches this regex first" do db.fingerprints.slice(0, i).each_index do |previous_i| prev_fp = db.fingerprints[previous_i] - prev_fp.tests.each do |prev_example| + prev_fp.tests.each do |_prev_example| match = prev_fp.match(example.content) - expect(match).to be_nil, "Matched regex ##{previous_i} (#{db.fingerprints[previous_i].regex}) rather than ##{i} (#{db.fingerprints[i].regex})" + expect(match).to be_nil, + "Matched regex ##{previous_i} (#{db.fingerprints[previous_i].regex}) rather than ##{i} (#{db.fingerprints[i].regex})" end end end end - end end - end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 836411b0..acaedf5b 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + FINGERPRINT_DIR = File.expand_path(File.join('..', 'xml'), __dir__) # setup code coverage @@ -22,7 +24,6 @@ # # See http://rubydoc.info/gems/rspec-core/RSpec/Core/Configuration RSpec.configure do |config| - # Run specs in random order to surface order dependencies. If you find an # order dependency and want to debug it, you can fix the order by providing # the seed, which is printed after each run. From 0961a7f9cdd3acb9d21d59bfede158705f0be6a0 Mon Sep 17 00:00:00 2001 From: Dean Welch Date: Wed, 28 Jun 2023 13:17:54 +0100 Subject: [PATCH 2/2] Remove ruby version from rubocop workflow, uses the version in `.ruby-version` instead --- .github/workflows/rubocop.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/rubocop.yml b/.github/workflows/rubocop.yml index 4f23f076..e7cac2d8 100644 --- a/.github/workflows/rubocop.yml +++ b/.github/workflows/rubocop.yml @@ -14,10 +14,9 @@ jobs: steps: - uses: actions/checkout@v3 - - name: Set up Ruby 2.7.5 + - name: Set up Ruby uses: ruby/setup-ruby@v1 with: - ruby-version: 2.7.5 bundler-cache: true - name: Run Rubocop