From 3aa5b7d939409bb5abab0fd4478e44f4e643e297 Mon Sep 17 00:00:00 2001
From: Kyle Farnung <kfarnung@microsoft.com>
Date: Thu, 15 Mar 2018 17:22:30 -0700
Subject: [PATCH] n-api: add more `int64_t` tests

* Updated tests for `Number` and `int32_t`
* Added new tests for `int64_t`
* Updated N-API `int64_t` behavior to return zero for all non-finite
  numbers
* Clarified the documentation for these calls.

Backport-PR-URL: https://github.com/nodejs/node/pull/19447
PR-URL: https://github.com/nodejs/node/pull/19402
Refs: https://github.com/nodejs/node-chakracore/pull/500
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 doc/api/n-api.md                     |  27 ++++--
 src/node_api.cc                      |  23 +++--
 test/addons-napi/test_number/test.js | 135 ++++++++++++++++++++-------
 3 files changed, 135 insertions(+), 50 deletions(-)

diff --git a/doc/api/n-api.md b/doc/api/n-api.md
index e176cbdf1ef9cd..38d7b22f558ec2 100644
--- a/doc/api/n-api.md
+++ b/doc/api/n-api.md
@@ -1806,13 +1806,17 @@ napi_status napi_get_value_int32(napi_env env,
 - `[out] result`: C int32 primitive equivalent of the given JavaScript Number.
 
 Returns `napi_ok` if the API succeeded. If a non-number `napi_value`
-is passed in `napi_number_expected .
+is passed in `napi_number_expected`.
 
 This API returns the C int32 primitive equivalent
-of the given JavaScript Number. If the number exceeds the range of the
-32 bit integer, then the result is truncated to the equivalent of the
-bottom 32 bits. This can result in a large positive number becoming
-a negative number if the value is > 2^31 -1.
+of the given JavaScript Number.
+
+If the number exceeds the range of the 32 bit integer, then the result is
+truncated to the equivalent of the bottom 32 bits. This can result in a large
+positive number becoming a negative number if the value is > 2^31 -1.
+
+Non-finite number values (NaN, positive infinity, or negative infinity) set the
+result to zero.
 
 #### napi_get_value_int64
 <!-- YAML
@@ -1831,8 +1835,17 @@ napi_status napi_get_value_int64(napi_env env,
 Returns `napi_ok` if the API succeeded. If a non-number `napi_value`
 is passed in it returns `napi_number_expected`.
 
-This API returns the C int64 primitive equivalent of the given
-JavaScript Number.
+This API returns the C int64 primitive equivalent of the given JavaScript
+Number.
+
+Number values outside the range of
+[`Number.MIN_SAFE_INTEGER`](https://tc39.github.io/ecma262/#sec-number.min_safe_integer)
+-(2^53 - 1) -
+[`Number.MAX_SAFE_INTEGER`](https://tc39.github.io/ecma262/#sec-number.max_safe_integer)
+(2^53 - 1) will lose precision.
+
+Non-finite number values (NaN, positive infinity, or negative infinity) set the
+result to zero.
 
 #### napi_get_value_string_latin1
 <!-- YAML
diff --git a/src/node_api.cc b/src/node_api.cc
index a13398006b652d..fbe468b5a30701 100644
--- a/src/node_api.cc
+++ b/src/node_api.cc
@@ -2193,15 +2193,24 @@ napi_status napi_get_value_int64(napi_env env,
 
   RETURN_STATUS_IF_FALSE(env, val->IsNumber(), napi_number_expected);
 
-  // v8::Value::IntegerValue() converts NaN to INT64_MIN, inconsistent with
-  // v8::Value::Int32Value() that converts NaN to 0. So special-case NaN here.
+  // v8::Value::IntegerValue() converts NaN, +Inf, and -Inf to INT64_MIN,
+  // inconsistent with v8::Value::Int32Value() which converts those values to 0.
+  // Special-case all non-finite values to match that behavior.
   double doubleValue = val.As<v8::Number>()->Value();
-  if (std::isnan(doubleValue)) {
-    *result = 0;
+  if (std::isfinite(doubleValue)) {
+    // v8::Value::IntegerValue() as shipped with v6.x returns inconsistent
+    // values outside of the int64_t range. We rectify that here.
+    if (doubleValue >= static_cast<double>(INT64_MAX)) {
+      *result = INT64_MAX;
+    } else if (doubleValue <= static_cast<double>((int64_t)-INT64_MAX - 1)) {
+      *result = -INT64_MAX - 1;
+    } else {
+      // Empty context: https://github.com/nodejs/node/issues/14379
+      v8::Local<v8::Context> context;
+      *result = val->IntegerValue(context).FromJust();
+    }
   } else {
-    // Empty context: https://github.com/nodejs/node/issues/14379
-    v8::Local<v8::Context> context;
-    *result = val->IntegerValue(context).FromJust();
+    *result = 0;
   }
 
   return napi_clear_last_error(env);
diff --git a/test/addons-napi/test_number/test.js b/test/addons-napi/test_number/test.js
index f9d6eb6a6f307a..3dbbb64932e502 100644
--- a/test/addons-napi/test_number/test.js
+++ b/test/addons-napi/test_number/test.js
@@ -5,50 +5,113 @@ const test_number = require(`./build/${common.buildType}/test_number`);
 
 
 // testing api calls for number
-assert.strictEqual(0, test_number.Test(0));
-assert.strictEqual(1, test_number.Test(1));
-assert.strictEqual(-1, test_number.Test(-1));
-assert.strictEqual(100, test_number.Test(100));
-assert.strictEqual(2121, test_number.Test(2121));
-assert.strictEqual(-1233, test_number.Test(-1233));
-assert.strictEqual(986583, test_number.Test(986583));
-assert.strictEqual(-976675, test_number.Test(-976675));
+function testNumber(num) {
+  assert.strictEqual(num, test_number.Test(num));
+}
 
-const num1 = 98765432213456789876546896323445679887645323232436587988766545658;
-assert.strictEqual(num1, test_number.Test(num1));
+testNumber(0);
+testNumber(-0);
+testNumber(1);
+testNumber(-1);
+testNumber(100);
+testNumber(2121);
+testNumber(-1233);
+testNumber(986583);
+testNumber(-976675);
 
-const num2 = -4350987086545760976737453646576078997096876957864353245245769809;
-assert.strictEqual(num2, test_number.Test(num2));
+testNumber(
+  98765432213456789876546896323445679887645323232436587988766545658);
+testNumber(
+  -4350987086545760976737453646576078997096876957864353245245769809);
+testNumber(Number.MIN_SAFE_INTEGER);
+testNumber(Number.MAX_SAFE_INTEGER);
+testNumber(Number.MAX_SAFE_INTEGER + 10);
 
-const num3 = Number.MAX_SAFE_INTEGER;
-assert.strictEqual(num3, test_number.Test(num3));
+testNumber(Number.MIN_VALUE);
+testNumber(Number.MAX_VALUE);
+testNumber(Number.MAX_VALUE + 10);
 
-const num4 = Number.MAX_SAFE_INTEGER + 10;
-assert.strictEqual(num4, test_number.Test(num4));
+testNumber(Number.POSITIVE_INFINITY);
+testNumber(Number.NEGATIVE_INFINITY);
+assert(Object.is(NaN, test_number.Test(NaN)));
 
-const num5 = Number.MAX_VALUE;
-assert.strictEqual(num5, test_number.Test(num5));
+// validate documented behavior when value is retrieved as 32-bit integer with
+// `napi_get_value_int32`
+function testInt32(input, expected = input) {
+  assert.strictEqual(expected, test_number.TestInt32Truncation(input));
+}
 
-const num6 = Number.MAX_VALUE + 10;
-assert.strictEqual(num6, test_number.Test(num6));
+// Test zero
+testInt32(0.0, 0);
+testInt32(-0.0, 0);
 
-const num7 = Number.POSITIVE_INFINITY;
-assert.strictEqual(num7, test_number.Test(num7));
+// Test min/max int32 range
+testInt32(-Math.pow(2, 31));
+testInt32(Math.pow(2, 31) - 1);
 
-const num8 = Number.NEGATIVE_INFINITY;
-assert.strictEqual(num8, test_number.Test(num8));
+// Test overflow scenarios
+testInt32(4294967297, 1);
+testInt32(4294967296, 0);
+testInt32(4294967295, -1);
+testInt32(4294967296 * 5 + 3, 3);
 
+// Test min/max safe integer range
+testInt32(Number.MIN_SAFE_INTEGER, 1);
+testInt32(Number.MAX_SAFE_INTEGER, -1);
 
-// validate documented behaviour when value is retrieved
-// as 32 bit integer with napi_get_value_int32
-assert.strictEqual(1, test_number.TestInt32Truncation(4294967297));
-assert.strictEqual(0, test_number.TestInt32Truncation(4294967296));
-assert.strictEqual(-1, test_number.TestInt32Truncation(4294967295));
-assert.strictEqual(3, test_number.TestInt32Truncation(4294967296 * 5 + 3));
+// Test within int64_t range (with precision loss)
+testInt32(-Math.pow(2, 63) + (Math.pow(2, 9) + 1), 1024);
+testInt32(Math.pow(2, 63) - (Math.pow(2, 9) + 1), -1024);
 
-// validate that the boundaries of safe integer can be passed through
-// successfully
-assert.strictEqual(Number.MAX_SAFE_INTEGER,
-                   test_number.TestInt64Truncation(Number.MAX_SAFE_INTEGER));
-assert.strictEqual(Number.MIN_SAFE_INTEGER,
-                   test_number.TestInt64Truncation(Number.MIN_SAFE_INTEGER));
+// Test min/max double value
+testInt32(-Number.MIN_VALUE, 0);
+testInt32(Number.MIN_VALUE, 0);
+testInt32(-Number.MAX_VALUE, 0);
+testInt32(Number.MAX_VALUE, 0);
+
+// Test outside int64_t range
+testInt32(-Math.pow(2, 63) + (Math.pow(2, 9)), 0);
+testInt32(Math.pow(2, 63) - (Math.pow(2, 9)), 0);
+
+// Test non-finite numbers
+testInt32(Number.POSITIVE_INFINITY, 0);
+testInt32(Number.NEGATIVE_INFINITY, 0);
+testInt32(Number.NaN, 0);
+
+// validate documented behavior when value is retrieved as 64-bit integer with
+// `napi_get_value_int64`
+function testInt64(input, expected = input) {
+  assert.strictEqual(expected, test_number.TestInt64Truncation(input));
+}
+
+// Both V8 and ChakraCore return a sentinel value of `0x8000000000000000` when
+// the conversion goes out of range, but V8 treats it as unsigned in some cases.
+const RANGEERROR_POSITIVE = Math.pow(2, 63);
+const RANGEERROR_NEGATIVE = -Math.pow(2, 63);
+
+// Test zero
+testInt64(0.0, 0);
+testInt64(-0.0, 0);
+
+// Test min/max safe integer range
+testInt64(Number.MIN_SAFE_INTEGER);
+testInt64(Number.MAX_SAFE_INTEGER);
+
+// Test within int64_t range (with precision loss)
+testInt64(-Math.pow(2, 63) + (Math.pow(2, 9) + 1));
+testInt64(Math.pow(2, 63) - (Math.pow(2, 9) + 1));
+
+// Test min/max double value
+testInt64(-Number.MIN_VALUE, 0);
+testInt64(Number.MIN_VALUE, 0);
+testInt64(-Number.MAX_VALUE, RANGEERROR_NEGATIVE);
+testInt64(Number.MAX_VALUE, RANGEERROR_POSITIVE);
+
+// Test outside int64_t range
+testInt64(-Math.pow(2, 63) + (Math.pow(2, 9)), RANGEERROR_NEGATIVE);
+testInt64(Math.pow(2, 63) - (Math.pow(2, 9)), RANGEERROR_POSITIVE);
+
+// Test non-finite numbers
+testInt64(Number.POSITIVE_INFINITY, 0);
+testInt64(Number.NEGATIVE_INFINITY, 0);
+testInt64(Number.NaN, 0);