diff --git a/Manifest.txt b/Manifest.txt index 7a9ebba3d6ab..6f37d0aee3fc 100644 --- a/Manifest.txt +++ b/Manifest.txt @@ -493,6 +493,8 @@ lib/rubygems/test_utilities.rb lib/rubygems/text.rb lib/rubygems/uninstaller.rb lib/rubygems/uri_formatter.rb +lib/rubygems/uri_parser.rb +lib/rubygems/uri_parsing.rb lib/rubygems/user_interaction.rb lib/rubygems/util.rb lib/rubygems/util/licenses.rb diff --git a/lib/rubygems/remote_fetcher.rb b/lib/rubygems/remote_fetcher.rb index c399cc9d95c6..2ed619f1bc6f 100644 --- a/lib/rubygems/remote_fetcher.rb +++ b/lib/rubygems/remote_fetcher.rb @@ -4,6 +4,7 @@ require 'rubygems/request/connection_pools' require 'rubygems/s3_uri_signer' require 'rubygems/uri_formatter' +require 'rubygems/uri_parsing' require 'rubygems/user_interaction' require 'resolv' require 'rubygems/deprecate' @@ -17,12 +18,16 @@ class Gem::RemoteFetcher include Gem::UserInteraction extend Gem::Deprecate + include Gem::UriParsing + ## # A FetchError exception wraps up the various possible IO and HTTP failures # that could happen while downloading from the internet. class FetchError < Gem::Exception + include Gem::UriParsing + ## # The URI which was being accessed when the exception happened. @@ -30,13 +35,12 @@ class FetchError < Gem::Exception def initialize(message, uri) super message - begin - uri = URI(uri) - uri.password = 'REDACTED' if uri.password - @uri = uri.to_s - rescue URI::InvalidURIError, ArgumentError - @uri = uri - end + + uri = parse_uri(uri) + + uri.password = 'REDACTED' if uri.respond_to?(:password) && uri.password + + @uri = uri.to_s end def to_s # :nodoc: @@ -107,7 +111,7 @@ def download_to_cache(dependency) spec, source = found.max_by { |(s,_)| s.version } - download spec, source.uri.to_s + download spec, source.uri end ## @@ -130,18 +134,7 @@ def download(spec, source_uri, install_dir = Gem.dir) FileUtils.mkdir_p cache_dir rescue nil unless File.exist? cache_dir - # Always escape URI's to deal with potential spaces and such - # It should also be considered that source_uri may already be - # a valid URI with escaped characters. e.g. "{DESede}" is encoded - # as "%7BDESede%7D". If this is escaped again the percentage - # symbols will be escaped. - unless source_uri.is_a?(URI::Generic) - begin - source_uri = URI.parse(source_uri) - rescue - source_uri = URI.parse(URI::DEFAULT_PARSER.escape(source_uri.to_s)) - end - end + source_uri = parse_uri(source_uri) scheme = source_uri.scheme @@ -159,7 +152,7 @@ def download(spec, source_uri, install_dir = Gem.dir) remote_gem_path = source_uri + "gems/#{gem_file_name}" self.cache_update_path remote_gem_path, local_gem_path - rescue Gem::RemoteFetcher::FetchError + rescue FetchError raise if spec.original_platform == spec.platform alternate_name = "#{spec.original_name}.gem" @@ -236,7 +229,7 @@ def fetch_http(uri, last_modified = nil, head = false, depth = 0) unless location = response['Location'] raise FetchError.new("redirecting but no redirect location was given", uri) end - location = URI.parse response['Location'] + location = parse_uri location if https?(uri) && !https?(location) raise FetchError.new("redirecting to non-https resource: #{location}", uri) @@ -254,9 +247,7 @@ def fetch_http(uri, last_modified = nil, head = false, depth = 0) # Downloads +uri+ and returns it as a String. def fetch_path(uri, mtime = nil, head = false) - uri = URI.parse uri unless URI::Generic === uri - - raise ArgumentError, "bad uri: #{uri}" unless uri + uri = parse_uri uri unless uri.scheme raise ArgumentError, "uri scheme is invalid: #{uri.scheme.inspect}" @@ -268,21 +259,19 @@ def fetch_path(uri, mtime = nil, head = false) begin data = Gem::Util.gunzip data rescue Zlib::GzipFile::Error - raise FetchError.new("server did not return a valid file", uri.to_s) + raise FetchError.new("server did not return a valid file", uri) end end data - rescue FetchError - raise rescue Timeout::Error - raise UnknownHostError.new('timed out', uri.to_s) + raise UnknownHostError.new('timed out', uri) rescue IOError, SocketError, SystemCallError, *(OpenSSL::SSL::SSLError if defined?(OpenSSL)) => e if e.message =~ /getaddrinfo/ - raise UnknownHostError.new('no such name', uri.to_s) + raise UnknownHostError.new('no such name', uri) else - raise FetchError.new("#{e.class}: #{e}", uri.to_s) + raise FetchError.new("#{e.class}: #{e}", uri) end end diff --git a/lib/rubygems/request.rb b/lib/rubygems/request.rb index 435c7c53cbe4..b5a14ec34d7f 100644 --- a/lib/rubygems/request.rb +++ b/lib/rubygems/request.rb @@ -19,6 +19,7 @@ def self.create_with_proxy(uri, request_class, last_modified, proxy) # :nodoc: end def self.proxy_uri(proxy) # :nodoc: + require "uri" case proxy when :no_proxy then nil when URI::HTTP then proxy @@ -173,6 +174,7 @@ def self.get_proxy_from_env(scheme = 'http') :no_proxy : get_proxy_from_env('http') end + require "uri" uri = URI(Gem::UriFormatter.new(env_proxy).normalize) if uri and uri.user.nil? and uri.password.nil? diff --git a/lib/rubygems/source.rb b/lib/rubygems/source.rb index d949aa838d0d..8572cb180623 100644 --- a/lib/rubygems/source.rb +++ b/lib/rubygems/source.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true autoload :FileUtils, 'fileutils' -autoload :URI, 'uri' require "rubygems/text" ## diff --git a/lib/rubygems/uri_formatter.rb b/lib/rubygems/uri_formatter.rb index 0a24dde24d4b..f3d510470bb0 100644 --- a/lib/rubygems/uri_formatter.rb +++ b/lib/rubygems/uri_formatter.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true require 'cgi' -require 'uri' ## # The UriFormatter handles URIs from user-input and escaping. diff --git a/lib/rubygems/uri_parser.rb b/lib/rubygems/uri_parser.rb new file mode 100644 index 000000000000..66ce054167a2 --- /dev/null +++ b/lib/rubygems/uri_parser.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +## +# The UriParser handles parsing URIs. +# + +class Gem::UriParser + + def initialize(uri_namespace) + @uri_namespace = uri_namespace + end + + ## + # Parses the #uri, raising if it's invalid + + def parse!(uri) + raise invalid_uri_error unless uri + + # Always escape URI's to deal with potential spaces and such + # It should also be considered that source_uri may already be + # a valid URI with escaped characters. e.g. "{DESede}" is encoded + # as "%7BDESede%7D". If this is escaped again the percentage + # symbols will be escaped. + begin + @uri_namespace.public_send(:parse, uri) + rescue invalid_uri_error + @uri_namespace.public_send(:parse, @uri_namespace.const_get(:DEFAULT_PARSER).escape(uri)) + end + end + + ## + # Parses the #uri, returning the original uri if it's invalid + + def parse(uri) + parse!(uri) + rescue invalid_uri_error + uri + end + + private + + def invalid_uri_error + @invalid_uri_error ||= @uri_namespace.const_get(:InvalidURIError) + end + +end diff --git a/lib/rubygems/uri_parsing.rb b/lib/rubygems/uri_parsing.rb new file mode 100644 index 000000000000..1380efebfdad --- /dev/null +++ b/lib/rubygems/uri_parsing.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require "rubygems/uri_parser" + +module Gem::UriParsing + + def parse_uri(source_uri) + return source_uri unless source_uri.is_a?(String) + + uri_parser.parse(source_uri) + end + + private :parse_uri + + def uri_parser + require "uri" + + Gem::UriParser.new(URI) + end + + private :uri_parser + +end diff --git a/test/rubygems/test_remote_fetch_error.rb b/test/rubygems/test_remote_fetch_error.rb index 780e5e79f778..766086756ecb 100644 --- a/test/rubygems/test_remote_fetch_error.rb +++ b/test/rubygems/test_remote_fetch_error.rb @@ -5,7 +5,7 @@ class TestRemoteFetchError < Gem::TestCase def test_password_redacted error = Gem::RemoteFetcher::FetchError.new('There was an error fetching', 'https://user:secret@gemsource.org') - refute_match error.to_s, 'secret' + refute_match 'secret', error.to_s end def test_invalid_url