Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DSL AST initialisation recursively searches up namespaces for consts #16

Open
invke opened this issue Jan 18, 2021 · 0 comments
Open

DSL AST initialisation recursively searches up namespaces for consts #16

invke opened this issue Jan 18, 2021 · 0 comments

Comments

@invke
Copy link

invke commented Jan 18, 2021

When the DSL AST initialises nodes, it performs a .const_get on the relative module. This has problems, as it will look recursively up the entire module tree for a relating const that it will then instantiate.

For example, in method_missing on Elasticsearch::DSL::Search::Query line 39, it calls const_defined? and const_get on Queries. If you happen to have say, a class defined in the root namespace called Report and you call report, it'll find this class, make a new instance and give it to you. This creates some strangeness with the other conditional, where it defers to binding to allow you to access methods in the outer context.

I am using the DSL to dynamically create queries, often using instances of this model, which I have sensibly named report in the context I am using the DSL. However, when the arity == 0, and I try to access this report, it hits the const_defined? and initializes a new instance of ::Report.

class Report
end

class Example
  include Elasticsearch::DSL
  
  attr_accessor :report
  
  def initialize(report)
    @report = report
  end
  
  def generate_query
    search do
      query do
        # Now anywhere in here report will be a Report.new everytime, instead of deferring to the outer caller's report.
      end
      aggregation :wicked_metric do
        # Similarly here, the same issue arises.
      end
    end
  end
end

I don't believe this is the intended behavior, or if it is intended, perhaps ensuring the found class is within the Elasticsearch::DSL::Search namespace would be sufficient safety.

Queries.const_defined? klass, false

I can just use the arity to prevent the issue, but that's pretty ugly on larger queries and creates a lot of clutter.

@invke invke changed the title DSL AST initialisation searches recessive searches for const up namespaces DSL AST initialisation recursively searches up namespaces for consts Jan 19, 2021
@picandocodigo picandocodigo transferred this issue from elastic/elasticsearch-ruby Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant