Commit b904a7db authored by Robert Speicher's avatar Robert Speicher

Make `Redis::Wrapper#_raw_config` and `#fetch_config` more resilient

These two methods now handle two additional real-world possibilities:

1. `config/resque.yml` not being present
2. `config/resque.yml` being present but not containing YAML

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/34941
parent 2a5d2ecf
...@@ -33,13 +33,16 @@ module Gitlab ...@@ -33,13 +33,16 @@ module Gitlab
def _raw_config def _raw_config
return @_raw_config if defined?(@_raw_config) return @_raw_config if defined?(@_raw_config)
@_raw_config =
begin begin
@_raw_config = ERB.new(File.read(config_file_name)).result.freeze if filename = config_file_name
ERB.new(File.read(filename)).result.freeze
else
false
end
rescue Errno::ENOENT rescue Errno::ENOENT
@_raw_config = false false
end end
@_raw_config
end end
def default_url def default_url
...@@ -116,7 +119,16 @@ module Gitlab ...@@ -116,7 +119,16 @@ module Gitlab
end end
def fetch_config def fetch_config
self.class._raw_config ? YAML.load(self.class._raw_config)[@rails_env] : false return false unless self.class._raw_config
yaml = YAML.load(self.class._raw_config)
# If the file has content but it's invalid YAML, `load` returns false
if yaml
yaml.fetch(@rails_env, false)
else
false
end
end end
end end
end end
......
...@@ -65,14 +65,6 @@ RSpec.shared_examples "redis_shared_examples" do ...@@ -65,14 +65,6 @@ RSpec.shared_examples "redis_shared_examples" do
end end
describe '.url' do describe '.url' do
it 'withstands mutation' do
url1 = described_class.url
url2 = described_class.url
url1 << 'foobar'
expect(url2).not_to end_with('foobar')
end
context 'when yml file with env variable' do context 'when yml file with env variable' do
let(:config_file_name) { config_with_environment_variable_inside } let(:config_file_name) { config_with_environment_variable_inside }
...@@ -97,6 +89,12 @@ RSpec.shared_examples "redis_shared_examples" do ...@@ -97,6 +89,12 @@ RSpec.shared_examples "redis_shared_examples" do
it 'returns false when the file does not exist' do it 'returns false when the file does not exist' do
expect(subject).to eq(false) expect(subject).to eq(false)
end end
it "returns false when the filename can't be determined" do
expect(described_class).to receive(:config_file_name).and_return(nil)
expect(subject).to eq(false)
end
end end
describe '.with' do describe '.with' do
...@@ -192,7 +190,13 @@ RSpec.shared_examples "redis_shared_examples" do ...@@ -192,7 +190,13 @@ RSpec.shared_examples "redis_shared_examples" do
it 'returns false when no config file is present' do it 'returns false when no config file is present' do
allow(described_class).to receive(:_raw_config) { false } allow(described_class).to receive(:_raw_config) { false }
expect(subject.send(:fetch_config)).to be_falsey expect(subject.send(:fetch_config)).to eq false
end
it 'returns false when config file is present but has invalid YAML' do
allow(described_class).to receive(:_raw_config) { "# development: true" }
expect(subject.send(:fetch_config)).to eq false
end end
end 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