[SERVER-16703] Javascript function may incorrectly return undefined when function definition contains "return" keyword Created: 31/Dec/14  Updated: 19/Sep/15  Resolved: 12/Aug/15

Status: Closed
Project: Core Server
Component/s: JavaScript
Affects Version/s: 2.6.5
Fix Version/s: 3.1.7

Type: Bug Priority: Major - P3
Reporter: Greg Pfeil Assignee: Jonathan Reams
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Backwards Compatibility: Fully Compatible
Operating System: ALL
Sprint: Platform 2 04/24/15, Platform 6 07/17/15, Build 7 08/10/15, Build 8 08/31/15
Participants:

 Description   

When parsing input from the user to create a Javascript function, the server tests for the presence of the "return" keyword in order to decide whether the input is an expression that needs to have the string "return " prepended to it before being wrapped in a function body. This test is not robust, and incorrectly passes where the string "return" appears with nested quote characters or nested functions.

As a result, Javascript functions created from user-provided expressions may incorrectly return undefined if the function definition contains the "return" keyword.

To reproduce:

> db.eval("5 === '\\' hello '")
false // Function returns false: expected.
> db.eval("5 === '\\' return '")
null // Function returns undefined: unexpected.

As a workaround, explicitly prepend the "return" keyword to the expression:

> db.eval("return 5 === '\\' return '")
false // Function returns false: expected.

Original description:

If a $where selector uses the string syntax, I can’t seem to use an anonymous function in the condition. If I do the same query using the function syntax, I get the expected result.

This is a problem for us, because we generate JS that uses lambdas to scope variables, and we use the Java driver, which seems to have no way to use the function syntax rather than the string syntax.

Steps to reproduce:
Using the zips database,

db.zips.count({ "$where" : function () { return -1 !== ["AZ", "CO"].indexOf((function (expr) { return (((typeof expr) !== "undefined") && (expr !== null)) ? expr.state : null; })(this)); } })

returns 684, as expected, but

db.zips.count({ "$where" : "-1 !== [\"AZ\", \"CO\"].indexOf((function (expr) { return (((typeof expr) !== \"undefined\") && (expr !== null)) ? expr.state : null; })(this))" })

(which I hope represents the same query) returns 0.



 Comments   
Comment by Githook User [ 12/Aug/15 ]

Author:

{u'username': u'jbreams', u'name': u'Jonathan Reams', u'email': u'jbreams@mongodb.com'}

Message: SERVER-16703 Improve parsing of JS function bodies and expressions
Branch: master
https://github.com/mongodb/mongo/commit/2bef8a3292a00090b8b62e27119481e5b21808cb

Comment by Githook User [ 11/Aug/15 ]

Author:

{u'username': u'jbreams', u'name': u'Jonathan Reams', u'email': u'jbreams@mongodb.com'}

Message: Revert "SERVER-16703 Improve parsing of JS function bodies and expressions"

This reverts commit 94582c724dffcf12c8ab1f7c17b70a012d74af72.
Branch: master
https://github.com/mongodb/mongo/commit/ba27b82f243df9ff78ac092667de65df9b456587

Comment by Githook User [ 11/Aug/15 ]

Author:

{u'username': u'jbreams', u'name': u'Jonathan Reams', u'email': u'jbreams@mongodb.com'}

Message: SERVER-16703 Improve parsing of JS function bodies and expressions
Branch: master
https://github.com/mongodb/mongo/commit/94582c724dffcf12c8ab1f7c17b70a012d74af72

Comment by J Rassi [ 09/Feb/15 ]

Hi,

I can confirm that there is a bug pertaining to the creation of Javascript functions on the server from input that contains the "return" keyword, in certain circumstances.

When sending a Javascript expression to the server like "5 === 5", the server converts the expression to a function like "function() { return 5 === 5 }" before evaluating it. The server skips the step of adding the "return" keyword if it sees one is already present, and skips the step of wrapping the input in "function() { ... }" if it sees the input is already a function. I can see that there is a bug in detecting whether the server needs to add the "return" keyword: if there is an anonymous function in the input, the server falsely concludes that it doesn't need to add the "return" keyword, and generates a function that returns nothing.

This issue doesn't manifest when you pass a function object to $where in the shell, as in this case the client will parse the input and encode it as "function() { return ... }" before sending it to the server.

As a workaround, add the "return" keyword explicitly before the expression you're passing to $where, like so:

> db.zips.count({ "$where" : "-1 !== [\"AZ\", \"CO\"].indexOf((function (expr) { return (((typeof expr) !== \"undefined\") && (expr !== null)) ? expr.state : null; })(this))" })
0
> db.zips.count({ "$where" : "return -1 !== [\"AZ\", \"CO\"].indexOf((function (expr) { return (((typeof expr) !== \"undefined\") && (expr !== null)) ? expr.state : null; })(this))" })
686

I'll update the description of this ticket to reflect the underlying problem. Thanks again for reporting this, and please continue to watch the ticket for updates.

~ Jason Rassi

Comment by Greg Pfeil [ 07/Jan/15 ]

Yep, you’re right. I’m surprised that this isn’t using $in already. We’ve done a lot of work to avoid generating $where conditions (eg, turning foo.a == foo.b into a projection of the boolean, then filtering on that boolean), but there are still a few cases we run into (like stringLength(foo.bar) < 4).

However, I’m working around this particular issue by using ?: instead of lambdas and consequently double-evaluating some things that hopefully don’t have side-effects, so while the bug still exists, it’s not (at the moment) getting in my way anymore.

Comment by Ramon Fernandez Marina [ 07/Jan/15 ]

sellout, I forgot to add that the use of $where will prevent you from taking advantages of indexes, so if performance is important to you I'd recommend you investigate the use of other operators like $in.

Comment by Ramon Fernandez Marina [ 07/Jan/15 ]

sellout, I can see the same behavior you describe and I'm looking into it. However, you may want to simplify your code using MongoDB operators, in this case $in:

db.zips.count({state:{$in:["AZ", "CO"]}})

Generated at Thu Feb 08 03:41:59 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.