Skip to content

Commit

Permalink
intl: unexpose Intl.v8BreakIterator
Browse files Browse the repository at this point in the history
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: #8865
Fixes: #14909
Refs: https://github.com/tc39/proposal-intl-segmenter
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/620755
  • Loading branch information
bnoordhuis committed Sep 8, 2017
1 parent 428bcb7 commit c770936
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 32 deletions.
5 changes: 3 additions & 2 deletions doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,10 @@ and should no longer be used.
<a id="DEP0017"></a>
### 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).

<a id="DEP0018"></a>
### DEP0018: Unhandled promise rejections
Expand Down
2 changes: 0 additions & 2 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"');
Expand Down
14 changes: 0 additions & 14 deletions lib/internal/process.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}


Expand Down
19 changes: 18 additions & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4533,11 +4533,28 @@ void FreeEnvironment(Environment* env) {
}


Local<Context> NewContext(Isolate* isolate,
Local<ObjectTemplate> 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<Value> intl_v;
Local<Object> 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 = Context::New(isolate);
Local<Context> context = NewContext(isolate);
Context::Scope context_scope(context);
Environment env(isolate_data, context);
CHECK_EQ(0, uv_key_create(&thread_local_env));
Expand Down
2 changes: 1 addition & 1 deletion src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ class ContextifyContext {
CreateDataWrapper(env));
object_template->SetHandler(config);

Local<Context> ctx = Context::New(env->isolate(), nullptr, object_template);
Local<Context> ctx = NewContext(env->isolate(), object_template);

if (ctx.IsEmpty()) {
env->ThrowError("Could not instantiate context");
Expand Down
8 changes: 8 additions & 0 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,14 @@ inline v8::Local<TypeName> PersistentToLocal(
v8::Isolate* isolate,
const v8::Persistent<TypeName>& 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<v8::Context> NewContext(
v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> object_template =
v8::Local<v8::ObjectTemplate>());

// 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.
Expand Down
17 changes: 5 additions & 12 deletions test/parallel/test-intl-v8BreakIterator.js
Original file line number Diff line number Diff line change
@@ -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'));

0 comments on commit c770936

Please sign in to comment.