Forbid NaN with the JavaScript driver

[neumino]

r.expr({a:NaN})

This query doesn't return an error.

I could insert the object in the database, but when I tried to retrieved it, the server crashed.

Version: rethinkdb 1.4.3-1207-gdb0014 (debug) (GCC 4.6.1)
error: Error in src/rdb_protocol/datum.cc at line 102:
error: Guarantee failed: [isfinite(r_num)] 
error: Backtrace:
error: Tue May  7 17:15:42 2013

       1: lazy_backtrace_t::lazy_backtrace_t() at backtrace.cc:249
       2: format_backtrace(bool) at backtrace.cc:196
       3: report_fatal_error(char const*, int, char const*, ...) at errors.cc:71
       4: ql::datum_t::init_json(cJSON*, ql::env_t*) at datum.cc:102
       5: ql::datum_t::datum_t(cJSON*, ql::env_t*) at datum.cc:128
       6: ql::datum_t::init_json(cJSON*, ql::env_t*) at datum.cc:118
       7: ql::datum_t::datum_t(boost::shared_ptr<scoped_cJSON_t> const&, ql::env_t*) at datum.cc:131
       8: ql::lazy_datum_stream_t::next_impl() at datum_stream.cc:234
       9: ql::datum_stream_t::next() at datum_stream.cc:30
       10: ql::stream_cache2_t::serve(long, Response*, signal_t*) at stream_cache.cc:42
       11: ql::run(Query*, scoped_ptr_t<ql::env_t>*, Response*, ql::stream_cache2_t*) at term.cc:172
       12: query2_server_t::handle(Query*, query2_server_t::context_t*) at pb_server.cc:53
       13: boost::_mfi::mf2<Response, query2_server_t, Query*, query2_server_t::context_t*>::operator()(query2_server_t*, Query*, query2_server_t::context_t*) const at mem_fn_template.hpp:281
       14: Response boost::_bi::list3<boost::_bi::value<query2_server_t*>, boost::arg<1>, boost::arg<2> >::operator()<Response, boost::_mfi::mf2<Response, query2_server_t, Query*, query2_server_t::context_t*>, boost::_bi::list2<Query*&, query2_server_t::context_t*&> >(boost::_bi::type<Response>, boost::_mfi::mf2<Response, query2_server_t, Query*, query2_server_t::context_t*>&, boost::_bi::list2<Query*&, query2_server_t::context_t*&>&, long) at bind.hpp:383
       15: Response boost::_bi::bind_t<Response, boost::_mfi::mf2<Response, query2_server_t, Query*, query2_server_t::context_t*>, boost::_bi::list3<boost::_bi::value<query2_server_t*>, boost::arg<1>, boost::arg<2> > >::operator()<Query*, query2_server_t::context_t*>(Query*&, query2_server_t::context_t*&) at bind_template.hpp:62
       16: boost::detail::function::function_obj_invoker2<boost::_bi::bind_t<Response, boost::_mfi::mf2<Response, query2_server_t, Query*, query2_server_t::context_t*>, boost::_bi::list3<boost::_bi::value<query2_server_t*>, boost::arg<1>, boost::arg<2> > >, Response, Query*, query2_server_t::context_t*>::invoke(boost::detail::function::function_buffer&, Query*, query2_server_t::context_t*) at function_template.hpp:133
       17: boost::function2<Response, Query*, query2_server_t::context_t*>::operator()(Query*, query2_server_t::context_t*) const at function_template.hpp:761
       18: protob_server_t<Query, Response, query2_server_t::context_t>::handle(http_req_t const&) at protob.tcc:211
       19: routing_http_app_t::handle(http_req_t const&) at routing_app.cc:29
       20: routing_http_app_t::handle(http_req_t const&) at routing_app.cc:29
       21: http_server_t::handle_conn(scoped_ptr_t<linux_tcp_conn_descriptor_t> const&, auto_drainer_t::lock_t) at http.cc:308
       22: boost::_mfi::mf2<void, http_server_t, scoped_ptr_t<linux_tcp_conn_descriptor_t> const&, auto_drainer_t::lock_t>::operator()(http_server_t*, scoped_ptr_t<linux_tcp_conn_descriptor_t> const&, auto_drainer_t::lock_t) const at mem_fn_template.hpp:280
       23: void boost::_bi::list3<boost::_bi::value<http_server_t*>, boost::arg<1>, boost::_bi::value<auto_drainer_t::lock_t> >::operator()<boost::_mfi::mf2<void, http_server_t, scoped_ptr_t<linux_tcp_conn_descriptor_t> const&, auto_drainer_t::lock_t>, boost::_bi::list1<scoped_ptr_t<linux_tcp_conn_descriptor_t>&> >(boost::_bi::type<void>, boost::_mfi::mf2<void, http_server_t, scoped_ptr_t<linux_tcp_conn_descriptor_t> const&, auto_drainer_t::lock_t>&, boost::_bi::list1<scoped_ptr_t<linux_tcp_conn_descriptor_t>&>&, int) at bind.hpp:392
       24: void boost::_bi::bind_t<void, boost::_mfi::mf2<void, http_server_t, scoped_ptr_t<linux_tcp_conn_descriptor_t> const&, auto_drainer_t::lock_t>, boost::_bi::list3<boost::_bi::value<http_server_t*>, boost::arg<1>, boost::_bi::value<auto_drainer_t::lock_t> > >::operator()<scoped_ptr_t<linux_tcp_conn_descriptor_t> >(scoped_ptr_t<linux_tcp_conn_descriptor_t>&) at bind_template.hpp:33
       25: boost::detail::function::void_function_obj_invoker1<boost::_bi::bind_t<void, boost::_mfi::mf2<void, http_server_t, scoped_ptr_t<linux_tcp_conn_descriptor_t> const&, auto_drainer_t::lock_t>, boost::_bi::list3<boost::_bi::value<http_server_t*>, boost::arg<1>, boost::_bi::value<auto_drainer_t::lock_t> > >, void, scoped_ptr_t<linux_tcp_conn_descriptor_t>&>::invoke(boost::detail::function::function_buffer&, scoped_ptr_t<linux_tcp_conn_descriptor_t>&) at function_template.hpp:154
       26: boost::function1<void, scoped_ptr_t<linux_tcp_conn_descriptor_t>&>::operator()(scoped_ptr_t<linux_tcp_conn_descriptor_t>&) const at function_template.hpp:761
       27: linux_nonthrowing_tcp_listener_t::handle(int) at network.cc:800
       28: boost::_mfi::mf1<void, linux_nonthrowing_tcp_listener_t, int>::operator()(linux_nonthrowing_tcp_listener_t*, int) const at mem_fn_template.hpp:166
       29: void boost::_bi::list2<boost::_bi::value<linux_nonthrowing_tcp_listener_t*>, boost::_bi::value<int> >::operator()<boost::_mfi::mf1<void, linux_nonthrowing_tcp_listener_t, int>, boost::_bi::list0>(boost::_bi::type<void>, boost::_mfi::mf1<void, linux_nonthrowing_tcp_listener_t, int>&, boost::_bi::list0&, int) at bind.hpp:314
       30: boost::_bi::bind_t<void, boost::_mfi::mf1<void, linux_nonthrowing_tcp_listener_t, int>, boost::_bi::list2<boost::_bi::value<linux_nonthrowing_tcp_listener_t*>, boost::_bi::value<int> > >::operator()() at bind_template.hpp:21
       31: callable_action_instance_t<boost::_bi::bind_t<void, boost::_mfi::mf1<void, linux_nonthrowing_tcp_listener_t, int>, boost::_bi::list2<boost::_bi::value<linux_nonthrowing_tcp_listener_t*>, boost::_bi::value<int> > > >::run_action() at runtime_utils.hpp:58
       32: callable_action_wrapper_t::run() at runtime_utils.cc:67
       33: coro_t::run() at coroutines.cc:178
error: Exiting.

Assigning to @wmrowan

[srh]

We need to forbid it on the server, not just the drivers.

[coffeemug]

Reassigning to @mlucy.

[wmrowan]

I tried playing around with this a bit myself. The server seems to accept NaN when you simply reflect it back. If you try to do math on NaN we get a sensible error back. It's only when retrieving a row containing NaN that we fail a guarantee. We should handle NaN the same way on every code path.

FYI, I can't actually replicate the crash. I get an ugly backtrace back in the data explorer but the server doesn't actually crash. I think this might be because I'm using the release build.

[mlucy]

It looks like the problem here is that datum_t::init_json is assuming that the cJSON code will have already errored on a non-finite value, which it apparently doesn't. This should be easy to fix.

[coffeemug]

@mlucy -- good morning :-D

[mlucy]

@coffeemug -- feel like dropping by the office and grabbing breakfast with me? ^.^

[coffeemug]

I'm in my bed in my proverbial pajamas, so no. I did consider it, but Newton's first law is seriously at play here :-D

[mlucy]

Actually, after looking at this a little more closely, I think the sanity check in init_json is correct. The only remaining places where we read from a cJSON object are when we're reading what was initially a datum_t object. So we just need to fix the original constructor.

[mlucy]

This is in code-review 500 by @srh .

[mlucy]

This is in next.