From ac365374f9054493aa07530ae1fe8524d26cb617 Mon Sep 17 00:00:00 2001 From: Evan Wallace Date: Sun, 18 Feb 2024 17:46:45 -0800 Subject: [PATCH] fix #3651: handle `__proto__` edge cases better --- CHANGELOG.md | 39 ++++++ .../bundler_tests/bundler_default_test.go | 121 ++++++++++++++++++ .../snapshots/snapshots_default.txt | 102 +++++++++++++++ internal/js_printer/js_printer.go | 33 ++++- internal/js_printer/js_printer_test.go | 18 ++- 5 files changed, 304 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b97f558a1b..1a4ea1936f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/internal/bundler_tests/bundler_default_test.go b/internal/bundler_tests/bundler_default_test.go index afad3fe1471..f89eb8ff862 100644 --- a/internal/bundler_tests/bundler_default_test.go +++ b/internal/bundler_tests/bundler_default_test.go @@ -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, + }, + }) +} diff --git a/internal/bundler_tests/snapshots/snapshots_default.txt b/internal/bundler_tests/snapshots/snapshots_default.txt index b813d26f5e8..e8a1a32d6b7 100644 --- a/internal/bundler_tests/snapshots/snapshots_default.txt +++ b/internal/bundler_tests/snapshots/snapshots_default.txt @@ -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 ---------- diff --git a/internal/js_printer/js_printer.go b/internal/js_printer/js_printer.go index a42c83c6061..f6cc144f50f 100644 --- a/internal/js_printer/js_printer.go +++ b/internal/js_printer/js_printer.go @@ -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) } @@ -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) @@ -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 { @@ -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 diff --git a/internal/js_printer/js_printer_test.go b/internal/js_printer/js_printer_test.go index 6a5759daa99..fed0c71a9d6 100644 --- a/internal/js_printer/js_printer_test.go +++ b/internal/js_printer/js_printer_test.go @@ -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) { @@ -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) {