Merge branch 'dm-gitlab-http-blocked-url-error' into 'master'

Raise more descriptive errors when URLs are blocked

See merge request gitlab-org/gitlab-ce!18058

4 files changed, 13 insertions, 13 deletions

diff --git a/spec/lib/gitlab/http_spec.rb b/spec/lib/gitlab/http_spec.rb
index b0bc081a3c8..d0dadfa78da 100644
--- a/spec/lib/gitlab/http_spec.rb
+++ b/spec/lib/gitlab/http_spec.rb
@@ -12,11 +12,11 @@ describe Gitlab::HTTP do
       end
 
       it 'deny requests to localhost' do
-        expect { described_class.get('http://localhost:3003') }.to raise_error(URI::InvalidURIError)
+        expect { described_class.get('http://localhost:3003') }.to raise_error(Gitlab::HTTP::BlockedUrlError)
       end
 
       it 'deny requests to private network' do
-        expect { described_class.get('http://192.168.1.2:3003') }.to raise_error(URI::InvalidURIError)
+        expect { described_class.get('http://192.168.1.2:3003') }.to raise_error(Gitlab::HTTP::BlockedUrlError)
       end
 
       context 'if allow_local_requests set to true' do
@@ -41,7 +41,7 @@ describe Gitlab::HTTP do
 
       context 'if allow_local_requests set to false' do
         it 'override the global value and ban requests to localhost or private network' do
-          expect { described_class.get('http://localhost:3003', allow_local_requests: false) }.to raise_error(URI::InvalidURIError)
+          expect { described_class.get('http://localhost:3003', allow_local_requests: false) }.to raise_error(Gitlab::HTTP::BlockedUrlError)
         end
       end
     end
diff --git a/spec/lib/gitlab/url_blocker_spec.rb b/spec/lib/gitlab/url_blocker_spec.rb
index 2d35b026485..a3b3dc3be6d 100644
--- a/spec/lib/gitlab/url_blocker_spec.rb
+++ b/spec/lib/gitlab/url_blocker_spec.rb
@@ -74,13 +74,13 @@ describe Gitlab::UrlBlocker do
       expect(described_class.blocked_url?('https://gitlab.com/foo/foo.git')).to be false
     end
 
-    context 'when allow_private_networks is' do
-      let(:private_networks) { ['192.168.1.2', '10.0.0.2', '172.16.0.2'] }
+    context 'when allow_local_network is' do
+      let(:local_ips) { ['192.168.1.2', '10.0.0.2', '172.16.0.2'] }
       let(:fake_domain) { 'www.fakedomain.fake' }
 
       context 'true (default)' do
         it 'does not block urls from private networks' do
-          private_networks.each do |ip|
+          local_ips.each do |ip|
             stub_domain_resolv(fake_domain, ip)
 
             expect(described_class).not_to be_blocked_url("http://#{fake_domain}")
@@ -94,14 +94,14 @@ describe Gitlab::UrlBlocker do
 
       context 'false' do
         it 'blocks urls from private networks' do
-          private_networks.each do |ip|
+          local_ips.each do |ip|
             stub_domain_resolv(fake_domain, ip)
 
-            expect(described_class).to be_blocked_url("http://#{fake_domain}", allow_private_networks: false)
+            expect(described_class).to be_blocked_url("http://#{fake_domain}", allow_local_network: false)
 
             unstub_domain_resolv
 
-            expect(described_class).to be_blocked_url("http://#{ip}", allow_private_networks: false)
+            expect(described_class).to be_blocked_url("http://#{ip}", allow_local_network: false)
           end
         end
       end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 96adf64bcec..fef868ac0f2 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -224,14 +224,14 @@ describe Project do
       project2 = build(:project, import_url: 'http://localhost:9000/t.git')
 
       expect(project2).to be_invalid
-      expect(project2.errors[:import_url]).to include('imports are not allowed from that URL')
+      expect(project2.errors[:import_url].first).to include('Requests to localhost are not allowed')
     end
 
     it "does not allow blocked import_url port" do
       project2 = build(:project, import_url: 'http://github.com:25/t.git')
 
       expect(project2).to be_invalid
-      expect(project2.errors[:import_url]).to include('imports are not allowed from that URL')
+      expect(project2.errors[:import_url].first).to include('Only allowed ports are 22, 80, 443')
     end
 
     describe 'project pending deletion' do
diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb
index bf7facaec99..30c89ebd821 100644
--- a/spec/services/projects/import_service_spec.rb
+++ b/spec/services/projects/import_service_spec.rb
@@ -156,7 +156,7 @@ describe Projects::ImportService do
         result = described_class.new(project, user).execute
 
         expect(result[:status]).to eq :error
-        expect(result[:message]).to end_with 'Blocked import URL.'
+        expect(result[:message]).to include('Requests to localhost are not allowed')
       end
 
       it 'fails with port 25' do
@@ -165,7 +165,7 @@ describe Projects::ImportService do
         result = described_class.new(project, user).execute
 
         expect(result[:status]).to eq :error
-        expect(result[:message]).to end_with 'Blocked import URL.'
+        expect(result[:message]).to include('Only allowed ports are 22, 80, 443')
       end
     end