# Makes closure/html types' constructors private

Also makes them throw an error in goog.DEBUG rather than set an empty value. This allows us to simplify the shape of the code in a way that it is easier to optimize & removes the need to preserve the token in production builds RELNOTES: Makes closure/html types' constructors private PiperOrigin-RevId: 527443596 Change-Id: I0a6d2db03a93c839c2381697de2aa98e5ca3f835
google · Apr 27, 2023 · f3db3e6 · f3db3e6
1 parent 918acbb
commit f3db3e6
Show file tree

Hide file tree

Showing 12 changed files with 75 additions and 15 deletions.
diff --git a/closure/goog/html/safehtml.js b/closure/goog/html/safehtml.js
@@ -82,18 +82,23 @@ const CONSTRUCTOR_TOKEN_PRIVATE = {};
  */
 class SafeHtml {
   /**
+   * @private
    * @param {!TrustedHTML|string} value
    * @param {!Object} token package-internal implementation detail.
    */
   constructor(value, token) {
+    if (goog.DEBUG && token !== CONSTRUCTOR_TOKEN_PRIVATE) {
+      throw Error('SafeHtml is not meant to be built directly');
+    }
+
     /**
      * The contained value of this SafeHtml.  The field has a purposely ugly
      * name to make (non-compiled) code that attempts to directly access this
      * field stand out.
+     * @const
      * @private {!TrustedHTML|string}
      */
-    this.privateDoNotAccessOrElseSafeHtmlWrappedValue_ =
-        (token === CONSTRUCTOR_TOKEN_PRIVATE) ? value : '';
+    this.privateDoNotAccessOrElseSafeHtmlWrappedValue_ = value;
 
     /**
      * @override

diff --git a/closure/goog/html/safehtml_test.js b/closure/goog/html/safehtml_test.js
@@ -34,6 +34,11 @@ testSuite({
     stubs.reset();
   },
 
+  testConstructor_throwsOnBadToken() {
+    assertThrows(() => new (/** @type {?} */ (SafeHtml))(''));
+    assertThrows(() => new (/** @type {?} */ (SafeHtml.EMPTY)).constructor(''));
+  },
+
   testSafeHtml() {
     // TODO(xtof): Consider using SafeHtmlBuilder instead of newSafeHtmlForTest,
     // when available.

diff --git a/closure/goog/html/safescript.js b/closure/goog/html/safescript.js
@@ -71,14 +71,18 @@ class SafeScript {
    * @param {!Object} token package-internal implementation detail.
    */
   constructor(value, token) {
+    if (goog.DEBUG && token !== CONSTRUCTOR_TOKEN_PRIVATE) {
+      throw Error('SafeScript is not meant to be built directly');
+    }
+
     /**
      * The contained value of this SafeScript.  The field has a purposely ugly
      * name to make (non-compiled) code that attempts to directly access this
      * field stand out.
+     * @const
      * @private {!TrustedScript|string}
      */
-    this.privateDoNotAccessOrElseSafeScriptWrappedValue_ =
-        (token === CONSTRUCTOR_TOKEN_PRIVATE) ? value : '';
+    this.privateDoNotAccessOrElseSafeScriptWrappedValue_ = value;
 
     /**
      * @override

diff --git a/closure/goog/html/safescript_test.js b/closure/goog/html/safescript_test.js
@@ -24,6 +24,12 @@ testSuite({
     stubs.reset();
   },
 
+  testConstructor_throwsOnBadToken() {
+    assertThrows(() => new (/** @type {?} */ (SafeScript))(''));
+    assertThrows(
+        () => new (/** @type {?} */ (SafeScript.EMPTY)).constructor(''));
+  },
+
   testSafeScript() {
     const script = 'var string = \'hello\';';
     const safeScript = SafeScript.fromConstant(Const.from(script));

diff --git a/closure/goog/html/safestyle.js b/closure/goog/html/safestyle.js
@@ -118,14 +118,18 @@ class SafeStyle {
    * @param {!Object} token package-internal implementation detail.
    */
   constructor(value, token) {
+    if (goog.DEBUG && token !== CONSTRUCTOR_TOKEN_PRIVATE) {
+      throw Error('SafeStyle is not meant to be built directly');
+    }
+
     /**
      * The contained value of this SafeStyle.  The field has a purposely
      * ugly name to make (non-compiled) code that attempts to directly access
      * this field stand out.
+     * @const
      * @private {string}
      */
-    this.privateDoNotAccessOrElseSafeStyleWrappedValue_ =
-        (token === CONSTRUCTOR_TOKEN_PRIVATE) ? value : '';
+    this.privateDoNotAccessOrElseSafeStyleWrappedValue_ = value;
 
     /**
      * @override

diff --git a/closure/goog/html/safestyle_test.js b/closure/goog/html/safestyle_test.js
@@ -33,6 +33,12 @@ testSuite({
     stubs.reset();
   },
 
+  testConstructor_throwsOnBadToken() {
+    assertThrows(() => new (/** @type {?} */ (SafeStyle))(''));
+    assertThrows(
+        () => new (/** @type {?} */ (SafeStyle.EMPTY)).constructor(''));
+  },
+
   testSafeStyle() {
     const style = 'width: 1em;height: 1em;';
     const safeStyle = SafeStyle.fromConstant(Const.from(style));

diff --git a/closure/goog/html/safestylesheet.js b/closure/goog/html/safestylesheet.js
@@ -73,14 +73,18 @@ class SafeStyleSheet {
    * @param {!Object} token package-internal implementation detail.
    */
   constructor(value, token) {
+    if (goog.DEBUG && token !== CONSTRUCTOR_TOKEN_PRIVATE) {
+      throw Error('SafeStyleSheet is not meant to be built directly');
+    }
+
     /**
      * The contained value of this SafeStyleSheet.  The field has a purposely
      * ugly name to make (non-compiled) code that attempts to directly access
      * this field stand out.
+     * @const
      * @private {string}
      */
-    this.privateDoNotAccessOrElseSafeStyleSheetWrappedValue_ =
-        (token === CONSTRUCTOR_TOKEN_PRIVATE) ? value : '';
+    this.privateDoNotAccessOrElseSafeStyleSheetWrappedValue_ = value;
 
     /**
      * @override

diff --git a/closure/goog/html/safestylesheet_test.js b/closure/goog/html/safestylesheet_test.js
@@ -26,6 +26,12 @@ function assertCreateRuleEquals(expected, selector, style) {
 }
 
 testSuite({
+  testConstructor_throwsOnBadToken() {
+    assertThrows(() => new (/** @type {?} */ (SafeStyleSheet))(''));
+    assertThrows(
+        () => new (/** @type {?} */ (SafeStyleSheet.EMPTY)).constructor(''));
+  },
+
   testSafeStyleSheet() {
     const styleSheet = 'P.special { color:red ; }';
     const safeStyleSheet = SafeStyleSheet.fromConstant(Const.from(styleSheet));

diff --git a/closure/goog/html/safeurl.js b/closure/goog/html/safeurl.js
@@ -64,14 +64,18 @@ goog.html.SafeUrl = class {
    * @param {!Object} token package-internal implementation detail.
    */
   constructor(value, token) {
+    if (goog.DEBUG && token !== goog.html.SafeUrl.CONSTRUCTOR_TOKEN_PRIVATE_) {
+      throw Error('SafeUrl is not meant to be built directly');
+    }
+
     /**
      * The contained value of this SafeUrl.  The field has a purposely ugly
      * name to make (non-compiled) code that attempts to directly access this
      * field stand out.
+     * @const
      * @private {string}
      */
-    this.privateDoNotAccessOrElseSafeUrlWrappedValue_ =
-        (token === goog.html.SafeUrl.CONSTRUCTOR_TOKEN_PRIVATE_) ? value : '';
+    this.privateDoNotAccessOrElseSafeUrlWrappedValue_ = value;
   }
 
   /**

diff --git a/closure/goog/html/safeurl_test.js b/closure/goog/html/safeurl_test.js
@@ -54,6 +54,12 @@ testSuite({
     stubs.reset();
   },
 
+  testConstructor_throwsOnBadToken() {
+    assertThrows(() => new (/** @type {?} */ (SafeUrl))(''));
+    assertThrows(
+        () => new (/** @type {?} */ (SafeUrl.ABOUT_BLANK)).constructor(''));
+  },
+
   testSafeUrl() {
     const safeUrl = SafeUrl.fromConstant(Const.from('javascript:trusted();'));
     const extracted = SafeUrl.unwrap(safeUrl);

diff --git a/closure/goog/html/trustedresourceurl.js b/closure/goog/html/trustedresourceurl.js
@@ -50,21 +50,24 @@ goog.require('goog.string.TypedString');
  */
 goog.html.TrustedResourceUrl = class {
   /**
+   * @private
    * @param {!TrustedScriptURL|string} value
    * @param {!Object} token package-internal implementation detail.
    */
   constructor(value, token) {
+    if (goog.DEBUG &&
+        token !== goog.html.TrustedResourceUrl.CONSTRUCTOR_TOKEN_PRIVATE_) {
+      throw Error('TrustedResourceUrl is not meant to be built directly');
+    }
+
     /**
      * The contained value of this TrustedResourceUrl.  The field has a
      * purposely ugly name to make (non-compiled) code that attempts to directly
      * access this field stand out.
      * @const
      * @private {!TrustedScriptURL|string}
      */
-    this.privateDoNotAccessOrElseTrustedResourceUrlWrappedValue_ =
-        (token === goog.html.TrustedResourceUrl.CONSTRUCTOR_TOKEN_PRIVATE_) ?
-        value :
-        '';
+    this.privateDoNotAccessOrElseTrustedResourceUrlWrappedValue_ = value;
   }
 
   /**
@@ -188,7 +191,8 @@ goog.html.TrustedResourceUrl.unwrapTrustedScriptURL = function(
     return trustedResourceUrl
         .privateDoNotAccessOrElseTrustedResourceUrlWrappedValue_;
   } else {
-    goog.asserts.fail('expected object of type TrustedResourceUrl, got \'' +
+    goog.asserts.fail(
+        'expected object of type TrustedResourceUrl, got \'' +
         trustedResourceUrl + '\' of type ' + goog.typeOf(trustedResourceUrl));
     return 'type_error:TrustedResourceUrl';
   }

diff --git a/closure/goog/html/trustedresourceurl_test.js b/closure/goog/html/trustedresourceurl_test.js
@@ -47,6 +47,12 @@ testSuite({
     stubs.reset();
   },
 
+  testConstructor_throwsOnBadToken() {
+    assertThrows(() => new (/** @type {?} */ (TrustedResourceUrl))('', {}));
+    const url = TrustedResourceUrl.fromConstant(Const.from(''));
+    assertThrows(() => new (/** @type {?} */ (url)).constructor('', {}));
+  },
+
   testTrustedResourceUrl() {
     const url = 'javascript:trusted();';
     const trustedResourceUrl = TrustedResourceUrl.fromConstant(Const.from(url));