From 46e773c5db9ebc106823594b82f4ff14a0a004f3 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Tue, 16 May 2017 15:15:14 +0200
Subject: [PATCH] src: check if --icu-data-dir= points to valid dir

Call uc_init() after u_setDataDirectory() to find out if the data
directory is actually valid.

This commit removes parallel/test-intl-no-icu-data, added in commit
46345b9 ("src: make --icu-data-dir= switch testable").  It no longer
works now that an invalid --icu-data-dir= argument is rejected.
Coverage is now provided by parallel/test-icu-data-dir.

Fixes: https://github.com/nodejs/node/issues/13043
Refs: https://github.com/nodejs/node-gyp/issues/1199
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
---
 src/node.cc                            |  7 +++++--
 src/node_i18n.cc                       |  7 ++++---
 test/parallel/test-cli-node-options.js |  1 -
 test/parallel/test-icu-data-dir.js     | 19 +++++++++++++++++++
 test/parallel/test-intl-no-icu-data.js |  8 --------
 5 files changed, 28 insertions(+), 14 deletions(-)
 create mode 100644 test/parallel/test-icu-data-dir.js
 delete mode 100644 test/parallel/test-intl-no-icu-data.js

diff --git a/src/node.cc b/src/node.cc
index fb98fcebc2a4a5..8827195570fd57 100644
--- a/src/node.cc
+++ b/src/node.cc
@@ -4342,8 +4342,11 @@ void Init(int* argc,
   // Initialize ICU.
   // If icu_data_dir is empty here, it will load the 'minimal' data.
   if (!i18n::InitializeICUDirectory(icu_data_dir)) {
-    FatalError(nullptr, "Could not initialize ICU "
-                     "(check NODE_ICU_DATA or --icu-data-dir parameters)");
+    fprintf(stderr,
+            "%s: could not initialize ICU "
+            "(check NODE_ICU_DATA or --icu-data-dir parameters)",
+            argv[0]);
+    exit(9);
   }
 #endif
 
diff --git a/src/node_i18n.cc b/src/node_i18n.cc
index 6d966bb117d382..30394f3a47b79f 100644
--- a/src/node_i18n.cc
+++ b/src/node_i18n.cc
@@ -55,6 +55,7 @@
 #include <unicode/utypes.h>
 #include <unicode/putil.h>
 #include <unicode/uchar.h>
+#include <unicode/uclean.h>
 #include <unicode/udata.h>
 #include <unicode/uidna.h>
 #include <unicode/ucnv.h>
@@ -418,19 +419,19 @@ void GetVersion(const FunctionCallbackInfo<Value>& args) {
 }  // anonymous namespace
 
 bool InitializeICUDirectory(const std::string& path) {
+  UErrorCode status = U_ZERO_ERROR;
   if (path.empty()) {
-    UErrorCode status = U_ZERO_ERROR;
 #ifdef NODE_HAVE_SMALL_ICU
     // install the 'small' data.
     udata_setCommonData(&SMALL_ICUDATA_ENTRY_POINT, &status);
 #else  // !NODE_HAVE_SMALL_ICU
     // no small data, so nothing to do.
 #endif  // !NODE_HAVE_SMALL_ICU
-    return (status == U_ZERO_ERROR);
   } else {
     u_setDataDirectory(path.c_str());
-    return true;  // No error.
+    u_init(&status);
   }
+  return status == U_ZERO_ERROR;
 }
 
 int32_t ToUnicode(MaybeStackBuffer<char>* buf,
diff --git a/test/parallel/test-cli-node-options.js b/test/parallel/test-cli-node-options.js
index 6b0b9f53f9a462..bff0471808906a 100644
--- a/test/parallel/test-cli-node-options.js
+++ b/test/parallel/test-cli-node-options.js
@@ -57,7 +57,6 @@ if (common.hasCrypto) {
   expect('--use-bundled-ca', 'B\n');
   expect('--openssl-config=_ossl_cfg', 'B\n');
 }
-expect('--icu-data-dir=_d', 'B\n');
 
     // V8 options
 expect('--max_old_space_size=0', 'B\n');
diff --git a/test/parallel/test-icu-data-dir.js b/test/parallel/test-icu-data-dir.js
new file mode 100644
index 00000000000000..07a4391505b1e2
--- /dev/null
+++ b/test/parallel/test-icu-data-dir.js
@@ -0,0 +1,19 @@
+'use strict';
+require('../common');
+const assert = require('assert');
+const { spawnSync } = require('child_process');
+
+const expected =
+    'could not initialize ICU ' +
+    '(check NODE_ICU_DATA or --icu-data-dir parameters)';
+
+{
+  const child = spawnSync(process.execPath, ['--icu-data-dir=/', '-e', '0']);
+  assert(child.stderr.toString().includes(expected));
+}
+
+{
+  const env = { NODE_ICU_DATA: '/' };
+  const child = spawnSync(process.execPath, ['-e', '0'], { env });
+  assert(child.stderr.toString().includes(expected));
+}
diff --git a/test/parallel/test-intl-no-icu-data.js b/test/parallel/test-intl-no-icu-data.js
deleted file mode 100644
index 695a4698b56738..00000000000000
--- a/test/parallel/test-intl-no-icu-data.js
+++ /dev/null
@@ -1,8 +0,0 @@
-// Flags: --icu-data-dir=test/fixtures/empty/
-'use strict';
-require('../common');
-const assert = require('assert');
-const config = process.binding('config');
-
-assert.deepStrictEqual(Intl.NumberFormat.supportedLocalesOf('en'), []);
-assert.strictEqual(config.icuDataDir, 'test/fixtures/empty/');