Project

General

Profile

Bug #11718

Constant access on `nil`

Added by vais (Vais Salikhov) over 3 years ago. Updated 4 days ago.

Status:
Closed
Priority:
Normal
Target version:
-
ruby -v:
ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-linux]
[ruby-core:71598]

Description

It is possible to access top-level constants by doing nil::CONSTANT, which looks like a bug according to Matz. Here are a couple of examples:

$ ruby -ve "Foo = 123; p nil::Foo"
ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-linux]
123
$ ruby -ve "class A; Foo = 456; end; p nil::A::Foo"
ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-linux]
456

Thanks!


Files

nil-const-access-11718.patch (5.32 KB) nil-const-access-11718.patch jeremyevans0 (Jeremy Evans), 08/14/2019 05:48 AM

Associated revisions

Revision fbcd0652
Added by jeremyevans (Jeremy Evans) 4 days ago

Remove support for nil::Constant

This was an intentional bug added in 1.9.

The approach taken here is to add a second operand to the
getconstant instruction for whether nil should be allowed and
treated as current scope.

Fixes [Bug #11718]

History

Updated by Hanmac (Hans Mackowiak) over 3 years ago

hm it might be that nil::Foo is parsed as ::Foo which is doing the top-level access.

but i am not deep enough in the parser to fix that.

Updated by Eregon (Benoit Daloze) over 3 years ago

I think this should really raise an error like:

$ ruby -e 'p nil::String'
-e:1:in `<main>': nil is not a class/module (TypeError)

Updated by Hanmac (Hans Mackowiak) over 3 years ago

yeah that should raise an error. A little test shows that this exist a while way back:

p nil::String

for 1.9.3:

String

for 1.8.7:

# is not a class/module (TypeError)

Updated by elia (Elia Schito) over 3 years ago

looks like it's somehow intended behavior

https://github.com/ruby/ruby/blob/trunk/insns.def#L179-L190

  /**
    @c variable
    @e
     Get constant variable id. If klass is Qnil, constants
     are searched in the current scope. If klass is Qfalse, constants
     are searched as top level constants. Otherwise, get constant under klass
     class or module.
    @j 定数 id の値を得る。
     klass が Qnil なら、そのスコープで得られる定数の値を得る。
     Qfalse なら、トップレベルスコープを得る。
     それ以外なら、klass クラスの下の定数を得る。
   */

apparently introduced here: https://github.com/ruby/ruby/commit/d84f9b16946bca06ce0557ebe99152d7d445c9ec to resolve bug #10943

Updated by Eregon (Benoit Daloze) over 3 years ago

I don't think it's intended, that commit should not introduce new behavior except on singleton class constants.
ko1 (Koichi Sasada): Could you confirm this is a bug?

Updated by ko1 (Koichi Sasada) over 3 years ago

This is a (intentional) bug. Because I expect nobody reveal such code :p

Updated by ko1 (Koichi Sasada) over 3 years ago

  • Assignee set to ko1 (Koichi Sasada)

Updated by ko1 (Koichi Sasada) over 3 years ago

  • This is a bug, so that anyone should not rely on this behavior.
  • This is a bug, so that this bug should be fixed
  • But not critical, so 2.3 can remain this behavior. It should be fix in a future.

Updated by Hanmac (Hans Mackowiak) over 3 years ago

@Koichi: good that you classify it, for a moment i thought that might be an feature ;P

Updated by jeremyevans0 (Jeremy Evans) 5 days ago

This bug is still present in the master branch. Attached is a patch that fixes it. It adds a second argument to the getconstant instruction. It would probably be better for performance to add a separate instruction for the case where this is needed, but I'm not sure it is worth it. With the patch:

nil::Object
# TypeError (nil is not a class/module)

Updated by ko1 (Koichi Sasada) 5 days ago

:+1: great patch! could you commit it?

Updated by nobu (Nobuyoshi Nakada) 5 days ago

I think it can be an operand of getconst than an argument on the stack, while it is always a constant.

Updated by jeremyevans0 (Jeremy Evans) 4 days ago

nobu: I agree, making it an operand instead of a stack argument makes more sense. I'm currently testing a patch for that and will commit if it passes.

#14

Updated by jeremyevans (Jeremy Evans) 4 days ago

  • Status changed from Open to Closed

Applied in changeset git|fbcd0652944568c43a6ae427960d909d62ce6a8d.


Remove support for nil::Constant

This was an intentional bug added in 1.9.

The approach taken here is to add a second operand to the
getconstant instruction for whether nil should be allowed and
treated as current scope.

Fixes [Bug #11718]

Updated by ko1 (Koichi Sasada) 4 days ago

I think it can be an operand of getconst than an argument on the stack, while it is always a constant.

No. It should not be an new operand because we need to change tools which depends on current bytecode.

Also available in: Atom PDF