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

Stack overflow in Painless #19475

Closed
clintongormley opened this issue Jul 18, 2016 · 6 comments

Comments

Projects
None yet
3 participants
@clintongormley
Copy link
Member

commented Jul 18, 2016

The following produces a stack overflow in Painless:

POST t/t/1/_update
{
  "upsert": {},
  "scripted_upsert": true,
  "script": {
    "lang": "painless",
    "inline": "for (def key : params.keySet()) { ctx._source[key] = params[key]}"
  }, 
  "params": {
      "bar": "two",
      "baz": "two"
  }
}
@clintongormley

This comment has been minimized.

Copy link
Member Author

commented Jul 18, 2016

And related (perhaps?), params can't be used with this syntax: params[var]:

POST t/t/1/_update
{
  "upsert": {},
  "scripted_upsert": true,
  "script": {
    "lang": "painless",
    "inline": "for (def key : ['bar','baz']) { ctx._source[key] = params[key]}"
  }, 
  "params": {
      "bar": "two",
      "baz": "two"
  }
}

produces:

    "_source": {
      "bar": null,
      "baz": null
    }
@jdconrad

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2016

Just got back from vacation, so I'll take a look at this issue today.

@jdconrad

This comment has been minimized.

Copy link
Contributor

commented Jul 25, 2016

@clintongormley

In the first example, the reason you are seeing stack overflow is because the result ends up being infinitely recursive. Params contains 'ctx,' 'bar,' and 'baz' (more than just the user input); not just 'bar' and 'baz.' This means that _source has 'ctx' -> '_source' -> 'ctx' -> 'source' ... forever :). This is interesting behavior (not exactly a bug), and maybe something we should exclusively prevent from happening.

In the second example, I'm not sure why those are null values, when I run the parameters as a local unit test, it seems to work well. There must be some disconnect between in coming user values somewhere along the line, and this will require some more investigation.

@clintongormley

This comment has been minimized.

Copy link
Member Author

commented Jul 27, 2016

Params contains 'ctx,' 'bar,' and 'baz' (more than just the user input); not just 'bar' and 'baz.' This means that _source has 'ctx' -> '_source' -> 'ctx' -> 'source' ... forever :).

Ah ok.

This is interesting behavior (not exactly a bug), and maybe something we should exclusively prevent from happening.

Just so you know, it crashes the node :)

@tlrx

This comment has been minimized.

Copy link
Member

commented Sep 22, 2016

@clintongormley In your second example from #19475 (comment), the params should be included in the script object, not at the same level.

@clintongormley

This comment has been minimized.

Copy link
Member Author

commented Sep 23, 2016

Doh - thanks

tlrx added a commit to tlrx/elasticsearch that referenced this issue Oct 11, 2016

XContentBuilder: Avoid building self-referencing objects
Some objects like maps, iterables or arrays of objects can self-reference themselves. This is mostly due to a bug in code but the XContentBuilder should be able to detect such situations and throws an IllegalArgumentException instead of building objects over and over until a stackoverflow occurs.

closes elastic#20540
closes elastic#19475

@tlrx tlrx closed this in #20550 Oct 11, 2016

tlrx added a commit that referenced this issue Oct 11, 2016

XContentBuilder: Avoid building self-referencing objects (#20550)
Some objects like maps, iterables or arrays of objects can self-reference themselves. This is mostly due to a bug in code but the XContentBuilder should be able to detect such situations and throws an IllegalArgumentException instead of building objects over and over until a stackoverflow occurs.

closes #20540
closes #19475

tlrx added a commit that referenced this issue Oct 11, 2016

XContentBuilder: Avoid building self-referencing objects
Some objects like maps, iterables or arrays of objects can self-reference themselves. This is mostly due to a bug in code but the XContentBuilder should be able to detect such situations and throws an IllegalArgumentException instead of building objects over and over until a stackoverflow occurs.

closes #20540
closes #19475

(cherry picked from commit 680b69c)

@clintongormley clintongormley added v5.1.1 and removed v5.0.0 labels Oct 18, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.