Skip to content

Commit

Permalink
fix #3651: handle __proto__ edge cases better
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Feb 19, 2024
1 parent 555db48 commit ac36537
Show file tree
Hide file tree
Showing 5 changed files with 304 additions and 9 deletions.
39 changes: 39 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,45 @@
console.log(!0,!1);
```

* Better handling of `__proto__` edge cases ([#3651](https://github.com/evanw/esbuild/pull/3651))

JavaScript object literal syntax contains a special case where a non-computed property with a key of `__proto__` sets the prototype of the object. This does not apply to computed properties or to properties that use the shorthand property syntax introduced in ES6. Previously esbuild didn't correctly preserve the "sets the prototype" status of properties inside an object literal, meaning a property that sets the prototype could accidentally be transformed into one that doesn't and vice versa. This has now been fixed:

```js
// Original code
function foo(__proto__) {
return { __proto__: __proto__ } // Note: sets the prototype
}
function bar(__proto__, proto) {
{
let __proto__ = proto
return { __proto__ } // Note: doesn't set the prototype
}
}

// Old output
function foo(__proto__) {
return { __proto__ }; // Note: no longer sets the prototype (WRONG)
}
function bar(__proto__, proto) {
{
let __proto__2 = proto;
return { __proto__: __proto__2 }; // Note: now sets the prototype (WRONG)
}
}

// New output
function foo(__proto__) {
return { __proto__: __proto__ }; // Note: sets the prototype (correct)
}
function bar(__proto__, proto) {
{
let __proto__2 = proto;
return { ["__proto__"]: __proto__2 }; // Note: doesn't set the prototype (correct)
}
}
```

* Fix cross-platform non-determinism with CSS color space transformations ([#3650](https://github.com/evanw/esbuild/issues/3650))

The Go compiler takes advantage of "fused multiply and add" (FMA) instructions on certain processors which do the operation `x*y + z` without intermediate rounding. This causes esbuild's CSS color space math to differ on different processors (currently `ppc64le` and `s390x`), which breaks esbuild's guarantee of deterministic output. To avoid this, esbuild's color space math now inserts a `float64()` cast around every single math operation. This tells the Go compiler not to use the FMA optimization.
Expand Down
121 changes: 121 additions & 0 deletions internal/bundler_tests/bundler_default_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8690,3 +8690,124 @@ func TestJSXDevSelfEdgeCases(t *testing.T) {
},
})
}

func TestObjectLiteralProtoSetterEdgeCases(t *testing.T) {
default_suite.expectBundled(t, bundled{
files: map[string]string{
"/local-shorthand.js": `
function foo(__proto__, bar) {
{
let __proto__, bar // These locals will be renamed
console.log(
'this must not become "{ __proto__: ... }":',
{
__proto__,
bar,
},
)
}
}
`,
"/local-normal.js": `
function foo(__proto__, bar) {
console.log(
'this must not become "{ __proto__ }":',
{
__proto__: __proto__,
bar: bar,
},
)
}
`,
"/import-shorthand.js": `
import { __proto__, bar } from 'foo'
function foo() {
console.log(
'this must not become "{ __proto__: ... }":',
{
__proto__,
bar,
},
)
}
`,
"/import-normal.js": `
import { __proto__, bar } from 'foo'
function foo() {
console.log(
'this must not become "{ __proto__ }":',
{
__proto__: __proto__,
bar: bar,
},
)
}
`,
},
entryPaths: []string{"*"},
options: config.Options{
AbsOutputDir: "/out",
},
})
}

func TestObjectLiteralProtoSetterEdgeCasesMinifySyntax(t *testing.T) {
default_suite.expectBundled(t, bundled{
files: map[string]string{
"/local-computed.js": `
function foo(__proto__, bar) {
{
let __proto__, bar // These locals will be renamed
console.log(
'this must not become "{ __proto__: ... }":',
{
['__proto__']: __proto__,
['bar']: bar,
},
)
}
}
`,
"/local-normal.js": `
function foo(__proto__, bar) {
console.log(
'this must not become "{ __proto__ }":',
{
__proto__: __proto__,
bar: bar,
},
)
}
`,
"/import-computed.js": `
import { __proto__, bar } from 'foo'
function foo() {
console.log(
'this must not become "{ __proto__: ... }":',
{
['__proto__']: __proto__,
['bar']: bar,
},
)
}
`,
"/import-normal.js": `
import { __proto__, bar } from 'foo'
function foo() {
console.log(
'this must not become "{ __proto__ }":',
{
__proto__: __proto__,
bar: bar,
},
)
}
`,
},
entryPaths: []string{"*"},
options: config.Options{
AbsOutputDir: "/out",
MinifySyntax: true,
},
})
}
102 changes: 102 additions & 0 deletions internal/bundler_tests/snapshots/snapshots_default.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5187,6 +5187,108 @@ export {
i as aap
};

================================================================================
TestObjectLiteralProtoSetterEdgeCases
---------- /out/import-normal.js ----------
import { __proto__, bar } from "foo";
function foo() {
console.log(
'this must not become "{ __proto__ }":',
{
__proto__: __proto__,
bar
}
);
}

---------- /out/import-shorthand.js ----------
import { __proto__, bar } from "foo";
function foo() {
console.log(
'this must not become "{ __proto__: ... }":',
{
__proto__,
bar
}
);
}

---------- /out/local-normal.js ----------
function foo(__proto__, bar) {
console.log(
'this must not become "{ __proto__ }":',
{
__proto__: __proto__,
bar
}
);
}

---------- /out/local-shorthand.js ----------
function foo(__proto__, bar) {
{
let __proto__2, bar2;
console.log(
'this must not become "{ __proto__: ... }":',
{
["__proto__"]: __proto__2,
bar: bar2
}
);
}
}

================================================================================
TestObjectLiteralProtoSetterEdgeCasesMinifySyntax
---------- /out/import-computed.js ----------
import { __proto__, bar } from "foo";
function foo() {
console.log(
'this must not become "{ __proto__: ... }":',
{
["__proto__"]: __proto__,
bar
}
);
}

---------- /out/import-normal.js ----------
import { __proto__, bar } from "foo";
function foo() {
console.log(
'this must not become "{ __proto__ }":',
{
__proto__: __proto__,
bar
}
);
}

---------- /out/local-computed.js ----------
function foo(__proto__, bar) {
{
let __proto__2, bar2;
console.log(
'this must not become "{ __proto__: ... }":',
{
["__proto__"]: __proto__2,
bar: bar2
}
);
}
}

---------- /out/local-normal.js ----------
function foo(__proto__, bar) {
console.log(
'this must not become "{ __proto__ }":',
{
__proto__: __proto__,
bar
}
);
}

================================================================================
TestOutbase
---------- /out/a/b/c.js ----------
Expand Down
33 changes: 31 additions & 2 deletions internal/js_printer/js_printer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1242,7 +1242,7 @@ func (p *printer) printProperty(property js_ast.Property) {
if !p.options.UnsupportedFeatures.Has(compat.ObjectExtensions) && property.ValueOrNil.Data != nil && !p.willPrintExprCommentsAtLoc(property.ValueOrNil.Loc) {
switch e := property.ValueOrNil.Data.(type) {
case *js_ast.EIdentifier:
if helpers.UTF16EqualsString(key.Value, p.renamer.NameForSymbol(e.Ref)) {
if canUseShorthandProperty(key.Value, p.renamer.NameForSymbol(e.Ref), property.Flags) {
if p.options.AddSourceMappings {
p.addSourceMappingForName(property.Key.Loc, helpers.UTF16ToString(key.Value), e.Ref)
}
Expand All @@ -1259,7 +1259,7 @@ func (p *printer) printProperty(property js_ast.Property) {
case *js_ast.EImportIdentifier:
// Make sure we're not using a property access instead of an identifier
ref := ast.FollowSymbols(p.symbols, e.Ref)
if symbol := p.symbols.Get(ref); symbol.NamespaceAlias == nil && helpers.UTF16EqualsString(key.Value, p.renamer.NameForSymbol(ref)) &&
if symbol := p.symbols.Get(ref); symbol.NamespaceAlias == nil && canUseShorthandProperty(key.Value, p.renamer.NameForSymbol(ref), property.Flags) &&
p.options.ConstValues[ref].Kind == js_ast.ConstValueNone {
if p.options.AddSourceMappings {
p.addSourceMappingForName(property.Key.Loc, helpers.UTF16ToString(key.Value), ref)
Expand All @@ -1276,6 +1276,21 @@ func (p *printer) printProperty(property js_ast.Property) {
}
}

// The JavaScript specification special-cases the property identifier
// "__proto__" with a colon after it to set the prototype of the object.
// If we keep the identifier but add a colon then we'll cause a behavior
// change because the prototype will now be set. Avoid using an identifier
// by using a computed property with a string instead. For more info see:
// https://tc39.es/ecma262/#sec-runtime-semantics-propertydefinitionevaluation
if property.Flags.Has(js_ast.PropertyWasShorthand) && !p.options.UnsupportedFeatures.Has(compat.ObjectExtensions) &&
helpers.UTF16EqualsString(key.Value, "__proto__") {
p.print("[")
p.addSourceMapping(property.Key.Loc)
p.printQuotedUTF16(key.Value, 0)
p.print("]")
break
}

p.addSourceMapping(property.Key.Loc)
p.printIdentifierUTF16(key.Value)
} else {
Expand Down Expand Up @@ -1314,6 +1329,20 @@ func (p *printer) printProperty(property js_ast.Property) {
}
}

func canUseShorthandProperty(key []uint16, name string, flags js_ast.PropertyFlags) bool {
// The JavaScript specification special-cases the property identifier
// "__proto__" with a colon after it to set the prototype of the object. If
// we remove the colon then we'll cause a behavior change because the
// prototype will no longer be set, but we also don't want to add a colon
// if it was omitted. Always use a shorthand property if the property is not
// "__proto__", otherwise try to preserve the original shorthand status. See:
// https://tc39.es/ecma262/#sec-runtime-semantics-propertydefinitionevaluation
if !helpers.UTF16EqualsString(key, name) {
return false
}
return helpers.UTF16EqualsString(key, name) && (name != "__proto__" || flags.Has(js_ast.PropertyWasShorthand))
}

func (p *printer) printQuotedUTF16(data []uint16, flags printQuotedFlags) {
if p.options.UnsupportedFeatures.Has(compat.TemplateLiteral) {
flags &= ^printQuotedAllowBacktick
Expand Down
18 changes: 11 additions & 7 deletions internal/js_printer/js_printer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,6 @@ import (
"github.com/evanw/esbuild/internal/test"
)

func assertEqual(t *testing.T, a interface{}, b interface{}) {
t.Helper()
if a != b {
t.Fatalf("%s != %s", a, b)
}
}

func expectPrintedCommon(t *testing.T, name string, contents string, expected string, options config.Options) {
t.Helper()
t.Run(name, func(t *testing.T) {
Expand Down Expand Up @@ -511,6 +504,17 @@ func TestObject(t *testing.T) {
expectPrinted(t, "let x = () => ({}.x)", "let x = () => ({}).x;\n")
expectPrinted(t, "let x = () => ({} = {})", "let x = () => ({} = {});\n")
expectPrinted(t, "let x = () => (x, {} = {})", "let x = () => (x, {} = {});\n")

// "{ __proto__: __proto__ }" must not become "{ __proto__ }"
expectPrinted(t, "function foo(__proto__) { return { __proto__: __proto__ } }", "function foo(__proto__) {\n return { __proto__: __proto__ };\n}\n")
expectPrinted(t, "function foo(__proto__) { return { '__proto__': __proto__ } }", "function foo(__proto__) {\n return { \"__proto__\": __proto__ };\n}\n")
expectPrinted(t, "function foo(__proto__) { return { ['__proto__']: __proto__ } }", "function foo(__proto__) {\n return { [\"__proto__\"]: __proto__ };\n}\n")
expectPrinted(t, "import { __proto__ } from 'foo'; let foo = () => ({ __proto__: __proto__ })", "import { __proto__ } from \"foo\";\nlet foo = () => ({ __proto__: __proto__ });\n")
expectPrinted(t, "import { __proto__ } from 'foo'; let foo = () => ({ '__proto__': __proto__ })", "import { __proto__ } from \"foo\";\nlet foo = () => ({ \"__proto__\": __proto__ });\n")
expectPrinted(t, "import { __proto__ } from 'foo'; let foo = () => ({ ['__proto__']: __proto__ })", "import { __proto__ } from \"foo\";\nlet foo = () => ({ [\"__proto__\"]: __proto__ });\n")

// Don't use ES6+ features (such as a shorthand or computed property name) in ES5
expectPrintedTarget(t, 5, "function foo(__proto__) { return { __proto__ } }", "function foo(__proto__) {\n return { __proto__: __proto__ };\n}\n")
}

func TestFor(t *testing.T) {
Expand Down

0 comments on commit ac36537

Please sign in to comment.