From cd18703582a9482464526e2abeda22a14fa91618 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Mon, 13 Feb 2023 21:15:59 +0100 Subject: [PATCH] Add audit for versions containing colons or slashes. --- Library/Homebrew/cask/audit.rb | 12 + Library/Homebrew/test/cask/audit_spec.rb | 219 +++++++++++------- .../fixtures/cask/Casks/version-colon.rb | 4 + 3 files changed, 146 insertions(+), 89 deletions(-) create mode 100644 Library/Homebrew/test/support/fixtures/cask/Casks/version-colon.rb diff --git a/Library/Homebrew/cask/audit.rb b/Library/Homebrew/cask/audit.rb index 09110a6888..3b128a07a3 100644 --- a/Library/Homebrew/cask/audit.rb +++ b/Library/Homebrew/cask/audit.rb @@ -228,6 +228,18 @@ module Cask add_warning "Cask should have a description. Please add a `desc` stanza." if cask.desc.blank? end + sig { void } + def audit_version_special_characters + return unless cask.version + + return if cask.version.latest? + + raw_version = cask.version.raw_version + return if raw_version.exclude?(":") && raw_version.exclude?("/") + + add_error "version should not contain colons or slashes" + end + sig { void } def audit_no_string_version_latest return unless cask.version diff --git a/Library/Homebrew/test/cask/audit_spec.rb b/Library/Homebrew/test/cask/audit_spec.rb index 19cb15265a..385c95890d 100644 --- a/Library/Homebrew/test/cask/audit_spec.rb +++ b/Library/Homebrew/test/cask/audit_spec.rb @@ -12,22 +12,55 @@ describe Cask::Audit, :cask do end end - matcher :pass do - match do |audit| - !audit.errors? && !audit.warnings? + def passed?(audit) + !audit.errors? && !audit.warnings? + end + + def outcome(audit) + if passed?(audit) + "passed" + else + message = "" + + message += "warned with #{audit.warnings.map { |e| e.fetch(:message).inspect }.join(",")}" if audit.warnings? + + if audit.errors? + message += " and " if audit.warnings? + message += "errored with #{audit.errors.map { |e| e.fetch(:message).inspect }.join(",")}" + end + + message end end - matcher :fail_with do |message| + matcher :pass do + match do |audit| + passed?(audit) + end + + failure_message do |audit| + "expected to pass, but #{outcome(audit)}" + end + end + + matcher :error_with do |message| match do |audit| include_msg?(audit.errors, message) end + + failure_message do |audit| + "expected to error with message #{message.inspect} but #{outcome(audit)}" + end end matcher :warn_with do |message| match do |audit| include_msg?(audit.warnings, message) end + + failure_message do |audit| + "expected to warn with message #{message.inspect} but #{outcome(audit)}" + end end let(:cask) { instance_double(Cask::Cask) } @@ -131,7 +164,7 @@ describe Cask::Audit, :cask do context "when missing #{stanza}" do let(:cask_token) { "missing-#{stanza}" } - it { is_expected.to fail_with(/#{stanza} stanza is required/) } + it { is_expected.to error_with(/#{stanza} stanza is required/) } end end end @@ -156,7 +189,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "Upper-Case" } it "fails" do - expect(run).to fail_with(/lowercase/) + expect(run).to error_with(/lowercase/) end end @@ -164,7 +197,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "ascii⌘" } it "fails" do - expect(run).to fail_with(/contains non-ascii characters/) + expect(run).to error_with(/contains non-ascii characters/) end end @@ -172,7 +205,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "app++" } it "fails" do - expect(run).to fail_with(/\+ should be replaced by -plus-/) + expect(run).to error_with(/\+ should be replaced by -plus-/) end end @@ -180,7 +213,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "app@stuff" } it "fails" do - expect(run).to fail_with(/@ should be replaced by -at-/) + expect(run).to error_with(/@ should be replaced by -at-/) end end @@ -188,7 +221,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "app stuff" } it "fails" do - expect(run).to fail_with(/whitespace should be replaced by hyphens/) + expect(run).to error_with(/whitespace should be replaced by hyphens/) end end @@ -196,7 +229,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "app_stuff" } it "fails" do - expect(run).to fail_with(/underscores should be replaced by hyphens/) + expect(run).to error_with(/underscores should be replaced by hyphens/) end end @@ -204,7 +237,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "app(stuff)" } it "fails" do - expect(run).to fail_with(/alphanumeric characters and hyphens/) + expect(run).to error_with(/alphanumeric characters and hyphens/) end end @@ -212,7 +245,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "app--stuff" } it "fails" do - expect(run).to fail_with(/should not contain double hyphens/) + expect(run).to error_with(/should not contain double hyphens/) end end @@ -220,7 +253,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "-app" } it "fails" do - expect(run).to fail_with(/should not have leading or trailing hyphens/) + expect(run).to error_with(/should not have leading or trailing hyphens/) end end @@ -228,7 +261,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "app-" } it "fails" do - expect(run).to fail_with(/should not have leading or trailing hyphens/) + expect(run).to error_with(/should not have leading or trailing hyphens/) end end end @@ -255,7 +288,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "token.app" } it "fails" do - expect(run).to fail_with(/token contains .app/) + expect(run).to error_with(/token contains .app/) end end @@ -265,7 +298,7 @@ describe Cask::Audit, :cask do it "fails if the cask is from an official tap" do allow(cask).to receive(:tap).and_return(Tap.fetch("homebrew/cask")) - expect(run).to fail_with(/token contains version designation/) + expect(run).to error_with(/token contains version designation/) end it "does not fail if the cask is from the `cask-versions` tap" do @@ -279,7 +312,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "token-launcher" } it "fails" do - expect(run).to fail_with(/token mentions launcher/) + expect(run).to error_with(/token mentions launcher/) end end @@ -287,7 +320,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "token-desktop" } it "fails" do - expect(run).to fail_with(/token mentions desktop/) + expect(run).to error_with(/token mentions desktop/) end end @@ -295,7 +328,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "token-osx" } it "fails" do - expect(run).to fail_with(/token mentions platform/) + expect(run).to error_with(/token mentions platform/) end end @@ -303,7 +336,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "token-x86" } it "fails" do - expect(run).to fail_with(/token mentions architecture/) + expect(run).to error_with(/token mentions architecture/) end end @@ -311,7 +344,7 @@ describe Cask::Audit, :cask do let(:cask_token) { "token-java" } it "fails" do - expect(run).to fail_with(/cask token mentions framework/) + expect(run).to error_with(/cask token mentions framework/) end end @@ -336,7 +369,7 @@ describe Cask::Audit, :cask do let(:new_cask) { true } it "fails" do - expect(run).to fail_with("#{cask_token} is listed in tap_migrations.json") + expect(run).to error_with("#{cask_token} is listed in tap_migrations.json") end end @@ -391,9 +424,9 @@ describe Cask::Audit, :cask do context "when cask locale is invalid" do it "error with invalid locale" do - expect(run).to fail_with(/Locale 'ZH-CN' is invalid\./) - expect(run).to fail_with(/Locale 'zh-' is invalid\./) - expect(run).to fail_with(/Locale 'zh-cn' is invalid\./) + expect(run).to error_with(/Locale 'ZH-CN' is invalid\./) + expect(run).to error_with(/Locale 'zh-' is invalid\./) + expect(run).to error_with(/Locale 'zh-cn' is invalid\./) end end end @@ -405,19 +438,19 @@ describe Cask::Audit, :cask do context "when the Cask has no pkg stanza" do let(:cask_token) { "basic-cask" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask does not have allow_untrusted" do let(:cask_token) { "with-uninstall-pkgutil" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask has allow_untrusted" do let(:cask_token) { "with-allow-untrusted" } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end end @@ -477,49 +510,49 @@ describe Cask::Audit, :cask do context "when the Cask has a livecheck block using skip" do let(:cask_token) { "livecheck/livecheck-skip" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask has a livecheck block referencing a Cask using skip" do let(:cask_token) { "livecheck/livecheck-skip-reference" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask is discontinued" do let(:cask_token) { "livecheck/discontinued" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask has a livecheck block referencing a discontinued Cask" do let(:cask_token) { "livecheck/discontinued-reference" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when version is :latest" do let(:cask_token) { "livecheck/version-latest" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask has a livecheck block referencing a Cask where version is :latest" do let(:cask_token) { "livecheck/version-latest-reference" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when url is unversioned" do let(:cask_token) { "livecheck/url-unversioned" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask has a livecheck block referencing a Cask with an unversioned url" do let(:cask_token) { "livecheck/url-unversioned-reference" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end end @@ -530,31 +563,31 @@ describe Cask::Audit, :cask do context "when the Cask does not require an uninstall" do let(:cask_token) { "basic-cask" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the pkg Cask has an uninstall" do let(:cask_token) { "with-uninstall-pkgutil" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the installer Cask has an uninstall" do let(:cask_token) { "installer-with-uninstall" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the installer Cask does not have an uninstall" do let(:cask_token) { "with-installer-manual" } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end context "when the pkg Cask does not have an uninstall" do let(:cask_token) { "pkg-without-uninstall" } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end end @@ -564,19 +597,19 @@ describe Cask::Audit, :cask do context "when the Cask has no preflight stanza" do let(:cask_token) { "with-zap-rmdir" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask has only one preflight stanza" do let(:cask_token) { "with-preflight" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask has multiple preflight stanzas" do let(:cask_token) { "with-preflight-multi" } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end end @@ -586,19 +619,19 @@ describe Cask::Audit, :cask do context "when the Cask has no postflight stanza" do let(:cask_token) { "with-zap-rmdir" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask has only one postflight stanza" do let(:cask_token) { "with-postflight" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask has multiple postflight stanzas" do let(:cask_token) { "with-postflight-multi" } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end end @@ -608,19 +641,19 @@ describe Cask::Audit, :cask do context "when the Cask has no uninstall_preflight stanza" do let(:cask_token) { "with-zap-rmdir" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask has only one uninstall_preflight stanza" do let(:cask_token) { "with-uninstall-preflight" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask has multiple uninstall_preflight stanzas" do let(:cask_token) { "with-uninstall-preflight-multi" } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end end @@ -630,19 +663,19 @@ describe Cask::Audit, :cask do context "when the Cask has no uninstall_postflight stanza" do let(:cask_token) { "with-zap-rmdir" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask has only one uninstall_postflight stanza" do let(:cask_token) { "with-uninstall-postflight" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask has multiple uninstall_postflight stanzas" do let(:cask_token) { "with-uninstall-postflight-multi" } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end end @@ -652,19 +685,19 @@ describe Cask::Audit, :cask do context "when the Cask has no zap stanza" do let(:cask_token) { "with-uninstall-rmdir" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask has only one zap stanza" do let(:cask_token) { "with-zap-rmdir" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask has multiple zap stanzas" do let(:cask_token) { "with-zap-multi" } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end end @@ -675,14 +708,22 @@ describe Cask::Audit, :cask do let(:only) { ["no_string_version_latest"] } let(:cask_token) { "version-latest-string" } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end context "when version is :latest" do let(:only) { ["sha256_no_check_if_latest"] } let(:cask_token) { "version-latest-with-checksum" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } + end + + context "when version contains a colon" do + let(:only) { ["version_special_characters"] } + let(:cask_token) { "version-colon" } + let(:message) { "version should not contain colons or slashes" } + + it { is_expected.to error_with(message) } end end @@ -691,21 +732,21 @@ describe Cask::Audit, :cask do let(:only) { ["sha256_no_check_if_latest"] } let(:cask_token) { "version-latest-with-checksum" } - it { is_expected.to fail_with("you should use sha256 :no_check when version is :latest") } + it { is_expected.to error_with("you should use sha256 :no_check when version is :latest") } end context "when sha256 is not a legal SHA-256 digest" do let(:only) { ["sha256_actually_256"] } let(:cask_token) { "invalid-sha256" } - it { is_expected.to fail_with("sha256 string must be of 64 hexadecimal characters") } + it { is_expected.to error_with("sha256 string must be of 64 hexadecimal characters") } end context "when sha256 is sha256 for empty string" do let(:only) { ["sha256_invalid"] } let(:cask_token) { "sha256-for-empty-string" } - it { is_expected.to fail_with(/cannot use the sha256 for an empty string/) } + it { is_expected.to error_with(/cannot use the sha256 for an empty string/) } end end @@ -716,44 +757,44 @@ describe Cask::Audit, :cask do context "when the download does not use hosting with a livecheck" do let(:cask_token) { "basic-cask" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the download is hosted on SourceForge and has a livecheck" do let(:cask_token) { "sourceforge-with-appcast" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the download is hosted on SourceForge and does not have a livecheck" do let(:cask_token) { "sourceforge-correct-url-format" } let(:online) { true } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end context "when the download is hosted on DevMate and has a livecheck" do let(:cask_token) { "devmate-with-appcast" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the download is hosted on DevMate and does not have a livecheck" do let(:cask_token) { "devmate-without-appcast" } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end context "when the download is hosted on HockeyApp and has a livecheck" do let(:cask_token) { "hockeyapp-with-appcast" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the download is hosted on HockeyApp and does not have a livecheck" do let(:cask_token) { "hockeyapp-without-appcast" } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end end @@ -764,19 +805,19 @@ describe Cask::Audit, :cask do context "when the Cask is :latest and does not have an appcast" do let(:cask_token) { "version-latest" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask is versioned and has an appcast" do let(:cask_token) { "with-appcast" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "when the Cask is :latest and has an appcast" do let(:cask_token) { "latest-with-appcast" } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end end @@ -793,7 +834,7 @@ describe Cask::Audit, :cask do context "when it's in the official Homebrew tap" do let(:cask_token) { "adobe-illustrator" } - it { is_expected.to fail_with(/#{cask_token} is not allowed: \w+/) } + it { is_expected.to error_with(/#{cask_token} is not allowed: \w+/) } end context "when it isn't in the official Homebrew tap" do @@ -829,7 +870,7 @@ describe Cask::Audit, :cask do context "when the Cask is :latest and has auto_updates" do let(:cask_token) { "latest-with-auto-updates" } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end end @@ -840,31 +881,31 @@ describe Cask::Audit, :cask do context "with incorrect SourceForge URL format" do let(:cask_token) { "sourceforge-incorrect-url-format" } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end context "with correct SourceForge URL format" do let(:cask_token) { "sourceforge-correct-url-format" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "with correct SourceForge URL format for version :latest" do let(:cask_token) { "sourceforge-version-latest-correct-url-format" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end context "with incorrect OSDN URL format" do let(:cask_token) { "osdn-incorrect-url-format" } - it { is_expected.to fail_with(message) } + it { is_expected.to error_with(message) } end context "with correct OSDN URL format" do let(:cask_token) { "osdn-correct-url-format" } - it { is_expected.not_to fail_with(message) } + it { is_expected.not_to error_with(message) } end end @@ -874,19 +915,19 @@ describe Cask::Audit, :cask do context "with relative target" do let(:cask_token) { "generic-artifact-relative-target" } - it { is_expected.to fail_with(/target must be.*absolute/) } + it { is_expected.to error_with(/target must be.*absolute/) } end context "with user-relative target" do let(:cask_token) { "generic-artifact-user-relative-target" } - it { is_expected.not_to fail_with(/target must be.*absolute/) } + it { is_expected.not_to error_with(/target must be.*absolute/) } end context "with absolute target" do let(:cask_token) { "generic-artifact-absolute-target" } - it { is_expected.not_to fail_with(/target must be.*absolute/) } + it { is_expected.not_to error_with(/target must be.*absolute/) } end end @@ -906,7 +947,7 @@ describe Cask::Audit, :cask do let(:online) { false } it "does not evaluate the block" do - expect(run).not_to fail_with(/Boom/) + expect(run).not_to error_with(/Boom/) end end @@ -914,7 +955,7 @@ describe Cask::Audit, :cask do let(:online) { true } it "evaluates the block" do - expect(run).to fail_with(/Boom/) + expect(run).to error_with(/Boom/) end end end @@ -961,7 +1002,7 @@ describe Cask::Audit, :cask do it "when download fails it fails" do expect(download_double).to receive(:fetch).and_raise(StandardError.new(message)) - expect(run).to fail_with(/#{message}/) + expect(run).to error_with(/#{message}/) end end @@ -971,7 +1012,7 @@ describe Cask::Audit, :cask do it "fails the audit" do expect(cask).to receive(:tap).and_raise(StandardError.new) - expect(run).to fail_with(/exception while auditing/) + expect(run).to error_with(/exception while auditing/) end end @@ -995,7 +1036,7 @@ describe Cask::Audit, :cask do let(:new_cask) { true } it "fails" do - expect(run).to fail_with(/should have a description/) + expect(run).to error_with(/should have a description/) end end @@ -1066,7 +1107,7 @@ describe Cask::Audit, :cask do RUBY end - it { is_expected.to fail_with(/a 'verified' parameter has to be added/) } + it { is_expected.to error_with(/a 'verified' parameter has to be added/) } end context "when the url does not match the homepage with verified" do @@ -1101,7 +1142,7 @@ describe Cask::Audit, :cask do RUBY end - it { is_expected.to fail_with(/a homepage stanza is required/) } + it { is_expected.to error_with(/a homepage stanza is required/) } end context "when url is lazy" do diff --git a/Library/Homebrew/test/support/fixtures/cask/Casks/version-colon.rb b/Library/Homebrew/test/support/fixtures/cask/Casks/version-colon.rb new file mode 100644 index 0000000000..c3285282c5 --- /dev/null +++ b/Library/Homebrew/test/support/fixtures/cask/Casks/version-colon.rb @@ -0,0 +1,4 @@ +cask "version-colon" do + version "1.2.3:1003" + sha256 :no_check +end