ruby-changes:67488
From: David <ko1@a...>
Date: Tue, 31 Aug 2021 19:08:12 +0900 (JST)
Subject: [ruby-changes:67488] 1e290c31f4 (master): [rubygems/rubygems] Merge `Gem::UriParser` and `Gem::PrintableUri` into a `Gem::Uri` class
https://git.ruby-lang.org/ruby.git/commit/?id=1e290c31f4 From 1e290c31f4fdfd330b9cd1d5c7fe61efa4ab066c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Rodr=C3=ADguez?= <deivid.rodriguez@r...> Date: Tue, 24 Aug 2021 12:02:29 +0200 Subject: [rubygems/rubygems] Merge `Gem::UriParser` and `Gem::PrintableUri` into a `Gem::Uri` class The new class is a wrapper on top of an URI. And then, when you want credentials redacted, you call `#redacted` that returns a copy of itself, but with credentials redacted. https://github.com/rubygems/rubygems/commit/9581c2740a --- lib/rubygems/commands/install_command.rb | 5 +- lib/rubygems/printable_uri.rb | 61 ------------------ lib/rubygems/remote_fetcher.rb | 17 +++--- lib/rubygems/request.rb | 3 +- lib/rubygems/uri.rb | 102 +++++++++++++++++++++++++++++++ lib/rubygems/uri_parser.rb | 42 ------------- test/rubygems/test_gem_printable_uri.rb | 44 ------------- test/rubygems/test_gem_uri.rb | 32 ++++++++++ test/rubygems/test_gem_uri_parser.rb | 17 ------ 9 files changed, 145 insertions(+), 178 deletions(-) delete mode 100644 lib/rubygems/printable_uri.rb create mode 100644 lib/rubygems/uri.rb delete mode 100644 lib/rubygems/uri_parser.rb delete mode 100644 test/rubygems/test_gem_printable_uri.rb create mode 100644 test/rubygems/test_gem_uri.rb delete mode 100644 test/rubygems/test_gem_uri_parser.rb diff --git a/lib/rubygems/commands/install_command.rb b/lib/rubygems/commands/install_command.rb index ee50cba..adf2cdb 100644 --- a/lib/rubygems/commands/install_command.rb +++ b/lib/rubygems/commands/install_command.rb @@ -5,7 +5,6 @@ require_relative '../dependency_installer' https://github.com/ruby/ruby/blob/trunk/lib/rubygems/commands/install_command.rb#L5 require_relative '../local_remote_options' require_relative '../validator' require_relative '../version_option' -require_relative '../printable_uri' ## # Gem installer command line tool @@ -261,8 +260,8 @@ You can use `i` command instead of `install`. https://github.com/ruby/ruby/blob/trunk/lib/rubygems/commands/install_command.rb#L260 errors.each do |x| return unless Gem::SourceFetchProblem === x - printable_uri = Gem::PrintableUri.parse_uri(x.source.uri.clone) - msg = "Unable to pull data from '#{printable_uri}': #{x.error.message}" + require_relative "../uri" + msg = "Unable to pull data from '#{Gem::Uri.new(x.source.uri).redacted}': #{x.error.message}" alert_warning msg end diff --git a/lib/rubygems/printable_uri.rb b/lib/rubygems/printable_uri.rb deleted file mode 100644 index 93c852e..0000000 --- a/lib/rubygems/printable_uri.rb +++ /dev/null @@ -1,61 +0,0 @@ https://github.com/ruby/ruby/blob/trunk/lib/rubygems/commands/install_command.rb#L0 -# frozen_string_literal: true - -require_relative 'uri_parser' - -class Gem::PrintableUri - def self.parse_uri(uri) - printable_uri = new(uri) - printable_uri.parse_uri - - printable_uri - end - - def initialize(original_uri) - @original_uri = original_uri - end - - def parse_uri - @original_uri = Gem::UriParser.parse_uri(@original_uri) - @uri = @original_uri.dup - redact_credential if valid_uri? - end - - def valid_uri? - @uri.respond_to?(:user) && - @uri.respond_to?(:user=) && - @uri.respond_to?(:password) && - @uri.respond_to?(:password=) - end - - def original_password - @original_uri.password - end - - def to_s - @uri.to_s - end - - private - - def redact_credential - if token? - @uri.user = 'REDACTED' - elsif oauth_basic? - @uri.user = 'REDACTED' - elsif password? - @uri.password = 'REDACTED' - end - end - - def password? - !!@uri.password - end - - def oauth_basic? - @uri.password == 'x-oauth-basic' - end - - def token? - !@uri.user.nil? && @uri.password.nil? - end -end diff --git a/lib/rubygems/remote_fetcher.rb b/lib/rubygems/remote_fetcher.rb index 33c02b4..0724a51 100644 --- a/lib/rubygems/remote_fetcher.rb +++ b/lib/rubygems/remote_fetcher.rb @@ -4,8 +4,7 @@ require_relative 'request' https://github.com/ruby/ruby/blob/trunk/lib/rubygems/remote_fetcher.rb#L4 require_relative 'request/connection_pools' require_relative 's3_uri_signer' require_relative 'uri_formatter' -require_relative 'uri_parser' -require_relative 'printable_uri' +require_relative 'uri' require_relative 'user_interaction' ## @@ -26,12 +25,12 @@ class Gem::RemoteFetcher https://github.com/ruby/ruby/blob/trunk/lib/rubygems/remote_fetcher.rb#L25 attr_accessor :uri, :original_uri def initialize(message, uri) - @original_uri = uri.dup - uri = Gem::PrintableUri.parse_uri(uri) + uri = Gem::Uri.new(uri) - super(uri.valid_uri? && uri.original_password ? message.sub(uri.original_password, 'REDACTED') : message) + super uri.redact_credentials_from(message) - @uri = uri.to_s + @original_uri = uri.to_s + @uri = uri.redacted.to_s end def to_s # :nodoc: @@ -127,7 +126,7 @@ class Gem::RemoteFetcher https://github.com/ruby/ruby/blob/trunk/lib/rubygems/remote_fetcher.rb#L126 require "fileutils" FileUtils.mkdir_p cache_dir rescue nil unless File.exist? cache_dir - source_uri = Gem::UriParser.parse_uri(source_uri) + source_uri = Gem::Uri.new(source_uri) scheme = source_uri.scheme @@ -222,7 +221,7 @@ class Gem::RemoteFetcher https://github.com/ruby/ruby/blob/trunk/lib/rubygems/remote_fetcher.rb#L221 unless location = response['Location'] raise FetchError.new("redirecting but no redirect location was given", uri) end - location = Gem::UriParser.parse_uri location + location = Gem::Uri.new location if https?(uri) && !https?(location) raise FetchError.new("redirecting to non-https resource: #{location}", uri) @@ -240,7 +239,7 @@ class Gem::RemoteFetcher https://github.com/ruby/ruby/blob/trunk/lib/rubygems/remote_fetcher.rb#L239 # Downloads +uri+ and returns it as a String. def fetch_path(uri, mtime = nil, head = false) - uri = Gem::UriParser.parse_uri uri + uri = Gem::Uri.new uri unless uri.scheme raise ArgumentError, "uri scheme is invalid: #{uri.scheme.inspect}" diff --git a/lib/rubygems/request.rb b/lib/rubygems/request.rb index fdc4c55..72e25e2 100644 --- a/lib/rubygems/request.rb +++ b/lib/rubygems/request.rb @@ -184,7 +184,6 @@ class Gem::Request https://github.com/ruby/ruby/blob/trunk/lib/rubygems/request.rb#L184 def perform_request(request) # :nodoc: connection = connection_for @uri - uri = Gem::PrintableUri.parse_uri(@uri) retried = false bad_response = false @@ -192,7 +191,7 @@ class Gem::Request https://github.com/ruby/ruby/blob/trunk/lib/rubygems/request.rb#L191 begin @requests[connection.object_id] += 1 - verbose "#{request.method} #{uri}" + verbose "#{request.method} #{Gem::Uri.new(@uri).redacted}" file_name = File.basename(@uri.path) # perform download progress reporter only for gems diff --git a/lib/rubygems/uri.rb b/lib/rubygems/uri.rb new file mode 100644 index 0000000..031d7e0 --- /dev/null +++ b/lib/rubygems/uri.rb @@ -0,0 +1,102 @@ https://github.com/ruby/ruby/blob/trunk/lib/rubygems/uri.rb#L1 +# frozen_string_literal: true + +## +# The Uri handles rubygems source URIs. +# + +class Gem::Uri + def initialize(source_uri) + @parsed_uri = parse(source_uri) + end + + def redacted + return self unless valid_uri? + + if token? || oauth_basic? + with_redacted_user + elsif password? + with_redacted_password + else + self + end + end + + def to_s + @parsed_uri.to_s + end + + def redact_credentials_from(text) + return text unless valid_uri? && password? + + text.sub(password, 'REDACTED') + end + + def method_missing(method_name, *args, &blk) + if @parsed_uri.respond_to?(method_name) + @parsed_uri.send(method_name, *args, &blk) + else + super + end + end + + def respond_to_missing?(method_name, include_private = false) + @parsed_uri.respond_to?(method_name, include_private) || super + end + + private + + ## + # Parses the #uri, raising if it's invalid + + def parse!(uri) + require "uri" + + raise URI::InvalidURIError 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.parse(uri) + rescue URI::InvalidURIError + URI.parse(URI::DEFAULT_PARSER.escape(uri)) + end + end + + ## + # Parses the #uri, returning the original uri if it's invalid + + def parse(uri) + return uri unless uri.is_a?(String) + + parse!(uri) + rescue URI::InvalidURIError + uri + end + + def with_redacted_user + clone.tap {|uri| uri.user = 'REDACTED' } + end + + def with_redacted_password + clone.tap {|uri| uri.password = 'REDACTED' } + end + + def valid_uri? + !@parsed_uri.is_a?(String) + end + + def password? + !!password + end + + def oauth_basic? + password == 'x-oauth-basic' + end + + def token? + !user.nil? && password.nil? + end +end diff --git a/lib/rubygems/uri_parser.rb b/lib/rubygems/uri_parser.rb deleted file mode 100644 index f51d77a..0000000 --- a/lib/rubygems/uri_parser.rb +++ /dev/null @@ -1,42 +0,0 @@ https://github.com/ruby/ruby/blob/trunk/lib/rubygems/uri.rb#L0 -# frozen_string_literal: true - -## -# The UriParser handles parsing URIs. -# - -class Gem::UriParser - def self.parse_uri(source_uri) - return source_uri unless source_uri.is_a?(String) - - new.parse(source_uri) - end - - ## - # Parses the #uri, raising if it's invalid - - def parse!(uri) - require "uri" - - raise URI::InvalidURIError unless uri - - # Always escape URI's to deal with (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/