From eac33df8c336ef5ed9263fb4a0a12086a83b5ee3 Mon Sep 17 00:00:00 2001 From: Phil Ross Date: Sat, 21 Jan 2023 20:18:15 +0000 Subject: [PATCH] Eliminate Object#untaint deprecation warnings on JRuby 9.4.0.0. Despite using the UntaintExt refinement that replaces Object#untaint, JRuby 9.4.0.0 calls the original Object#untaint method and outputs warnings as a result: warning: Object#untaint is deprecated and will be removed in Ruby 3.2 This is caused by the unorthodox `send(:using, UntaintExt)` approach being used to deal with issues with older JRuby versions (see #114). JRuby 9.4.0.0 doesn't detect this as using the refinement. This will be allowed again in JRuby 9.4.1.0 (see jruby/jruby#7599). Replace the UntaintExt refinement with an untaint method in RubyCoreSupport (now reinstated having previously been removed in 2.0.0). Change tests for handling of tainted inputs to skip when taint/untaint is undefined or a no-op. There's no point in running these tests unless the inputs are actually tainted. Remove the (test-only) TaintExt refinement too. Resolves #145. --- lib/tzinfo.rb | 8 +--- .../data_sources/posix_time_zone_parser.rb | 8 +--- lib/tzinfo/data_sources/ruby_data_source.rb | 6 +-- .../data_sources/zoneinfo_data_source.rb | 6 +-- lib/tzinfo/data_sources/zoneinfo_reader.rb | 6 +-- lib/tzinfo/ruby_core_support.rb | 38 +++++++++++++++++++ lib/tzinfo/untaint_ext.rb | 18 --------- .../data_sources/tc_posix_time_zone_parser.rb | 6 +-- test/data_sources/tc_ruby_data_source.rb | 12 ++++-- test/data_sources/tc_zoneinfo_data_source.rb | 19 ++++++---- test/data_sources/tc_zoneinfo_reader.rb | 10 ++--- test/tc_country.rb | 10 +++-- test/tc_ruby_core_support.rb | 19 ++++++++++ test/tc_timezone.rb | 10 +++-- test/test_utils.rb | 30 ++++++++++----- test/ts_all_zoneinfo.rb | 2 +- 16 files changed, 125 insertions(+), 83 deletions(-) create mode 100644 lib/tzinfo/ruby_core_support.rb delete mode 100644 lib/tzinfo/untaint_ext.rb create mode 100644 test/tc_ruby_core_support.rb diff --git a/lib/tzinfo.rb b/lib/tzinfo.rb index b1b53449..dc8e0856 100644 --- a/lib/tzinfo.rb +++ b/lib/tzinfo.rb @@ -17,12 +17,8 @@ def eager_load! end end -# Object#untaint is a deprecated no-op in Ruby >= 2.7 and will be removed in -# 3.2. Add a refinement to either silence the warning, or supply the method if -# needed. -if !Object.new.respond_to?(:untaint) || RUBY_VERSION =~ /\A(\d+)\.(\d+)(?:\.|\z)/ && ($1 == '2' && $2.to_i >= 7 || $1.to_i >= 3) - require_relative 'tzinfo/untaint_ext' -end + +require_relative 'tzinfo/ruby_core_support' require_relative 'tzinfo/version' diff --git a/lib/tzinfo/data_sources/posix_time_zone_parser.rb b/lib/tzinfo/data_sources/posix_time_zone_parser.rb index b3d2b2e3..5583b43e 100644 --- a/lib/tzinfo/data_sources/posix_time_zone_parser.rb +++ b/lib/tzinfo/data_sources/posix_time_zone_parser.rb @@ -4,10 +4,6 @@ require 'strscan' module TZInfo - # Use send as a workaround for erroneous 'wrong number of arguments' errors - # with JRuby 9.0.5.0 when calling methods with Java implementations. See #114. - send(:using, UntaintExt) if TZInfo.const_defined?(:UntaintExt) - module DataSources # An {InvalidPosixTimeZone} exception is raised if an invalid POSIX-style # time zone string is encountered. @@ -43,12 +39,12 @@ def parse(tz_string) s = StringScanner.new(tz_string) check_scan(s, /([^-+,\d<][^-+,\d]*) | <([^>]+)>/x) - std_abbrev = @string_deduper.dedupe((s[1] || s[2]).untaint) + std_abbrev = @string_deduper.dedupe(RubyCoreSupport.untaint(s[1] || s[2])) check_scan(s, /([-+]?\d+)(?::(\d+)(?::(\d+))?)?/) std_offset = get_offset_from_hms(s[1], s[2], s[3]) if s.scan(/([^-+,\d<][^-+,\d]*) | <([^>]+)>/x) - dst_abbrev = @string_deduper.dedupe((s[1] || s[2]).untaint) + dst_abbrev = @string_deduper.dedupe(RubyCoreSupport.untaint(s[1] || s[2])) if s.scan(/([-+]?\d+)(?::(\d+)(?::(\d+))?)?/) dst_offset = get_offset_from_hms(s[1], s[2], s[3]) diff --git a/lib/tzinfo/data_sources/ruby_data_source.rb b/lib/tzinfo/data_sources/ruby_data_source.rb index 9c123225..41c384a4 100644 --- a/lib/tzinfo/data_sources/ruby_data_source.rb +++ b/lib/tzinfo/data_sources/ruby_data_source.rb @@ -2,10 +2,6 @@ # frozen_string_literal: true module TZInfo - # Use send as a workaround for erroneous 'wrong number of arguments' errors - # with JRuby 9.0.5.0 when calling methods with Java implementations. See #114. - send(:using, UntaintExt) if TZInfo.const_defined?(:UntaintExt) - module DataSources # A {TZInfoDataNotFound} exception is raised if the tzinfo-data gem could # not be found (i.e. `require 'tzinfo/data'` failed) when selecting the Ruby @@ -52,7 +48,7 @@ def initialize data_file = File.join('', 'tzinfo', 'data.rb') path = $".reverse_each.detect {|p| p.end_with?(data_file) } if path - @base_path = File.join(File.dirname(path), 'data').untaint + @base_path = RubyCoreSupport.untaint(File.join(File.dirname(path), 'data')) else @base_path = 'tzinfo/data' end diff --git a/lib/tzinfo/data_sources/zoneinfo_data_source.rb b/lib/tzinfo/data_sources/zoneinfo_data_source.rb index cf77576f..4b7de32c 100644 --- a/lib/tzinfo/data_sources/zoneinfo_data_source.rb +++ b/lib/tzinfo/data_sources/zoneinfo_data_source.rb @@ -2,10 +2,6 @@ # frozen_string_literal: true module TZInfo - # Use send as a workaround for erroneous 'wrong number of arguments' errors - # with JRuby 9.0.5.0 when calling methods with Java implementations. See #114. - send(:using, UntaintExt) if TZInfo.const_defined?(:UntaintExt) - module DataSources # An {InvalidZoneinfoDirectory} exception is raised if {ZoneinfoDataSource} # is initialized with a specific zoneinfo path that is not a valid zoneinfo @@ -444,7 +440,7 @@ def enum_timezones(dir, exclude = [], &block) end unless entry =~ /\./ || exclude.include?(entry) - entry.untaint + RubyCoreSupport.untaint(entry) path = dir + [entry] full_path = File.join(@zoneinfo_dir, *path) diff --git a/lib/tzinfo/data_sources/zoneinfo_reader.rb b/lib/tzinfo/data_sources/zoneinfo_reader.rb index ac82585c..7f598875 100644 --- a/lib/tzinfo/data_sources/zoneinfo_reader.rb +++ b/lib/tzinfo/data_sources/zoneinfo_reader.rb @@ -2,10 +2,6 @@ # frozen_string_literal: true module TZInfo - # Use send as a workaround for erroneous 'wrong number of arguments' errors - # with JRuby 9.0.5.0 when calling methods with Java implementations. See #114. - send(:using, UntaintExt) if TZInfo.const_defined?(:UntaintExt) - module DataSources # An {InvalidZoneinfoFile} exception is raised if an attempt is made to load # an invalid zoneinfo file. @@ -448,7 +444,7 @@ def parse(file) abbrev_end = abbrev.index("\0", abbrev_start) raise InvalidZoneinfoFile, "Missing abbreviation null terminator in file '#{file.path}'." unless abbrev_end - abbr = @string_deduper.dedupe(abbrev[abbrev_start...abbrev_end].force_encoding(Encoding::UTF_8).untaint) + abbr = @string_deduper.dedupe(RubyCoreSupport.untaint(abbrev[abbrev_start...abbrev_end].force_encoding(Encoding::UTF_8))) TimezoneOffset.new(base_utc_offset, std_offset, abbr) end diff --git a/lib/tzinfo/ruby_core_support.rb b/lib/tzinfo/ruby_core_support.rb new file mode 100644 index 00000000..9e43e3c9 --- /dev/null +++ b/lib/tzinfo/ruby_core_support.rb @@ -0,0 +1,38 @@ +module TZInfo + + # Methods to support different versions of Ruby. + # + # @private + module RubyCoreSupport #:nodoc: + class << self + # Object#untaint is deprecated and becomes a no-op in Ruby >= 2.7. It has + # been removed from Ruby 3.2. + if !Object.new.respond_to?(:untaint) || RUBY_VERSION =~ /\A(\d+)\.(\d+)(?:\.|\z)/ && ($1 == '2' && $2.to_i >= 7 || $1.to_i >= 3) + # :nocov_functional_untaint: + + # Returns the supplied `Object` + # + # @param o [Object] the `Object` to untaint. + # @return [Object] `o`. + def untaint(o) + o + end + + # :nocov_functional_untaint: + else + # :nocov_no_functional_untaint: + + # Untaints and returns the supplied `Object`. + # + # @param o [Object] the `Object` to untaint. + # @return [Object] `o`. + def untaint(o) + o.untaint + end + + # :nocov_no_functional_untaint: + end + end + end + private_constant :RubyCoreSupport +end diff --git a/lib/tzinfo/untaint_ext.rb b/lib/tzinfo/untaint_ext.rb deleted file mode 100644 index 4e8d0c07..00000000 --- a/lib/tzinfo/untaint_ext.rb +++ /dev/null @@ -1,18 +0,0 @@ -# encoding: UTF-8 -# frozen_string_literal: true - -module TZInfo - # Object#untaint is deprecated in Ruby >= 2.7 and will be removed in 3.2. - # UntaintExt adds a refinement to make Object#untaint a no-op and avoid the - # warning. - # - # @private - module UntaintExt # :nodoc: - refine Object do - def untaint - self - end - end - end - private_constant :UntaintExt -end diff --git a/test/data_sources/tc_posix_time_zone_parser.rb b/test/data_sources/tc_posix_time_zone_parser.rb index 73dbb747..57bf5d9c 100644 --- a/test/data_sources/tc_posix_time_zone_parser.rb +++ b/test/data_sources/tc_posix_time_zone_parser.rb @@ -3,10 +3,6 @@ require_relative '../test_utils' -# Use send as a workaround for erroneous 'wrong number of arguments' errors with -# JRuby 9.0.5.0 when calling methods with Java implementations. See #114. -send(:using, TestUtils::TaintExt) if TestUtils.const_defined?(:TaintExt) - module DataSources class TCPosixTimeZoneParser < Minitest::Test include TZInfo @@ -259,6 +255,8 @@ def test_returns_deduped_strings end def test_parses_tainted_string_in_safe_mode_and_returns_untainted_abbreviations + skip_if_taint_is_undefined_or_no_op + safe_test(unavailable: :skip) do result = @parser.parse('STD1DST,60,300'.dup.taint) diff --git a/test/data_sources/tc_ruby_data_source.rb b/test/data_sources/tc_ruby_data_source.rb index 1447bc76..88eb55d2 100644 --- a/test/data_sources/tc_ruby_data_source.rb +++ b/test/data_sources/tc_ruby_data_source.rb @@ -3,10 +3,6 @@ require_relative '../test_utils' -# Use send as a workaround for erroneous 'wrong number of arguments' errors with -# JRuby 9.0.5.0 when calling methods with Java implementations. See #114. -send(:using, TestUtils::TaintExt) if TestUtils.const_defined?(:TaintExt) - module DataSources class TCRubyDataSource < Minitest::Test include TZInfo @@ -123,6 +119,8 @@ def test_load_timezone_info_minus end def test_load_timezone_info_tainted + skip_if_taint_is_undefined_or_no_op + safe_test(unavailable: :skip) do identifier = 'Europe/Amsterdam'.dup.taint assert(identifier.tainted?) @@ -133,6 +131,8 @@ def test_load_timezone_info_tainted end def test_load_timezone_info_tainted_and_frozen + skip_if_taint_is_undefined_or_no_op + safe_test do info = @data_source.send(:load_timezone_info, 'Europe/Amsterdam'.dup.taint.freeze) assert_equal('Europe/Amsterdam', info.identifier) @@ -232,6 +232,8 @@ def test_load_country_info_case end def test_load_country_info_tainted + skip_if_taint_is_undefined_or_no_op + safe_test(unavailable: :skip) do code = 'NL'.dup.taint assert(code.tainted?) @@ -242,6 +244,8 @@ def test_load_country_info_tainted end def test_load_country_info_tainted_and_frozen + skip_if_taint_is_undefined_or_no_op + safe_test do info = @data_source.send(:load_country_info, 'NL'.dup.taint.freeze) assert_equal('NL', info.code) diff --git a/test/data_sources/tc_zoneinfo_data_source.rb b/test/data_sources/tc_zoneinfo_data_source.rb index 5327d3bb..d812a174 100644 --- a/test/data_sources/tc_zoneinfo_data_source.rb +++ b/test/data_sources/tc_zoneinfo_data_source.rb @@ -6,17 +6,13 @@ require 'pathname' require 'tmpdir' -# Use send as a workaround for erroneous 'wrong number of arguments' errors with -# JRuby 9.0.5.0 when calling methods with Java implementations. See #114. -send(:using, TestUtils::TaintExt) if TestUtils.const_defined?(:TaintExt) -send(:using, TZInfo.const_get(:UntaintExt)) if TZInfo.const_defined?(:UntaintExt) - module DataSources class TCZoneinfoDataSource < Minitest::Test include TZInfo include TZInfo::DataSources - ZONEINFO_DIR = File.expand_path(File.join(File.dirname(__FILE__), '..', 'zoneinfo')).untaint + ZONEINFO_DIR = File.expand_path(File.join(File.dirname(__FILE__), '..', 'zoneinfo')) + RubyCoreSupport.untaint(ZONEINFO_DIR) def setup @orig_search_path = ZoneinfoDataSource.search_path.clone @@ -788,6 +784,8 @@ def test_load_timezone_info_linked_relative_parent_inside end def test_load_timezone_info_tainted + skip_if_taint_is_undefined_or_no_op + safe_test(unavailable: :skip) do identifier = 'Europe/Amsterdam'.dup.taint assert(identifier.tainted?) @@ -798,6 +796,8 @@ def test_load_timezone_info_tainted end def test_load_timezone_info_tainted_and_frozen + skip_if_taint_is_undefined_or_no_op + safe_test do info = @data_source.send(:load_timezone_info, 'Europe/Amsterdam'.dup.taint.freeze) assert_equal('Europe/Amsterdam', info.identifier) @@ -805,6 +805,8 @@ def test_load_timezone_info_tainted_and_frozen end def test_load_timezone_info_tainted_zoneinfo_dir_safe_mode + skip_if_taint_is_undefined_or_no_op + safe_test(unavailable: :skip) do assert_raises(SecurityError) do ZoneinfoDataSource.new(@data_source.zoneinfo_dir.dup.taint) @@ -813,6 +815,7 @@ def test_load_timezone_info_tainted_zoneinfo_dir_safe_mode end def test_load_timezone_info_tainted_zoneinfo_dir + skip_if_taint_is_undefined_or_no_op data_source = ZoneinfoDataSource.new(@data_source.zoneinfo_dir.dup.taint) info = data_source.send(:load_timezone_info, 'Europe/London') assert_kind_of(TransitionsDataTimezoneInfo, info) @@ -849,7 +852,7 @@ def get_timezone_filenames(directory) entries = Dir.glob(File.join(directory, '**', '*')) entries = entries.select do |file| - file.untaint + RubyCoreSupport.untaint(file) File.file?(file) end @@ -1149,6 +1152,8 @@ def test_load_country_info_case end def test_load_country_info_tainted + skip_if_taint_is_undefined_or_no_op + safe_test(unavailable: :skip) do code = 'NL'.dup.taint assert(code.tainted?) diff --git a/test/data_sources/tc_zoneinfo_reader.rb b/test/data_sources/tc_zoneinfo_reader.rb index 4a52228f..0d46cf5f 100644 --- a/test/data_sources/tc_zoneinfo_reader.rb +++ b/test/data_sources/tc_zoneinfo_reader.rb @@ -4,10 +4,6 @@ require_relative '../test_utils' require 'tempfile' -# Use send as a workaround for erroneous 'wrong number of arguments' errors with -# JRuby 9.0.5.0 when calling methods with Java implementations. See #114. -send(:using, TZInfo.const_get(:UntaintExt)) if TZInfo.const_defined?(:UntaintExt) - module DataSources class TCZoneinfoReader < Minitest::Test include TZInfo @@ -1257,7 +1253,7 @@ def test_read_untainted_in_safe_mode tzif_test(offsets, []) do |path, format| safe_test do # Temp file path is tainted with Ruby >= 2.3 & < 2.7. Untaint for this test. - path.untaint + RubyCoreSupport.untaint(path) assert_equal(o0, @reader.read(path)) end @@ -1265,6 +1261,8 @@ def test_read_untainted_in_safe_mode end def test_read_tainted_in_safe_mode + skip_if_taint_is_undefined_or_no_op + offsets = [{gmtoff: -12094, isdst: false, abbrev: 'LT'}] tzif_test(offsets, []) do |path, format| @@ -1278,6 +1276,8 @@ def test_read_tainted_in_safe_mode end def test_read_abbreviations_not_tainted + skip_if_taint_is_undefined_or_no_op + offsets = [{gmtoff: -12094, isdst: false, abbrev: 'LT'}] tzif_test(offsets, []) do |path, format| diff --git a/test/tc_country.rb b/test/tc_country.rb index 02883f9e..3807b2ab 100644 --- a/test/tc_country.rb +++ b/test/tc_country.rb @@ -3,10 +3,6 @@ require_relative 'test_utils' -# Use send as a workaround for erroneous 'wrong number of arguments' errors with -# JRuby 9.0.5.0 when calling methods with Java implementations. See #114. -send(:using, TestUtils::TaintExt) if TestUtils.const_defined?(:TaintExt) - class TCCountry < Minitest::Test include TZInfo @@ -48,6 +44,7 @@ def test_get_nil end def test_get_tainted_loaded + skip_if_taint_is_undefined_or_no_op Country.get('GB') safe_test(unavailable: :skip) do @@ -60,6 +57,7 @@ def test_get_tainted_loaded end def test_get_tainted_and_frozen_loaded + skip_if_taint_is_undefined_or_no_op Country.get('GB') safe_test do @@ -69,6 +67,8 @@ def test_get_tainted_and_frozen_loaded end def test_get_tainted_not_previously_loaded + skip_if_taint_is_undefined_or_no_op + safe_test(unavailable: :skip) do code = 'GB'.dup.taint assert(code.tainted?) @@ -79,6 +79,8 @@ def test_get_tainted_not_previously_loaded end def test_get_tainted_and_frozen_not_previously_loaded + skip_if_taint_is_undefined_or_no_op + safe_test do country = Country.get('GB'.dup.taint.freeze) assert_equal('GB', country.code) diff --git a/test/tc_ruby_core_support.rb b/test/tc_ruby_core_support.rb new file mode 100644 index 00000000..7253c0ea --- /dev/null +++ b/test/tc_ruby_core_support.rb @@ -0,0 +1,19 @@ +# encoding: UTF-8 +# frozen_string_literal: true + +require_relative 'test_utils' + +class TCRubyCoreSupport < Minitest::Test + def test_untaint_returns_object + o = Object.new + assert_same(o, TZInfo.const_get(:RubyCoreSupport).untaint(o)) + end + + def test_untaint_untaints_object + skip_if_taint_is_undefined_or_no_op + o = Object.new.taint + assert(o.tainted?) + TZInfo.const_get(:RubyCoreSupport).untaint(o) + refute(o.tainted?) + end +end diff --git a/test/tc_timezone.rb b/test/tc_timezone.rb index 0dc3300b..bd82af8b 100644 --- a/test/tc_timezone.rb +++ b/test/tc_timezone.rb @@ -3,10 +3,6 @@ require_relative 'test_utils' -# Use send as a workaround for erroneous 'wrong number of arguments' errors with -# JRuby 9.0.5.0 when calling methods with Java implementations. See #114. -send(:using, TestUtils::TaintExt) if TestUtils.const_defined?(:TaintExt) - class TCTimezone < Minitest::Test include TZInfo @@ -262,6 +258,7 @@ def test_get_nil end def test_get_tainted_loaded + skip_if_taint_is_undefined_or_no_op Timezone.get('Europe/Andorra') safe_test(unavailable: :skip) do @@ -274,6 +271,7 @@ def test_get_tainted_loaded end def test_get_tainted_and_frozen_loaded + skip_if_taint_is_undefined_or_no_op Timezone.get('Europe/Andorra') safe_test do @@ -283,6 +281,8 @@ def test_get_tainted_and_frozen_loaded end def test_get_tainted_not_previously_loaded + skip_if_taint_is_undefined_or_no_op + safe_test(unavailable: :skip) do identifier = 'Europe/Andorra'.dup.taint assert(identifier.tainted?) @@ -293,6 +293,8 @@ def test_get_tainted_not_previously_loaded end def test_get_tainted_and_frozen_not_previously_loaded + skip_if_taint_is_undefined_or_no_op + safe_test do tz = Timezone.get('Europe/Amsterdam'.dup.taint.freeze) assert_equal('Europe/Amsterdam', tz.identifier) diff --git a/test/test_utils.rb b/test/test_utils.rb index ec83bf30..e23d7d24 100644 --- a/test/test_utils.rb +++ b/test/test_utils.rb @@ -28,7 +28,12 @@ false end - feature_support = [['deduping_string_unary_minus', string_unary_minus_does_dedupe]].map do |feature, available| + untaint_is_functional = Object.new.respond_to?(:untaint) && RUBY_VERSION < '2.7' + + feature_support = [ + ['deduping_string_unary_minus', string_unary_minus_does_dedupe], + ['functional_untaint', untaint_is_functional] + ].map do |feature, available| "#{available ? '' : 'no_'}#{feature}" end @@ -509,16 +514,23 @@ def assert_equal_with_offset_and_class(expected, actual) assert_equal_with_offset(expected, actual) assert_equal(expected.class, actual.class) end - end - # Object#taint is a deprecated no-op in Ruby 2.7 and outputs a warning. It - # will be removed in 3.2. Silence the warning or supply a replacement. - if TZInfo.const_defined?(:UntaintExt) - module TaintExt - refine Object do - def taint - self + # Object#taint is deprecated in Ruby >= 2.7 and will be removed in 3.2. + # 2.7 makes it a no-op with a warning. + # Define a method that will skip for use in tests that deal with tainted + # objects. + if Object.respond_to?(:taint) + if RUBY_VERSION >= '2.7' + def skip_if_taint_is_undefined_or_no_op + skip('Object#taint is a no-op') end + else + def skip_if_taint_is_undefined_or_no_op + end + end + else + def skip_if_taint_is_undefined_or_no_op + skip('Object#taint is not defined') end end end diff --git a/test/ts_all_zoneinfo.rb b/test/ts_all_zoneinfo.rb index 870a0618..d55bb286 100644 --- a/test/ts_all_zoneinfo.rb +++ b/test/ts_all_zoneinfo.rb @@ -8,7 +8,7 @@ # Use a zoneinfo directory containing files needed by the tests. # The symlinks in this directory are set up in test_utils.rb. zoneinfo_path = File.join(File.expand_path(File.dirname(__FILE__)), 'zoneinfo') -zoneinfo_path.untaint if RUBY_VERSION < '2.7' +TZInfo::const_get(:RubyCoreSupport).untaint(zoneinfo_path) TZInfo::DataSource.set(:zoneinfo, zoneinfo_path) require_relative 'ts_all'