Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: validate rhs of in when transpiling #p in C #15133

Merged
merged 9 commits into from Nov 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions Makefile
Expand Up @@ -192,8 +192,8 @@ new-version-checklist:
# @echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
# @echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
# @echo "!!!!!! !!!!!!"
# @echo "!!!!!! Write any message that should !!!!!!"
# @echo "!!!!!! block the release here !!!!!!"
# @echo "!!!!!! Update the minVersion of packages/babel-helpers/src/helpers/checkInRHS.js"
# @echo "!!!!!! !!!!!!"
# @echo "!!!!!! !!!!!!"
# @echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
# @echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
Expand Down
Expand Up @@ -142,17 +142,7 @@ function extractElementDescriptor(
}

function addDecorateHelper(file: File) {
try {
return file.addHelper("decorate");
} catch (err) {
if (err.code === "BABEL_HELPER_UNKNOWN") {
err.message +=
"\n '@babel/plugin-transform-decorators' in non-legacy mode" +
" requires '@babel/core' version ^7.0.2 and you appear to be using" +
" an older version.";
}
throw err;
}
return file.addHelper("decorate");
}

type ClassElement = t.Class["body"]["body"][number];
Expand Down
30 changes: 26 additions & 4 deletions packages/babel-helper-create-class-features-plugin/src/fields.ts
Expand Up @@ -206,12 +206,21 @@ function unshadow(
}
}

export function buildCheckInRHS(
rhs: t.Expression,
file: File,
inRHSIsObject?: boolean,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter is added so that we don't have to deal with different interface across different helper-create-class-features-plugin version. In this future we will pass this parameter for the inRHSIsObject assumption.

) {
if (inRHSIsObject || !file.availableHelper?.("checkInRHS")) return rhs;
return t.callExpression(file.addHelper("checkInRHS"), [rhs]);
}

const privateInVisitor = privateNameVisitorFactory<{
classRef: t.Identifier;
file: File;
innerBinding?: t.Identifier;
}>({
BinaryExpression(path) {
BinaryExpression(path, { file }) {
const { operator, left, right } = path.node;
if (operator !== "in") return;
if (!t.isPrivateName(left)) return;
Expand All @@ -230,19 +239,32 @@ const privateInVisitor = privateNameVisitorFactory<{
if (privateFieldsAsProperties) {
const { id } = privateNamesMap.get(name);
path.replaceWith(template.expression.ast`
Object.prototype.hasOwnProperty.call(${right}, ${t.cloneNode(id)})
Object.prototype.hasOwnProperty.call(${buildCheckInRHS(
right,
file,
)}, ${t.cloneNode(id)})
`);
return;
}

const { id, static: isStatic } = privateNamesMap.get(name);

if (isStatic) {
path.replaceWith(template.expression.ast`${right} === ${this.classRef}`);
path.replaceWith(
template.expression.ast`${buildCheckInRHS(
right,
file,
)} === ${t.cloneNode(this.classRef)}`,
);
return;
}

path.replaceWith(template.expression.ast`${t.cloneNode(id)}.has(${right})`);
path.replaceWith(
template.expression.ast`${t.cloneNode(id)}.has(${buildCheckInRHS(
right,
file,
)})`,
);
},
});

Expand Down
Expand Up @@ -8,14 +8,15 @@ import {
buildPrivateNamesMap,
transformPrivateNamesUsage,
buildFieldsInitNodes,
buildCheckInRHS,
} from "./fields";
import type { PropPath } from "./fields";
import { buildDecoratedClass, hasDecorators } from "./decorators";
import { injectInitialization, extractComputedKeys } from "./misc";
import { enableFeature, FEATURES, isLoose, shouldTransform } from "./features";
import { assertFieldTransformed } from "./typescript";

export { FEATURES, enableFeature, injectInitialization };
export { FEATURES, enableFeature, injectInitialization, buildCheckInRHS };

declare const PACKAGE_JSON: { name: string; version: string };

Expand Down
4 changes: 4 additions & 0 deletions packages/babel-helpers/src/helpers-generated.ts
Expand Up @@ -41,6 +41,10 @@ export default Object.freeze({
"7.0.0-beta.0",
'import OverloadYield from"OverloadYield";export default function _awaitAsyncGenerator(value){return new OverloadYield(value,0)}',
),
checkInRHS: helper(
"7.20.1",
'export default function _checkInRHS(value){if(Object(value)!==value)throw TypeError("right-hand side of \'in\' should be an object, got "+(null!==value?typeof value:"null"));return value}',
),
jsx: helper(
"7.0.0-beta.0",
'var REACT_ELEMENT_TYPE;export default function _createRawReactElement(type,props,key,children){REACT_ELEMENT_TYPE||(REACT_ELEMENT_TYPE="function"==typeof Symbol&&Symbol.for&&Symbol.for("react.element")||60103);var defaultProps=type&&type.defaultProps,childrenLength=arguments.length-3;if(props||0===childrenLength||(props={children:void 0}),1===childrenLength)props.children=children;else if(childrenLength>1){for(var childArray=new Array(childrenLength),i=0;i<childrenLength;i++)childArray[i]=arguments[i+3];props.children=childArray}if(props&&defaultProps)for(var propName in defaultProps)void 0===props[propName]&&(props[propName]=defaultProps[propName]);else props||(props=defaultProps||{});return{$$typeof:REACT_ELEMENT_TYPE,type:type,key:void 0===key?null:""+key,ref:null,props:props,_owner:null}}',
Expand Down
11 changes: 11 additions & 0 deletions packages/babel-helpers/src/helpers/checkInRHS.js
@@ -0,0 +1,11 @@
/* @minVersion 7.20.1 */

export default function _checkInRHS(value) {
if (Object(value) !== value) {
throw TypeError(
"right-hand side of 'in' should be an object, got " +
(value !== null ? typeof value : "null")
);
}
return value;
}
1 change: 1 addition & 0 deletions packages/babel-plugin-proposal-decorators/src/index.ts
Expand Up @@ -40,6 +40,7 @@ export default declare((api, options: Options) => {
} else if (version === "2021-12" || version === "2022-03") {
return transformer2022_03(api, options, version);
} else if (!process.env.BABEL_8_BREAKING) {
api.assertVersion("^7.0.2");
return createClassFeaturePlugin({
name: "proposal-decorators",

Expand Down
Expand Up @@ -8,8 +8,8 @@ new (_x = /*#__PURE__*/new WeakMap(), _m = /*#__PURE__*/new WeakSet(), (_temp =
writable: true,
value: void 0
}), babelHelpers.defineProperty(this, "x", void 0)), (() => {
hasX = o => _x.has(o);
hasM = o => _m.has(o);
hasX = o => _x.has(babelHelpers.checkInRHS(o));
hasM = o => _m.has(babelHelpers.checkInRHS(o));
})(), _initClass();
}
}, (() => {
Expand Down
Expand Up @@ -8,8 +8,8 @@ new (_x = /*#__PURE__*/new WeakMap(), _m = /*#__PURE__*/new WeakSet(), (_temp =
writable: true,
value: void 0
}), babelHelpers.defineProperty(this, "x", void 0)), (() => {
hasX = o => _x.has(o);
hasM = o => _m.has(o);
hasX = o => _x.has(babelHelpers.checkInRHS(o));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we move the .has call to the helper, and just transform #x in o to babelHelpers.privateHas(_x, o)? Or, if we keep it as a separate operation, it might be worth adding an assumption to skip the check.

Copy link
Contributor Author

@JLHwung JLHwung Nov 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I lean to a new assumption inRHSIsObject to skip the check. Otherwise we have to come up with at least three helpers:

  • staticPrivateHas for static #p = #p in C
  • privateHas for #p = #p in this
  • privateAsPropertiesHas for #p = #p in this

And we can implement the checkInRHS helper as

function checkInRHS(v) {
  0 in v;
  return v;
}

so that the engine will do the checking job for us.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, we can't because 0 in v is observable when using proxies.

hasM = o => _m.has(babelHelpers.checkInRHS(o));
})(), _initClass();
}
}, (() => {
Expand Down
Expand Up @@ -4,6 +4,7 @@ import {
enableFeature,
FEATURES,
injectInitialization as injectConstructorInit,
buildCheckInRHS,
} from "@babel/helper-create-class-features-plugin";
import annotateAsPure from "@babel/helper-annotate-as-pure";
import type * as t from "@babel/types";
Expand Down Expand Up @@ -119,8 +120,9 @@ export default declare((api, opt: Options) => {
enableFeature(this.file, FEATURES.privateIn, loose);
},
visitor: {
BinaryExpression(path) {
BinaryExpression(path, state) {
const { node } = path;
const { file } = state;
if (node.operator !== "in") return;
if (!t.isPrivateName(node.left)) return;

Expand Down Expand Up @@ -158,7 +160,10 @@ export default declare((api, opt: Options) => {
}
path.replaceWith(
template.expression.ast`
${t.cloneNode(outerClass.node.id)} === ${path.node.right}
${t.cloneNode(outerClass.node.id)} === ${buildCheckInRHS(
node.right,
file,
)}
`,
);
} else {
Expand All @@ -171,7 +176,10 @@ export default declare((api, opt: Options) => {
);

path.replaceWith(
template.expression.ast`${id}.has(${path.node.right})`,
template.expression.ast`${id}.has(${buildCheckInRHS(
node.right,
file,
)})`,
);
}
} else {
Expand All @@ -187,7 +195,10 @@ export default declare((api, opt: Options) => {
);

path.replaceWith(
template.expression.ast`${id}.has(${path.node.right})`,
template.expression.ast`${id}.has(${buildCheckInRHS(
node.right,
file,
)})`,
);
}
},
Expand Down
Expand Up @@ -7,7 +7,7 @@ class Foo {
});
}
test(other) {
return Object.prototype.hasOwnProperty.call(other, _foo);
return Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(other), _foo);
}
}
function _get_foo() {}
Expand Up @@ -13,12 +13,12 @@ let Foo = /*#__PURE__*/function () {
babelHelpers.createClass(Foo, [{
key: "test",
value: function test() {
return Object.prototype.hasOwnProperty.call(this, _bar);
return Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(this), _bar);
}
}], [{
key: "test",
value: function test() {
return Object.prototype.hasOwnProperty.call(Foo, _foo);
return Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(Foo), _foo);
}
}]);
return Foo;
Expand Down
Expand Up @@ -7,6 +7,6 @@ class Foo {
});
}
test(other) {
return Object.prototype.hasOwnProperty.call(other, _foo);
return Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(other), _foo);
}
}
Expand Up @@ -6,7 +6,7 @@ class Foo {
});
}
test(other) {
return Object.prototype.hasOwnProperty.call(other, _foo);
return Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(other), _foo);
}
}
function _foo2() {}
Expand Up @@ -21,11 +21,11 @@ class Foo {
});
}
test() {
Object.prototype.hasOwnProperty.call(this, _foo);
Object.prototype.hasOwnProperty.call(this, _bar2);
Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(this), _foo);
Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(this), _bar2);
}
}
Object.prototype.hasOwnProperty.call(this, _foo);
Object.prototype.hasOwnProperty.call(this, _bar);
Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(this), _foo);
Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(this), _bar);
}
}
Expand Up @@ -16,9 +16,9 @@ class Foo {
});
}
test() {
Object.prototype.hasOwnProperty.call(this, _foo2);
Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(this), _foo2);
}
}
Object.prototype.hasOwnProperty.call(this, _foo);
Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(this), _foo);
}
}
Expand Up @@ -9,9 +9,9 @@ class Foo {
test() {
class Nested {
test() {
Object.prototype.hasOwnProperty.call(this, _foo);
Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(this), _foo);
}
}
Object.prototype.hasOwnProperty.call(this, _foo);
Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(this), _foo);
}
}
@@ -0,0 +1,31 @@
expect(() => class { static #p = #p in 0 }).toThrowError(`right-hand side of 'in' should be an object, got number`);
expect(() => class { static #p = #p in "" }).toThrowError(`right-hand side of 'in' should be an object, got string`);
expect(() => class { static #p = #p in true }).toThrowError(`right-hand side of 'in' should be an object, got boolean`);
expect(() => class { static #p = #p in void 0 }).toThrowError(`right-hand side of 'in' should be an object, got undefined`);
expect(() => class { static #p = #p in null }).toThrowError(`right-hand side of 'in' should be an object, got null`);
expect(() => class { static #p = #p in Symbol.iterator }).toThrowError(`right-hand side of 'in' should be an object, got symbol`);
expect(() => class { static #p = #p in 0n }).toThrowError(`right-hand side of 'in' should be an object, got bigint`);

expect(() => class { static #p() {}; static q = #p in 0 }).toThrowError(`right-hand side of 'in' should be an object, got number`);
expect(() => class { static #p() {}; static q = #p in "" }).toThrowError(`right-hand side of 'in' should be an object, got string`);
expect(() => class { static #p() {}; static q = #p in true }).toThrowError(`right-hand side of 'in' should be an object, got boolean`);
expect(() => class { static #p() {}; static q = #p in void 0 }).toThrowError(`right-hand side of 'in' should be an object, got undefined`);
expect(() => class { static #p() {}; static q = #p in null }).toThrowError(`right-hand side of 'in' should be an object, got null`);
expect(() => class { static #p() {}; static q = #p in Symbol.iterator }).toThrowError(`right-hand side of 'in' should be an object, got symbol`);
expect(() => class { static #p() {}; static q = #p in 0n }).toThrowError(`right-hand side of 'in' should be an object, got bigint`);

expect(() => new class { #p = #p in 0 }).toThrowError(`right-hand side of 'in' should be an object, got number`);
expect(() => new class { #p = #p in "" }).toThrowError(`right-hand side of 'in' should be an object, got string`);
expect(() => new class { #p = #p in true }).toThrowError(`right-hand side of 'in' should be an object, got boolean`);
expect(() => new class { #p = #p in void 0 }).toThrowError(`right-hand side of 'in' should be an object, got undefined`);
expect(() => new class { #p = #p in null }).toThrowError(`right-hand side of 'in' should be an object, got null`);
expect(() => new class { #p = #p in Symbol.iterator }).toThrowError(`right-hand side of 'in' should be an object, got symbol`);
expect(() => new class { #p = #p in 0n }).toThrowError(`right-hand side of 'in' should be an object, got bigint`);

expect(() => new class { #p() {}; q = #p in 0 }).toThrowError(`right-hand side of 'in' should be an object, got number`);
expect(() => new class { #p() {}; q = #p in "" }).toThrowError(`right-hand side of 'in' should be an object, got string`);
expect(() => new class { #p() {}; q = #p in true }).toThrowError(`right-hand side of 'in' should be an object, got boolean`);
expect(() => new class { #p() {}; q = #p in void 0 }).toThrowError(`right-hand side of 'in' should be an object, got undefined`);
expect(() => new class { #p() {}; q = #p in null }).toThrowError(`right-hand side of 'in' should be an object, got null`);
expect(() => new class { #p() {}; q = #p in Symbol.iterator }).toThrowError(`right-hand side of 'in' should be an object, got symbol`);
expect(() => new class { #p() {}; q = #p in 0n }).toThrowError(`right-hand side of 'in' should be an object, got bigint`);
@@ -0,0 +1,3 @@
{
"minNodeVersion": "12.0.0"
}
@@ -1,7 +1,7 @@
var _foo = /*#__PURE__*/babelHelpers.classPrivateFieldLooseKey("foo");
class Foo {
test(other) {
return Object.prototype.hasOwnProperty.call(other, _foo);
return Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(other), _foo);
}
}
function _get_foo() {}
Expand Down
@@ -1,7 +1,7 @@
var _foo = /*#__PURE__*/babelHelpers.classPrivateFieldLooseKey("foo");
class Foo {
test(other) {
return Object.prototype.hasOwnProperty.call(other, _foo);
return Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(other), _foo);
}
}
Object.defineProperty(Foo, _foo, {
Expand Down
@@ -1,7 +1,7 @@
var _foo = /*#__PURE__*/babelHelpers.classPrivateFieldLooseKey("foo");
class Foo {
test(other) {
return Object.prototype.hasOwnProperty.call(other, _foo);
return Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(other), _foo);
}
}
function _foo2() {}
Expand Down
Expand Up @@ -12,7 +12,7 @@ let Foo = /*#__PURE__*/function () {
babelHelpers.createClass(Foo, [{
key: "test",
value: function test(other) {
return Object.prototype.hasOwnProperty.call(other, _foo);
return Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(other), _foo);
}
}]);
return Foo;
Expand Down
Expand Up @@ -12,7 +12,7 @@ let Foo = /*#__PURE__*/function () {
babelHelpers.createClass(Foo, [{
key: "test",
value: function test(other) {
return Object.prototype.hasOwnProperty.call(other, _foo);
return Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(other), _foo);
}
}]);
return Foo;
Expand Down
Expand Up @@ -11,7 +11,7 @@ let Foo = /*#__PURE__*/function () {
babelHelpers.createClass(Foo, [{
key: "test",
value: function test(other) {
return Object.prototype.hasOwnProperty.call(other, _foo);
return Object.prototype.hasOwnProperty.call(babelHelpers.checkInRHS(other), _foo);
}
}]);
return Foo;
Expand Down
Expand Up @@ -7,10 +7,10 @@ class Foo {
});
}
static test() {
return Foo === Foo;
return babelHelpers.checkInRHS(Foo) === Foo;
}
test() {
return _bar.has(this);
return _bar.has(babelHelpers.checkInRHS(this));
}
}
var _foo = {
Expand Down