1 files changed, 54 insertions, 23 deletions
diff --git a/spec/controllers/concerns/internal_redirect_spec.rb b/spec/controllers/concerns/internal_redirect_spec.rb
index 97119438ca1..da68c8c8697 100644
--- a/spec/controllers/concerns/internal_redirect_spec.rb
+++ b/spec/controllers/concerns/internal_redirect_spec.rb
@@ -15,44 +15,71 @@ describe InternalRedirect do
   subject(:controller) { controller_class.new }
 
   describe '#safe_redirect_path' do
-    it 'is `nil` for invalid uris' do
-      expect(controller.safe_redirect_path('Hello world')).to be_nil
+    where(:input) do
+      [
+        'Hello world',
+        '//example.com/hello/world',
+        'https://example.com/hello/world'
+      ]
     end
 
-    it 'is `nil` for paths trying to include a host' do
-      expect(controller.safe_redirect_path('//example.com/hello/world')).to be_nil
+    with_them 'being invalid' do
+      it 'returns nil' do
+        expect(controller.safe_redirect_path(input)).to be_nil
+      end
     end
 
-    it 'returns the path if it is valid' do
-      expect(controller.safe_redirect_path('/hello/world')).to eq('/hello/world')
+    where(:input) do
+      [
+        '/hello/world',
+        '/-/ide/project/path'
+      ]
     end
 
-    it 'returns the path with querystring if it is valid' do
-      expect(controller.safe_redirect_path('/hello/world?hello=world#L123'))
-        .to eq('/hello/world?hello=world#L123')
+    with_them 'being valid' do
+      it 'returns the path' do
+        expect(controller.safe_redirect_path(input)).to eq(input)
+      end
+
+      it 'returns the path with querystring and fragment' do
+        expect(controller.safe_redirect_path("#{input}?hello=world#L123"))
+          .to eq("#{input}?hello=world#L123")
+      end
     end
   end
 
   describe '#safe_redirect_path_for_url' do
-    it 'is `nil` for invalid urls' do
-      expect(controller.safe_redirect_path_for_url('Hello world')).to be_nil
+    where(:input) do
+      [
+        'Hello world',
+        'http://example.com/hello/world',
+        'http://test.host:3000/hello/world'
+      ]
     end
 
-    it 'is `nil` for urls from a with a different host' do
-      expect(controller.safe_redirect_path_for_url('http://example.com/hello/world')).to be_nil
+    with_them 'being invalid' do
+      it 'returns nil' do
+        expect(controller.safe_redirect_path_for_url(input)).to be_nil
+      end
     end
 
-    it 'is `nil` for urls from a with a different port' do
-      expect(controller.safe_redirect_path_for_url('http://test.host:3000/hello/world')).to be_nil
+    where(:input) do
+      [
+        'http://test.host/hello/world'
+      ]
     end
 
-    it 'returns the path if the url is on the same host' do
-      expect(controller.safe_redirect_path_for_url('http://test.host/hello/world')).to eq('/hello/world')
-    end
+    with_them 'being on the same host' do
+      let(:path) { URI(input).path }
 
-    it 'returns the path including querystring if the url is on the same host' do
-      expect(controller.safe_redirect_path_for_url('http://test.host/hello/world?hello=world#L123'))
-        .to eq('/hello/world?hello=world#L123')
+      it 'returns the path' do
+        expect(controller.safe_redirect_path_for_url(input)).to eq(path)
+      end
+
+      it 'returns the path with querystring and fragment' do
+        expect(controller.safe_redirect_path_for_url("#{input}?hello=world#L123"))
+          .to eq("#{path}?hello=world#L123")
+      end
     end
   end
 
@@ -82,12 +109,16 @@ describe InternalRedirect do
   end
 
   describe '#host_allowed?' do
-    it 'allows uris with the same host and port' do
+    it 'allows URI with the same host and port' do
       expect(controller.host_allowed?(URI('http://test.host/test'))).to be(true)
     end
 
-    it 'rejects uris with other host and port' do
+    it 'rejects URI with other host' do
       expect(controller.host_allowed?(URI('http://example.com/test'))).to be(false)
     end
+
+    it 'rejects URI with other port' do
+      expect(controller.host_allowed?(URI('http://test.host:3000/test'))).to be(false)
+    end
   end
 end