From 75a556a127eb7a1b81e1fe6f7e8560f976ba311d Mon Sep 17 00:00:00 2001 From: Roman Sandler Date: Fri, 22 Apr 2016 10:13:29 +1000 Subject: Do not log the credentials used to contact a gem server Adds a filter_uri method to HTTPError backed by the URICredentialsFilter to be used when preparing error output. In the tests, replace a double object with a real URI and change a test hostname to be valid so that older versions of Ruby's URI module don't choke on it. It would be cool to somehow replace this work with the `anonymized_uri` in the Bundler::Source::Rubygems::Remote class. --- lib/bundler/errors.rb | 7 ++++++- lib/bundler/fetcher.rb | 3 +++ lib/bundler/fetcher/downloader.rb | 2 +- spec/bundler/fetcher/index_spec.rb | 20 +++++++++----------- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/lib/bundler/errors.rb b/lib/bundler/errors.rb index 69eb57e8..1f0f5f82 100644 --- a/lib/bundler/errors.rb +++ b/lib/bundler/errors.rb @@ -30,7 +30,12 @@ module Bundler class GemspecError < BundlerError; status_code(14); end class InvalidOption < BundlerError; status_code(15); end class ProductionError < BundlerError; status_code(16); end - class HTTPError < BundlerError; status_code(17); end + class HTTPError < BundlerError + status_code(17) + def filter_uri(uri) + URICredentialsFilter.credential_filtered_uri(uri) + end + end class RubyVersionMismatch < BundlerError; status_code(18); end class SecurityError < BundlerError; status_code(19); end class LockfileError < BundlerError; status_code(20); end diff --git a/lib/bundler/fetcher.rb b/lib/bundler/fetcher.rb index 19611b17..ce9d30c1 100644 --- a/lib/bundler/fetcher.rb +++ b/lib/bundler/fetcher.rb @@ -19,6 +19,7 @@ module Bundler # This is the error raised if OpenSSL fails the cert verification class CertificateFailureError < HTTPError def initialize(remote_uri) + remote_uri = filter_uri(remote_uri) super "Could not verify the SSL certificate for #{remote_uri}.\nThere" \ " is a chance you are experiencing a man-in-the-middle attack, but" \ " most likely your system doesn't have the CA certificates needed" \ @@ -39,6 +40,7 @@ module Bundler # This error is raised if HTTP authentication is required, but not provided. class AuthenticationRequiredError < HTTPError def initialize(remote_uri) + remote_uri = filter_uri(remote_uri) super "Authentication is required for #{remote_uri}.\n" \ "Please supply credentials for this source. You can do this by running:\n" \ " bundle config #{remote_uri} username:password" @@ -47,6 +49,7 @@ module Bundler # This error is raised if HTTP authentication is provided, but incorrect. class BadAuthenticationError < HTTPError def initialize(remote_uri) + remote_uri = filter_uri(remote_uri) super "Bad username or password for #{remote_uri}.\n" \ "Please double-check your credentials and correct them." end diff --git a/lib/bundler/fetcher/downloader.rb b/lib/bundler/fetcher/downloader.rb index 204e3338..a4ba4f3a 100644 --- a/lib/bundler/fetcher/downloader.rb +++ b/lib/bundler/fetcher/downloader.rb @@ -58,7 +58,7 @@ module Bundler raise NetworkDownError, "Could not reach host #{uri.host}. Check your network " \ "connection and try again." else - raise HTTPError, "Network error while fetching #{uri}" + raise HTTPError, "Network error while fetching #{URICredentialsFilter.credential_filtered_uri(uri)}" end end end diff --git a/spec/bundler/fetcher/index_spec.rb b/spec/bundler/fetcher/index_spec.rb index e22dc143..f81a655c 100644 --- a/spec/bundler/fetcher/index_spec.rb +++ b/spec/bundler/fetcher/index_spec.rb @@ -19,6 +19,11 @@ describe Bundler::Fetcher::Index do context "error handling" do shared_examples_for "the error is properly handled" do + let(:remote_uri) { URI("http://remote-uri.org") } + before do + allow(subject).to receive(:remote_uri).and_return(remote_uri) + end + context "when certificate verify failed" do let(:error_message) { "certificate verify failed" } @@ -30,25 +35,18 @@ describe Bundler::Fetcher::Index do context "when a 401 response occurs" do let(:error_message) { "401" } - let(:remote_uri) { double(:remote_uri, :to_s => "http://remote_uri.org") } - - before { allow(subject).to receive(:remote_uri).and_return(remote_uri) } it "should raise a Bundler::Fetcher::AuthenticationRequiredError" do expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::AuthenticationRequiredError, - %r{Authentication is required for http://remote_uri.org}) + %r{Authentication is required for http://remote-uri.org}) end end context "when a 403 response occurs" do - let(:error_message) { "403" } - let(:remote_uri) { double(:remote_uri) } - let(:remote_uri_string) { "http://remote_uri.org" } + let(:error_message) { "403" } before do - allow(subject).to receive(:remote_uri).and_return(remote_uri) allow(remote_uri).to receive(:userinfo).and_return(userinfo) - allow(remote_uri).to receive(:to_s).and_return(remote_uri_string) end context "and there was userinfo" do @@ -56,7 +54,7 @@ describe Bundler::Fetcher::Index do it "should raise a Bundler::Fetcher::BadAuthenticationError" do expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::BadAuthenticationError, - %r{Bad username or password for http://remote_uri.org}) + %r{Bad username or password for http://remote-uri.org}) end end @@ -65,7 +63,7 @@ describe Bundler::Fetcher::Index do it "should raise a Bundler::Fetcher::AuthenticationRequiredError" do expect { subject.specs(gem_names) }.to raise_error(Bundler::Fetcher::AuthenticationRequiredError, - %r{Authentication is required for http://remote_uri.org}) + %r{Authentication is required for http://remote-uri.org}) end end end -- cgit v1.2.3