Project

General

Profile

Actions

Feature #7795

open

Symbol.defined? and/or to_existing_symbol

Added by Student (Nathan Zook) over 11 years ago. Updated 7 months ago.

Status:
Assigned
Target version:
-
[ruby-core:51965]

Description

I'm pulling this out from deep in the discussions of issue http://bugs.ruby-lang.org/issues/7791, Let Symbols be Garbage Collected.

The problem is that the extreme utility of symbols makes them enticed to use, which results in a DOS vulnerability. My proposal is to add either of a pair of methods that would make it easy to defend against a DOS along these lines.

#1) Symbol.defined?

In existing code, it would might like this:
class Symbol
def self.defined?(string)
all_symbols.any?{|sym| sym.to_s == string}
end
end

#2) to_existing_sym. This would be defined in the same places as to_sym, but would through an argument error if the symbol did not already exist.

Actions #1

Updated by alexeymuranov (Alexey Muranov) over 11 years ago

In my opinion, it would be more useful to have a method that checks if a given string matches one of symbols in a given set. It is hard for me to think of a situation where one needs to know if a string matches any of the created symbols whatsoever. Similarly, instead of #to_existing_symbol, it seems to me useful to have a method that efficiently finds a symbol in a set by its string representation. In fact, something like String#to_one_of_symbols(set), which returns nil if no matching symbol found, would seem enough for most purposes.

Updated by alexeymuranov (Alexey Muranov) over 11 years ago

I think also that finding a symbol in a set is related to (can be used in) Hash With Indifferent Access. HWIA are implemented in RoR and Sinatra in different ways. Would be nice if Ruby itself had Hash With Indifferent Access.

Updated by phluid61 (Matthew Kerwin) over 11 years ago

On 7 February 2013 17:17, alexeymuranov (Alexey Muranov) wrote:

In my opinion, it would be more useful to have a method that checks if a
given string matches one of a symbols in a given set. It is hard for me
to think of a situation where one needs to know is a string matches any
of the created symbols whatsoever. Similarly, instead of
#to_existing_symbol, it seems to me useful to have a method that
efficiently finds a symbol in a set by its string representation.

That would be a property of the set rather than of Symbol, and can easily
be achieved by constructing a Hash whose values are the Symbols in question
and keys are the .to_s of the values. e.g.

 class SymbolSet
   def initialize
     @hash = {}
   end
   def <<(sym)
     @hash[sym.to_s] = sym
   end
   def defined?(s)
     @hash.has_key? s.to_s
   end
   def existing_sym(s)
     @hash[s.to_s] or raise "symbol :'#{s}' not defined in this set"
   end
 end

I believe the original feature request is more useful, because while it is
trivial to construct the above class (or an improved version) it is much
more difficult to manage membership, especially when you care about "the
set of all symbols", which is a legitimate concern in the case of the DOS
attack mentioned in the original request. Since the set of all symbols is
already available to Symbol (as Symbol.all_symbols), that seems the logical
place to implement the feature, if at all.

Updated by Student (Nathan Zook) over 11 years ago

When methods can be dynamically generated, things get hairy. For instance, active record 1 & 2 defined > n! dynamic finders for each model where n is the number of columns in the model. These methods are never all generated at once, but they can be.

Suppose one were to pass options to the to_json method that serializes the return data. In rails, any argumentless method on any object attached through any relationship chain can be valid. That's a lot to monitor.

Once the hash is parsed, checks can be made to see if we want to allow the method to be called. But that is only after the symbols have been created.

Thus the desire to check to see if the proposed symbol is new before creating it.

Updated by ko1 (Koichi Sasada) over 11 years ago

  • Assignee set to matz (Yukihiro Matsumoto)

Updated by matz (Yukihiro Matsumoto) over 11 years ago

I agree with the basic concept of the proposal.
I am not sure Symbol#defined? is a appropriate name for it yet.

The possible addition I like is either:

  • add Symbol#define? or similar method
  • add optional keyword argument to intern e.g. "foo".intern(exist: true)

Matz.

Updated by Student (Nathan Zook) over 11 years ago

These sound like my (new & preferred) proposal for Symbol[string] #7854.
That is, return the symbol if it already exists, nil if not.

Updated by vo.x (Vit Ondruch) over 11 years ago

Student (Nathan Zook) wrote:

#2) to_existing_sym. This would be defined in the same places as to_sym, but would through an argument error if the symbol did not already exist.

Reading the documentation, it says "Returns the Symbol corresponding to str, creating the symbol if it did not previously exist." So what would this method did different? Or is the documentation wrong?

Updated by phluid61 (Matthew Kerwin) over 11 years ago

On 22 February 2013 17:13, vo.x (Vit Ondruch) wrote:

Reading the documentation, it says "Returns the Symbol corresponding to
str, creating the symbol if it did not previously exist." So what would
this method did different? Or is the documentation wrong?

Existing method:
to_sym: Returns the Symbol corresponding to str, creating the symbol if
it did not previously exist.

New (proposed) method:
to_existing_sym: Returns the Symbol corresponding to str, raising an
Exception if it did not previously exist.

Updated by phluid61 (Matthew Kerwin) over 11 years ago

matz (Yukihiro Matsumoto) wrote:

I agree with the basic concept of the proposal.
I am not sure Symbol#defined? is a appropriate name for it yet.

The possible addition I like is either:

  • add Symbol#define? or similar method
  • add optional keyword argument to intern e.g. "foo".intern(exist: true)

Matz.

My ruby core abilities are somewhat limited as yet, but in experimentation on a local fork I have implemented "foo".to_existing_sym (which raises an error) and "foo".interned (which returns nil); https://gist.github.com/phluid61/5086304

My next goal, now that I have some familiarity in this area, would be to instead extend the existing rb_str_intern to accept the 'exist' keyword argument.

I assume it's ok that to_sym also accepts the kwarg?

Updated by Student (Nathan Zook) over 11 years ago

phluid61 (Matthew Kerwin) wrote:

matz (Yukihiro Matsumoto) wrote:

I agree with the basic concept of the proposal.
I am not sure Symbol#defined? is a appropriate name for it yet.

The possible addition I like is either:

  • add Symbol#define? or similar method
  • add optional keyword argument to intern e.g. "foo".intern(exist: true)

Matz.

My ruby core abilities are somewhat limited as yet, but in experimentation on a local fork I have implemented "foo".to_existing_sym (which raises an error) and "foo".interned (which returns nil); https://gist.github.com/phluid61/5086304

My next goal, now that I have some familiarity in this area, would be to instead extend the existing rb_str_intern to accept the 'exist' keyword argument.

I assume it's ok that to_sym also accepts the kwarg?

I agree with Matz that the names are problematic. What about Symbol[] ? (#7854)

Updated by phluid61 (Matthew Kerwin) over 11 years ago

Student (Nathan Zook) wrote:

phluid61 (Matthew Kerwin) wrote:

matz (Yukihiro Matsumoto) wrote:

I agree with the basic concept of the proposal.
I am not sure Symbol#defined? is a appropriate name for it yet.

The possible addition I like is either:

  • add Symbol#define? or similar method
  • add optional keyword argument to intern e.g. "foo".intern(exist: true)

Matz.

My ruby core abilities are somewhat limited as yet, but in experimentation on a local fork I have implemented "foo".to_existing_sym (which raises an error) and "foo".interned (which returns nil); https://gist.github.com/phluid61/5086304

My next goal, now that I have some familiarity in this area, would be to instead extend the existing rb_str_intern to accept the 'exist' keyword argument.

I assume it's ok that to_sym also accepts the kwarg?

I agree with Matz that the names are problematic. What about Symbol[] ? (#7854)

Having experimented with multiple implementations ( e.g. https://gist.github.com/phluid61/5104973 ) I agree that Symbol[] does seem like a much more useful, all-encompassing method.

Actions #13

Updated by naruse (Yui NARUSE) almost 7 years ago

  • Target version deleted (2.6)
Actions #14

Updated by hsbt (Hiroshi SHIBATA) 7 months ago

  • Status changed from Open to Assigned
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0