[31583] avoid parsing strings to yaml

If a configuration/ENV value has YML-incompatible contents, it is broken
due to the `YAML.load` approach.

However, if the outcome of `YAML.load` is a string, we can expect the
input to be an actual string, not something YAML should parse.

Thus as a workaround until proper configuration types are added (or
better, settings and configuration is joined into one), this avoids
breaking passwords in ENV configuration inputs.

https://community.openproject.com/wp/31583
pull/7843/head
Oliver Günther 5 years ago
parent 859233ebb7
commit 269d91cfb9
No known key found for this signature in database
GPG Key ID: A3A8BDAD7C0C552C
  1. 14
      lib/open_project/configuration.rb
  2. 11
      spec/lib/open_project/configuration_spec.rb

@ -406,15 +406,21 @@ module OpenProject
# using YAML. # using YAML.
# #
# @param key [String] The key of the input within the source hash. # @param key [String] The key of the input within the source hash.
# @param value [String] The string from which to extract the actual value. # @param original_value [String] The string from which to extract the actual value.
# @return A ruby object (e.g. Integer, Float, String, Hash, Boolean, etc.) # @return A ruby object (e.g. Integer, Float, String, Hash, Boolean, etc.)
# @raise [ArgumentError] If the string could not be parsed. # @raise [ArgumentError] If the string could not be parsed.
def extract_value(key, value) def extract_value(key, original_value)
# YAML parses '' as false, but empty ENV variables will be passed as that. # YAML parses '' as false, but empty ENV variables will be passed as that.
# To specify specific values, one can use !!str (-> '') or !!null (-> nil) # To specify specific values, one can use !!str (-> '') or !!null (-> nil)
return value if value == '' return original_value if original_value == ''
YAML.load(value) parsed = YAML.load(original_value)
if parsed.is_a?(String)
original_value
else
parsed
end
rescue StandardError => e rescue StandardError => e
raise ArgumentError, "Configuration value for '#{key}' is invalid: #{e.message}" raise ArgumentError, "Configuration value for '#{key}' is invalid: #{e.message}"
end end

@ -99,6 +99,7 @@ describe OpenProject::Configuration do
'nil' => 'foobar', 'nil' => 'foobar',
'str_empty' => 'foobar', 'str_empty' => 'foobar',
'somesetting' => 'foo', 'somesetting' => 'foo',
'invalid_yaml' => nil,
'some_list_entry' => nil, 'some_list_entry' => nil,
'nested' => { 'nested' => {
'key' => 'value', 'key' => 'value',
@ -120,7 +121,7 @@ describe OpenProject::Configuration do
'SOMEEMPTYSETTING' => '', 'SOMEEMPTYSETTING' => '',
'SOMESETTING' => 'bar', 'SOMESETTING' => 'bar',
'NIL' => '!!null', 'NIL' => '!!null',
'STR_EMPTY' => '!!str', 'INVALID_YAML' => "'foo'! #234@@½%%%",
'OPTEST_SOME__LIST__ENTRY' => '[foo, bar , xyz, whut wat]', 'OPTEST_SOME__LIST__ENTRY' => '[foo, bar , xyz, whut wat]',
'OPTEST_NESTED_KEY' => 'baz', 'OPTEST_NESTED_KEY' => 'baz',
'OPTEST_NESTED_DEEPLY__NESTED_KEY' => '42', 'OPTEST_NESTED_DEEPLY__NESTED_KEY' => '42',
@ -135,6 +136,10 @@ describe OpenProject::Configuration do
OpenProject::Configuration.send :override_config!, config, env_vars OpenProject::Configuration.send :override_config!, config, env_vars
end end
it 'returns the original string, not the invalid YAML one' do
expect(config['invalid_yaml']).to eq env_vars['INVALID_YAML']
end
it 'should not parse the empty value' do it 'should not parse the empty value' do
expect(config['someemptysetting']).to eq('') expect(config['someemptysetting']).to eq('')
end end
@ -143,10 +148,6 @@ describe OpenProject::Configuration do
expect(config['nil']).to be_nil expect(config['nil']).to be_nil
end end
it 'should parse the empty string' do
expect(config['str_empty']).to eq('')
end
it 'should override the previous setting value' do it 'should override the previous setting value' do
expect(config['somesetting']).to eq('bar') expect(config['somesetting']).to eq('bar')
end end

Loading…
Cancel
Save