From c770936d2e0383718e1e9d4426b7460cb9c4a4f5 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 --- 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 01fce09759c6..e5cec17d0316 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 10e5be8a4405..dc744d61e7b8 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 c96f99ccfd82..21a74abba7e3 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 5cd0ffc29db1..be92e2f8b6b7 100644 --- a/src/node.cc +++ b/src/node.cc @@ -4533,11 +4533,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 c2037c4cbe73..b04bd6253e93 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 a241e671edda..6faf2750d4d0 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 6e9c9dbe3a1b..4f501e6ef6db 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'));