From 668ad449226940db92a5092822b77f3620631748 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 7 Sep 2017 13:26:47 +0200 Subject: [PATCH] intl: unexpose Intl.v8BreakIterator It was never an official Ecma-402 API, it is about to be superseded by `Intl.Segmenter` and it's prone to crash under some circumstances. Searches don't turn up any usage in the wild and the recommendation from the V8 team is to remove it. Now seems like a good a time as any to do that. Fixes: https://github.com/nodejs/node/issues/8865 Fixes: https://github.com/nodejs/node/issues/14909 Refs: https://github.com/tc39/proposal-intl-segmenter Refs: https://chromium-review.googlesource.com/c/v8/v8/+/620755 PR-URL: https://github.com/nodejs/node/pull/15238 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- doc/api/deprecations.md | 5 +++-- lib/internal/errors.js | 2 -- lib/internal/process.js | 14 -------------- src/node.cc | 19 ++++++++++++++++++- src/node_contextify.cc | 2 +- src/node_internals.h | 8 ++++++++ test/parallel/test-intl-v8BreakIterator.js | 17 +++++------------ 7 files changed, 35 insertions(+), 32 deletions(-) diff --git a/doc/api/deprecations.md b/doc/api/deprecations.md index 01fce09759c67f..e5cec17d03163e 100644 --- a/doc/api/deprecations.md +++ b/doc/api/deprecations.md @@ -185,9 +185,10 @@ and should no longer be used. ### DEP0017: Intl.v8BreakIterator -Type: Runtime +Type: End-of-Life -The `Intl.v8BreakIterator` is deprecated and will be removed or replaced soon. +`Intl.v8BreakIterator` was a non-standard extension and has been removed. +See [`Intl.Segmenter`](https://github.com/tc39/proposal-intl-segmenter). ### DEP0018: Unhandled promise rejections diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 10e5be8a44056d..dc744d61e7b8a3 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -275,8 +275,6 @@ E('ERR_UNKNOWN_STREAM_TYPE', 'Unknown stream file type'); E('ERR_VALUE_OUT_OF_RANGE', (start, end, value) => { return `The value of "${start}" must be ${end}. Received "${value}"`; }); -E('ERR_V8BREAKITERATOR', 'Full ICU data not installed. ' + - 'See https://github.com/nodejs/node/wiki/Intl'); E('ERR_VALID_PERFORMANCE_ENTRY_TYPE', 'At least one valid performance entry type is required'); E('ERR_VALUE_OUT_OF_RANGE', 'The value of "%s" must be %s. Received "%s"'); diff --git a/lib/internal/process.js b/lib/internal/process.js index c96f99ccfd8299..21a74abba7e313 100644 --- a/lib/internal/process.js +++ b/lib/internal/process.js @@ -133,20 +133,6 @@ function setupConfig(_source) { if (value === 'false') return false; return value; }); - const processConfig = process.binding('config'); - if (typeof Intl !== 'undefined' && Intl.hasOwnProperty('v8BreakIterator')) { - const oldV8BreakIterator = Intl.v8BreakIterator; - const des = Object.getOwnPropertyDescriptor(Intl, 'v8BreakIterator'); - des.value = require('internal/util').deprecate(function v8BreakIterator() { - if (processConfig.hasSmallICU && !processConfig.icuDataDir) { - // Intl.v8BreakIterator() would crash w/ fatal error, so throw instead. - throw new errors.Error('ERR_V8BREAKITERATOR'); - } - return Reflect.construct(oldV8BreakIterator, arguments); - }, 'Intl.v8BreakIterator is deprecated and will be removed soon.', - 'DEP0017'); - Object.defineProperty(Intl, 'v8BreakIterator', des); - } } diff --git a/src/node.cc b/src/node.cc index a569011894781d..664ae22a9ae258 100644 --- a/src/node.cc +++ b/src/node.cc @@ -4586,11 +4586,28 @@ void FreeEnvironment(Environment* env) { } +Local NewContext(Isolate* isolate, + Local object_template) { + auto context = Context::New(isolate, nullptr, object_template); + if (context.IsEmpty()) return context; + HandleScope handle_scope(isolate); + auto intl_key = FIXED_ONE_BYTE_STRING(isolate, "Intl"); + auto break_iter_key = FIXED_ONE_BYTE_STRING(isolate, "v8BreakIterator"); + Local intl_v; + Local intl; + if (context->Global()->Get(context, intl_key).ToLocal(&intl_v) && + intl_v->ToObject(context).ToLocal(&intl)) { + intl->Delete(context, break_iter_key).FromJust(); + } + return context; +} + + inline int Start(Isolate* isolate, IsolateData* isolate_data, int argc, const char* const* argv, int exec_argc, const char* const* exec_argv) { HandleScope handle_scope(isolate); - Local context = Context::New(isolate); + Local context = NewContext(isolate); Context::Scope context_scope(context); Environment env(isolate_data, context); CHECK_EQ(0, uv_key_create(&thread_local_env)); diff --git a/src/node_contextify.cc b/src/node_contextify.cc index c2037c4cbe7354..b04bd6253e9386 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -240,7 +240,7 @@ class ContextifyContext { CreateDataWrapper(env)); object_template->SetHandler(config); - Local ctx = Context::New(env->isolate(), nullptr, object_template); + Local ctx = NewContext(env->isolate(), object_template); if (ctx.IsEmpty()) { env->ThrowError("Could not instantiate context"); diff --git a/src/node_internals.h b/src/node_internals.h index a241e671edda48..6faf2750d4d039 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -125,6 +125,14 @@ inline v8::Local PersistentToLocal( v8::Isolate* isolate, const v8::Persistent& persistent); +// Creates a new context with Node.js-specific tweaks. Currently, it removes +// the `v8BreakIterator` property from the global `Intl` object if present. +// See https://github.com/nodejs/node/issues/14909 for more info. +v8::Local NewContext( + v8::Isolate* isolate, + v8::Local object_template = + v8::Local()); + // Convert a struct sockaddr to a { address: '1.2.3.4', port: 1234 } JS object. // Sets address and port properties on the info object and returns it. // If |info| is omitted, a new object is returned. diff --git a/test/parallel/test-intl-v8BreakIterator.js b/test/parallel/test-intl-v8BreakIterator.js index 6e9c9dbe3a1bcb..4f501e6ef6db28 100644 --- a/test/parallel/test-intl-v8BreakIterator.js +++ b/test/parallel/test-intl-v8BreakIterator.js @@ -1,17 +1,10 @@ 'use strict'; const common = require('../common'); +const assert = require('assert'); +const vm = require('vm'); -if (!common.hasIntl || Intl.v8BreakIterator === undefined) +if (typeof Intl === 'undefined') common.skip('missing Intl'); -const assert = require('assert'); -const warning = 'Intl.v8BreakIterator is deprecated and will be removed soon.'; -common.expectWarning('DeprecationWarning', warning); - -try { - new Intl.v8BreakIterator(); - // May succeed if data is available - OK -} catch (e) { - // May throw this error if ICU data is not available - OK - assert.throws(() => new Intl.v8BreakIterator(), /ICU data/); -} +assert(!('v8BreakIterator' in Intl)); +assert(!vm.runInNewContext('"v8BreakIterator" in Intl'));