Project

General

Profile

Bug #11091

Symbolized Strings May Break Keyword Arguments

Added by ds-makandra (Dominik Schöler) over 2 years ago. Updated about 2 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
ruby -v:
ruby 2.2.2p95 (2015-04-13 revision 50295) [x86_64-linux]
[ruby-core:68970]

Description

See https://makandracards.com/makandra/32333-bugreport-symbolized-strings-break-keyword-arguments-in-ruby-2-2.


TL;DR Under certain circumstances, dynamically defined symbols may break keyword arguments in Ruby 2.2.

Specifically, when

  • there is a method with several keyword arguments and a double-splat argument (e.g. def m(foo: 'bar, option: 'will be lost', **further_options))
  • there is a dynamically created Symbol (e.g. 'culprit'.to_sym) that is created before the method is parsed
  • the method gets called with both the option and a culprit keyword argument

then the option keyword argument will be nil inside of #m.

Affected Ruby Versions

  • Affected version: 2.2.1, 2.2.2
  • Unaffected versions: 1.x, 2.0, 2.1

How to Expose it

The following code exposes the bug. Save it, make sure you have Ruby 2.2 and run it.

test_symbol = '__test'

# The bug only occurs when a symbol used in a keyword argument is dynamically
# added to the Ruby symbols table *before* Ruby first sees the keyword argument.
existing = Symbol.all_symbols.map(&:to_s).grep('__test')
raise "Symbol #{test_symbol} already exists in symbol table!" if existing.any?

'__test'.to_sym # breaks it
# :__test # does not break it

# GC.start # fixes it

# Why #eval?
# Without, Ruby would parse the symbols in this code into its symbol table
# before running the file, which prevents the bug.
eval <<-RUBY
  $hash = { __test: '__test', lost: 'lost', q: 'q' }

  def _report(name, value)
    puts name.to_s << ': ' << (value ? 'ok' : 'broken')
  end

  # Confirmed broken when:
  # - `lost` is the second keyword argument Oo
  # - there is a double-splat argument
  def vulnerable_method_1(p: 'p', lost: 'lost', **options)
    _report(__method__, lost)
  end

  def vulnerable_method_2(p: 'p', lost: 'lost', q: 'q', **options)
    _report(__method__, lost)
  end

  def immune_method_1(lost: 'lost', p: 'p', **options)
    _report(__method__, lost)
  end

  def immune_method_2(q: 'q', lost: 'lost', __test: '__test')
    _report(__method__, lost)
  end

  def immune_method_3(lost: 'lost', **options)
    _report(__method__, lost)
  end
RUBY

# Exposure #####################################################################

puts '', 'Broken when calling with a hash'
vulnerable_method_1($hash)
vulnerable_method_2($hash)
immune_method_1($hash)
immune_method_2($hash)
immune_method_3($hash)

puts '', 'Double splat (**) has no influence:'
vulnerable_method_1(**$hash)
vulnerable_method_2(**$hash)
immune_method_1(**$hash)
immune_method_2(**$hash)
immune_method_3(**$hash)

puts '', 'Hash order does not matter:'
inversed_hash = Hash[$hash.to_a.reverse]
vulnerable_method_1(inversed_hash)
vulnerable_method_2(inversed_hash)
immune_method_1(inversed_hash)
immune_method_2(inversed_hash)
immune_method_3(inversed_hash)

References

symbol_bug.rb (1.92 KB) symbol_bug.rb ds-makandra (Dominik Schöler), 04/23/2015 02:20 PM
test_case.rb (482 Bytes) test_case.rb danfinnie (Daniel Finnie), 05/04/2015 04:18 PM

Related issues

Is duplicate of Ruby trunk - Bug #11027: Named Argument assignment from Hash failureClosed

History

#1 [ruby-core:68971] Updated by ds-makandra (Dominik Schöler) over 2 years ago

It appears to be fixed in 2.3-dev (trunk).

#2 Updated by nobu (Nobuyoshi Nakada) over 2 years ago

  • Is duplicate of Bug #11027: Named Argument assignment from Hash failure added

#3 [ruby-core:68978] Updated by nobu (Nobuyoshi Nakada) over 2 years ago

  • Status changed from Open to Closed
  • Backport changed from 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN to 2.0.0: DONTNEED, 2.1: DONTNEED, 2.2: REQUIRED

#4 [ruby-core:69072] Updated by danfinnie (Daniel Finnie) about 2 years ago

We just ran into this bug as well. Our script to reproduce is attached. We also noticed the same requirements as the original poster: multiple required keyword arguments, a **splat, and a dynamically created symbol.

We ran the script below in 2.3.0-dev, where it seems to be fixed. We're still seeing the behavior in 2.2.2 (ruby 2.2.2p95 (2015-04-13 revision 50295) [x86_64-darwin13]). Will this fix be backported?

#5 [ruby-core:69075] Updated by nobu (Nobuyoshi Nakada) about 2 years ago

  • Description updated (diff)

Yes, see "Backport" field.

#6 [ruby-core:69587] Updated by nagachika (Tomoyuki Chikanaga) about 2 years ago

  • Backport changed from 2.0.0: DONTNEED, 2.1: DONTNEED, 2.2: REQUIRED to 2.0.0: DONTNEED, 2.1: DONTNEED, 2.2: DONE

already backported into ruby_2_2 branch at r50562. see #11027.

Also available in: Atom PDF