Commit 1c87bacb authored by Nick Thomas's avatar Nick Thomas

Fix a regression breaking projects with an empty import URL

parent 0ae313c4
......@@ -29,9 +29,7 @@ class AddressableUrlValidator < ActiveModel::EachValidator
private
def valid_url?(value)
return false unless value
valid_protocol?(value) && valid_uri?(value)
valid_uri?(value) && valid_protocol?(value)
end
def valid_uri?(value)
......
---
title: Fix a regression breaking projects with an empty import URL
merge_request: 2824
author:
type: fixed
......@@ -384,7 +384,8 @@ module EE
def username_only_import_url=(value)
unless ::Gitlab::UrlSanitizer.valid?(value)
self.import_url = value
return
self.import_data&.user = nil
return value
end
url = ::Gitlab::UrlSanitizer.new(value)
......
......@@ -9,7 +9,7 @@ module Gitlab
end
def self.valid?(url)
return false unless url
return false unless url.present?
Addressable::URI.parse(url.strip)
......
......@@ -814,28 +814,24 @@ describe Project do
end
describe '#username_only_import_url' do
def build_project(username: 'user', password: 'password')
build(:project, import_url: 'http://example.com').tap do |project|
project.build_import_data(credentials: { user: username, password: password })
end
end
where(:import_url, :username, :expected_import_url) do
'' | 'foo' | ''
'' | '' | ''
'' | nil | ''
it 'shows the bare url when no username is present' do
project = build_project(username: nil)
nil | 'foo' | nil
nil | '' | nil
nil | nil | nil
expect(project.username_only_import_url).to eq('http://example.com')
'http://example.com' | 'foo' | 'http://foo@example.com'
'http://example.com' | '' | 'http://example.com'
'http://example.com' | nil | 'http://example.com'
end
it 'shows the URL with username when present' do
project = build_project(password: nil)
with_them do
let(:project) { build(:project, import_url: import_url, import_data_attributes: { user: username, password: 'password' }) }
expect(project.username_only_import_url).to eq('http://user@example.com')
end
it 'excludes the pasword when present' do
project = build_project
expect(project.username_only_import_url).to eq('http://user@example.com')
it { expect(project.username_only_import_url).to eq(expected_import_url) }
end
end
......@@ -855,5 +851,15 @@ describe Project do
expect(project.import_url).to eq('http://user:pass@example.com')
expect(project.import_data.password).to eq('pass')
end
it 'clears the username if passed the empty string' do
project = build(:project, import_url: 'http://olduser:pass@old.example.com')
project.username_only_import_url = ''
expect(project.username_only_import_url).to eq('')
expect(project.import_url).to eq('')
expect(project.import_data.user).to be_nil
expect(project.import_data.password).to eq('pass')
end
end
end
......@@ -57,6 +57,7 @@ describe Gitlab::UrlSanitizer do
describe '.valid?' do
it 'validates url strings' do
expect(described_class.valid?(nil)).to be(false)
expect(described_class.valid?('')).to be(false)
expect(described_class.valid?('valid@project:url.git')).to be(true)
expect(described_class.valid?('123://invalid:url')).to be(false)
end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment