Merge pull request #322 from rhenium/ky/config-deprecate-modify

config: deprecate OpenSSL::Config#add_value and #[]=

History.md

@@ -7,6 +7,8 @@ Compatibility notes
 * Remove unsupported MDC2, DSS, DSS1, and SHA algorithms.
 * Remove `OpenSSL::PKCS7::SignerInfo#name` alias for `#issuer`.
  [[GitHub #266]](https://github.com/ruby/openssl/pull/266)
+* Deprecate `OpenSSL::Config#add_value` and `#[]=` for future removal.
+ [[GitHub #322]](https://github.com/ruby/openssl/pull/322)
 
 
 Notable changes

lib/openssl/config.rb

@@ -37,7 +37,7 @@ class << self
  def parse(string)
  c = new()
  parse_config(StringIO.new(string)).each do |section, hash|
- c[section] = hash
+ c.set_section(section, hash)
  end
  c
  end
@@ -248,7 +248,7 @@ def initialize(filename = nil)
  if filename
  File.open(filename.to_s) do |file|
  Config.parse_config(file).each do |section, hash|
- self[section] = hash
+ set_section(section, hash)
  end
  end
  end
@@ -297,6 +297,8 @@ def value(arg1, arg2 = nil) # :nodoc:
  end
 
  ##
+ # *Deprecated in v2.2.0*. This method will be removed in a future release.
+ #
  # Set the target _key_ with a given _value_ under a specific _section_.
  #
  # Given the following configurating file being loaded:
@@ -351,6 +353,8 @@ def section(name) # :nodoc:
  end
 
  ##
+ # *Deprecated in v2.2.0*. This method will be removed in a future release.
+ #
  # Sets a specific _section_ name with a Hash _pairs_.
  #
  # Given the following configuration being created:
@@ -376,9 +380,13 @@ def section(name) # :nodoc:
  #
  def []=(section, pairs)
  check_modify
- @data[section] ||= {}
+ set_section(section, pairs)
+ end
+
+ def set_section(section, pairs) # :nodoc:
+ hash = @data[section] ||= {}
  pairs.each do |key, value|
- self.add_value(section, key, value)
+ hash[key] = value
  end
  end
 
@@ -463,6 +471,8 @@ def initialize_copy(other)
  end
 
  def check_modify
+ warn "#{caller(2, 1)[0]}: warning: do not modify OpenSSL::Config; this " \
+ "method is deprecated and will be removed in a future release."
  raise TypeError.new("Insecure: can't modify OpenSSL config") if frozen?
  end
 

test/openssl/test_config.rb

@@ -211,45 +211,54 @@ def test_section
 
  def test_sections
  assert_equal(['CA_default', 'ca', 'default'], @it.sections.sort)
- @it['new_section'] = {'foo' => 'bar'}
- assert_equal(['CA_default', 'ca', 'default', 'new_section'], @it.sections.sort)
- @it['new_section'] = {}
- assert_equal(['CA_default', 'ca', 'default', 'new_section'], @it.sections.sort)
+ # OpenSSL::Config#[]= is deprecated
+ EnvUtil.suppress_warning do
+ @it['new_section'] = {'foo' => 'bar'}
+ assert_equal(['CA_default', 'ca', 'default', 'new_section'], @it.sections.sort)
+ @it['new_section'] = {}
+ assert_equal(['CA_default', 'ca', 'default', 'new_section'], @it.sections.sort)
+ end
  end
 
  def test_add_value
- c = OpenSSL::Config.new
- assert_equal("", c.to_s)
- # add key
- c.add_value('default', 'foo', 'bar')
- assert_equal("[ default ]\nfoo=bar\n\n", c.to_s)
- # add another key
- c.add_value('default', 'baz', 'qux')
- assert_equal('bar', c['default']['foo'])
- assert_equal('qux', c['default']['baz'])
- # update the value
- c.add_value('default', 'baz', 'quxxx')
- assert_equal('bar', c['default']['foo'])
- assert_equal('quxxx', c['default']['baz'])
- # add section and key
- c.add_value('section', 'foo', 'bar')
- assert_equal('bar', c['default']['foo'])
- assert_equal('quxxx', c['default']['baz'])
- assert_equal('bar', c['section']['foo'])
+ # OpenSSL::Config#add_value is deprecated
+ EnvUtil.suppress_warning do
+ c = OpenSSL::Config.new
+ assert_equal("", c.to_s)
+ # add key
+ c.add_value('default', 'foo', 'bar')
+ assert_equal("[ default ]\nfoo=bar\n\n", c.to_s)
+ # add another key
+ c.add_value('default', 'baz', 'qux')
+ assert_equal('bar', c['default']['foo'])
+ assert_equal('qux', c['default']['baz'])
+ # update the value
+ c.add_value('default', 'baz', 'quxxx')
+ assert_equal('bar', c['default']['foo'])
+ assert_equal('quxxx', c['default']['baz'])
+ # add section and key
+ c.add_value('section', 'foo', 'bar')
+ assert_equal('bar', c['default']['foo'])
+ assert_equal('quxxx', c['default']['baz'])
+ assert_equal('bar', c['section']['foo'])
+ end
  end
 
  def test_aset
- @it['foo'] = {'bar' => 'baz'}
- assert_equal({'bar' => 'baz'}, @it['foo'])
- @it['foo'] = {'bar' => 'qux', 'baz' => 'quxx'}
- assert_equal({'bar' => 'qux', 'baz' => 'quxx'}, @it['foo'])
-
- # OpenSSL::Config is add only for now.
- @it['foo'] = {'foo' => 'foo'}
- assert_equal({'foo' => 'foo', 'bar' => 'qux', 'baz' => 'quxx'}, @it['foo'])
- # you cannot override or remove any section and key.
- @it['foo'] = {}
- assert_equal({'foo' => 'foo', 'bar' => 'qux', 'baz' => 'quxx'}, @it['foo'])
+ # OpenSSL::Config#[]= is deprecated
+ EnvUtil.suppress_warning do
+ @it['foo'] = {'bar' => 'baz'}
+ assert_equal({'bar' => 'baz'}, @it['foo'])
+ @it['foo'] = {'bar' => 'qux', 'baz' => 'quxx'}
+ assert_equal({'bar' => 'qux', 'baz' => 'quxx'}, @it['foo'])
+
+ # OpenSSL::Config is add only for now.
+ @it['foo'] = {'foo' => 'foo'}
+ assert_equal({'foo' => 'foo', 'bar' => 'qux', 'baz' => 'quxx'}, @it['foo'])
+ # you cannot override or remove any section and key.
+ @it['foo'] = {}
+ assert_equal({'foo' => 'foo', 'bar' => 'qux', 'baz' => 'quxx'}, @it['foo'])
+ end
  end
 
  def test_each
@@ -272,32 +281,39 @@ def test_inspect
  end
 
  def test_freeze
- c = OpenSSL::Config.new
- c['foo'] = [['key', 'value']]
- c.freeze
+ @it.freeze
 
- bug = '[ruby-core:18377]'
- # RuntimeError for 1.9, TypeError for 1.8
- e = assert_raise(TypeError, bug) do
- c['foo'] = [['key', 'wrong']]
+ # Modifying OpenSSL::Config produces a warning
+ EnvUtil.suppress_warning do
+ bug = '[ruby-core:18377]'
+ # RuntimeError for 1.9, TypeError for 1.8
+ e = assert_raise(TypeError, bug) do
+ @it['foo'] = [['key', 'wrong']]
+ end
+ assert_match(/can't modify/, e.message, bug)
  end
- assert_match(/can't modify/, e.message, bug)
  end
 
  def test_dup
  assert(!@it.sections.empty?)
  c = @it.dup
  assert_equal(@it.sections.sort, c.sections.sort)
- @it['newsection'] = {'a' => 'b'}
- assert_not_equal(@it.sections.sort, c.sections.sort)
+ # OpenSSL::Config#[]= is deprecated
+ EnvUtil.suppress_warning do
+ @it['newsection'] = {'a' => 'b'}
+ assert_not_equal(@it.sections.sort, c.sections.sort)
+ end
  end
 
  def test_clone
  assert(!@it.sections.empty?)
  c = @it.clone
  assert_equal(@it.sections.sort, c.sections.sort)
- @it['newsection'] = {'a' => 'b'}
- assert_not_equal(@it.sections.sort, c.sections.sort)
+ # OpenSSL::Config#[]= is deprecated
+ EnvUtil.suppress_warning do
+ @it['newsection'] = {'a' => 'b'}
+ assert_not_equal(@it.sections.sort, c.sections.sort)
+ end
  end
 end