fix(core): warn when style property bindings receive invalid values

Report unsupported style property binding values in development mode while preserving existing binding behavior. Unwrap trusted style values before appending unit suffixes and link NG0318 warnings to the corresponding error guide.

(cherry picked from commit e43eb96341)

adev/src/content/reference/errors/NG0318.md

@@ -0,0 +1,28 @@
+# Invalid style property binding value
+
+Angular detected a value with an unsupported type in a style property binding. This can occur in
+an individual binding such as `[style.width]` or in a property of a `[style]` map.
+
+Style property values accept strings, numbers, style `SafeValue` objects, `null`, and `undefined`.
+Other values, including `SafeValue` objects created for a non-style security context, produce this
+warning in development mode.
+
+For example, the following binding passes a boolean instead of a CSS value:
+
+```angular-html
+<div [style.display]="isVisible"></div>
+```
+
+## Debugging the error
+
+Use the property name and value in the error message to locate the binding. Convert the value to a
+valid CSS string or number, or use `null` or `undefined` to remove the style.
+
+When using a unit suffix such as `[style.width.px]`, bind the numeric portion of the value:
+
+```angular-html
+<div [style.width.px]="width"></div>
+```
+
+Do not pass a `SafeValue` created for another security context, such as trusted HTML, to a style
+property binding.

adev/src/content/reference/errors/overview.md

@@ -15,6 +15,7 @@
 | `NG0300`  | [Selector Collision](errors/NG0300)                                                  |
 | `NG0301`  | [Export Not Found](errors/NG0301)                                                    |
 | `NG0302`  | [Pipe Not Found](errors/NG0302)                                                      |
+| `NG0318`  | [Invalid style property binding value](errors/NG0318)                                |
 | `NG0401`  | [Missing platform](errors/NG0401)                                                    |
 | `NG0403`  | [Bootstrapped NgModule doesn't specify which component to initialize](errors/NG0403) |
 | `NG0500`  | [Hydration Node Mismatch](errors/NG0500)                                             |

goldens/public-api/core/errors.api.md

@@ -100,6 +100,8 @@ export const enum RuntimeErrorCode {
     // (undocumented)
     INVALID_SKIP_HYDRATION_HOST = -504,
     // (undocumented)
+    INVALID_STYLE_PROP_VALUE = -318,
+    // (undocumented)
     LOOP_TRACK_DUPLICATE_KEYS = -955,
     // (undocumented)
     LOOP_TRACK_RECREATE = -956,

packages/core/src/errors.ts

@@ -66,6 +66,7 @@ export const enum RuntimeErrorCode {
   NO_BINDING_TARGET = 315,
   INVALID_BINDING_TARGET = 316,
   INVALID_SET_INPUT_CALL = 317,
+  INVALID_STYLE_PROP_VALUE = -318,
 
   // Bootstrap Errors
   MULTIPLE_PLATFORMS = 400,

packages/core/src/render3/instructions/styling.ts

@@ -6,7 +6,13 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import {SafeValue, unwrapSafeValue} from '../../sanitization/bypass';
+import {formatRuntimeError, RuntimeErrorCode} from '../../errors';
+import {
+  BypassType,
+  getSanitizationBypassType,
+  SafeValue,
+  unwrapSafeValue,
+} from '../../sanitization/bypass';
 import {KeyValueArray, keyValueArrayGet, keyValueArraySet} from '../../util/array_utils';
 import {
   assertDefined,
@@ -216,6 +222,9 @@ export function checkStylingProperty(
     stylingFirstUpdatePass(tView, prop, bindingIndex, isClassBased);
   }
   if (value !== NO_CHANGE && bindingUpdated(lView, bindingIndex, value)) {
+    if (ngDevMode && !isClassBased) {
+      warnInvalidStylePropValue(prop, value);
+    }
     const tNode = tView.data[getSelectedIndex()] as TNode;
     updateStyling(
       tView,
@@ -230,6 +239,36 @@ export function checkStylingProperty(
   }
 }
 
+function warnInvalidStylePropValue(prop: string, value: unknown): void {
+  if (
+    value == null ||
+    typeof value === 'string' ||
+    typeof value === 'number' ||
+    getSanitizationBypassType(value) === BypassType.Style
+  ) {
+    return;
+  }
+
+  console.warn(
+    formatRuntimeError(
+      RuntimeErrorCode.INVALID_STYLE_PROP_VALUE,
+      `\`[style.${prop}]\` was bound to an invalid value. ` +
+        `Expected a string, number, SafeValue, null, or undefined, but received ` +
+        `\`${typeof value}\` (\`${stringifyInvalidStylePropValue(value)}\`).`,
+    ),
+  );
+}
+
+function stringifyInvalidStylePropValue(value: unknown): string {
+  try {
+    return stringify(value);
+  } catch {
+    // Invalid values may have throwing accessors or conversion methods. A development diagnostic
+    // must not interrupt the binding update when the value cannot be represented in the warning.
+    return '[unstringifiable value]';
+  }
+}
+
 /**
  * Common code between `ɵɵclassMap` and `ɵɵstyleMap`.
  *
@@ -687,10 +726,7 @@ export function toStylingKeyValueArray(
   if (value == null /*|| value === undefined */ || value === '') return EMPTY_ARRAY as any;
   const styleKeyValueArray: KeyValueArray<any> = [] as any;
   const unwrappedValue = unwrapSafeValue(value) as
-    | string
-    | string[]
-    | Set<string>
-    | {[key: string]: any};
+    string | string[] | Set<string> | {[key: string]: any};
   if (Array.isArray(unwrappedValue)) {
     for (let i = 0; i < unwrappedValue.length; i++) {
       keyValueArraySet(styleKeyValueArray, unwrappedValue[i], true);
@@ -726,6 +762,7 @@ export function toStylingKeyValueArray(
  * @param value The value to set.
  */
 export function styleKeyValueArraySet(keyValueArray: KeyValueArray<any>, key: string, value: any) {
+  ngDevMode && warnInvalidStylePropValue(key, value);
   keyValueArraySet(keyValueArray, key, unwrapSafeValue(value));
 }
 
@@ -997,7 +1034,7 @@ function normalizeSuffix(
     // As it produce invalid CSS, which the browsers will automatically omit but Domino will not.
     // Example: `"left": "px;"` instead of `"left": ""`.
   } else if (typeof suffix === 'string') {
-    value = value + suffix;
+    value = unwrapSafeValue(value) + suffix;
   } else if (typeof value === 'object') {
     value = stringify(unwrapSafeValue(value));
   }

packages/core/test/render3/instructions/styling_spec.ts

@@ -10,6 +10,7 @@ import {AttributeMarker, DirectiveDef} from '../../../src/render3';
 import {ɵɵdefineDirective} from '../../../src/render3/definition';
 import {
   classStringParser,
+  styleKeyValueArraySet,
   styleStringParser,
   toStylingKeyValueArray,
   ɵɵclassProp,
@@ -32,6 +33,10 @@ import {
 import {HEADER_OFFSET, TVIEW} from '../../../src/render3/interfaces/view';
 import {getLView, leaveView, setBindingRootForHostBindings} from '../../../src/render3/state';
 import {getNativeByIndex} from '../../../src/render3/util/view_utils';
+import {
+  bypassSanitizationTrustHtml,
+  bypassSanitizationTrustStyle,
+} from '../../../src/sanitization/bypass';
 import {keyValueArraySet} from '../../../src/util/array_utils';
 import {getElementClasses, getElementStyles} from '../../../testing/src/styling';
 
@@ -423,6 +428,46 @@ describe('styling', () => {
             'x',
           ] as any);
         });
+
+        it('should accept supported style values', () => {
+          const warnSpy = spyOn(console, 'warn');
+
+          expect(
+            toStylingKeyValueArray(styleKeyValueArraySet, null!, {
+              color: 'red',
+              width: 10,
+              display: bypassSanitizationTrustStyle('block'),
+              opacity: null,
+            }),
+          ).toEqual(['color', 'red', 'display', 'block', 'opacity', null, 'width', 10] as any);
+          expect(warnSpy).not.toHaveBeenCalled();
+        });
+
+        it('should warn about unsupported style values', () => {
+          const warnSpy = spyOn(console, 'warn');
+
+          expect(
+            toStylingKeyValueArray(styleKeyValueArraySet, null!, {
+              display: true,
+              color: bypassSanitizationTrustHtml('red'),
+            }),
+          ).toEqual(['color', 'red', 'display', true] as any);
+          expect(warnSpy).toHaveBeenCalledTimes(2);
+          expect(warnSpy).toHaveBeenCalledWith(
+            'NG0318: `[style.display]` was bound to an invalid value. ' +
+              'Expected a string, number, SafeValue, null, or undefined, but received ' +
+              '`boolean` (`true`). ' +
+              'Find more at https://next.angular.dev/errors/NG0318',
+          );
+          expect(warnSpy).toHaveBeenCalledWith(
+            'NG0318: `[style.color]` was bound to an invalid value. ' +
+              'Expected a string, number, SafeValue, null, or undefined, but received `object` ' +
+              '(`SafeValue must use [property]=binding: red ' +
+              '(see https://angular.dev/best-practices/security#preventing-cross-site-scripting-xss)`). ' +
+              'Find more at https://next.angular.dev/errors/NG0318',
+          );
+        });
+
         it('should parse objects with null prototype', () => {
           const nullProtoObj = Object.assign(Object.create(null), {X: 'x', A: 'a'});
           expect(toStylingKeyValueArray(keyValueArraySet, null!, nullProtoObj)).toEqual([

packages/core/test/render3/instructions_spec.ts

@@ -197,6 +197,150 @@ describe('instructions', () => {
   });
 
   describe('styleProp', () => {
+    it('should warn when a style property is bound to a boolean', () => {
+      const warnSpy = spyOn(console, 'warn');
+      const t = new ViewFixture({
+        create: createDiv,
+        update: () => ɵɵstyleProp('display', true),
+        decls: 1,
+        vars: 2,
+      });
+
+      t.update();
+
+      expect(warnSpy).toHaveBeenCalledOnceWith(
+        'NG0318: `[style.display]` was bound to an invalid value. ' +
+          'Expected a string, number, SafeValue, null, or undefined, but received `boolean` (`true`). ' +
+          'Find more at https://next.angular.dev/errors/NG0318',
+      );
+    });
+
+    it('should warn when a style property is bound to an object', () => {
+      const warnSpy = spyOn(console, 'warn');
+      const t = new ViewFixture({
+        create: createDiv,
+        update: () => ɵɵstyleProp('color', {r: 255, g: 0, b: 0}),
+        decls: 1,
+        vars: 2,
+      });
+
+      t.update();
+
+      expect(warnSpy).toHaveBeenCalledOnceWith(
+        'NG0318: `[style.color]` was bound to an invalid value. ' +
+          'Expected a string, number, SafeValue, null, or undefined, but received ' +
+          '`object` (`[object Object]`). ' +
+          'Find more at https://next.angular.dev/errors/NG0318',
+      );
+    });
+
+    it('should warn when a style property is bound to a non-style SafeValue', () => {
+      const warnSpy = spyOn(console, 'warn');
+      const t = new ViewFixture({
+        create: createDiv,
+        update: () => ɵɵstyleProp('color', bypassSanitizationTrustHtml('red')),
+        decls: 1,
+        vars: 2,
+      });
+
+      t.update();
+
+      expect(warnSpy).toHaveBeenCalledOnceWith(
+        'NG0318: `[style.color]` was bound to an invalid value. ' +
+          'Expected a string, number, SafeValue, null, or undefined, but received `object` ' +
+          '(`SafeValue must use [property]=binding: red ' +
+          '(see https://angular.dev/best-practices/security#preventing-cross-site-scripting-xss)`). ' +
+          'Find more at https://next.angular.dev/errors/NG0318',
+      );
+    });
+
+    it('should unwrap a style SafeValue before appending a suffix', () => {
+      const warnSpy = spyOn(console, 'warn');
+      const t = new ViewFixture({
+        create: createDiv,
+        update: () => ɵɵstyleProp('width', bypassSanitizationTrustStyle('100'), 'px'),
+        decls: 1,
+        vars: 2,
+      });
+
+      t.update();
+
+      expect((t.host.firstChild as HTMLElement).style.width).toBe('100px');
+      expect(warnSpy).not.toHaveBeenCalled();
+    });
+
+    it('should warn when a style property with a suffix is bound to a non-style SafeValue', () => {
+      const warnSpy = spyOn(console, 'warn');
+      const t = new ViewFixture({
+        create: createDiv,
+        update: () => ɵɵstyleProp('width', bypassSanitizationTrustHtml('100'), 'px'),
+        decls: 1,
+        vars: 2,
+      });
+
+      t.update();
+
+      expect(warnSpy).toHaveBeenCalledOnceWith(
+        'NG0318: `[style.width]` was bound to an invalid value. ' +
+          'Expected a string, number, SafeValue, null, or undefined, but received `object` ' +
+          '(`SafeValue must use [property]=binding: 100 ' +
+          '(see https://angular.dev/best-practices/security#preventing-cross-site-scripting-xss)`). ' +
+          'Find more at https://next.angular.dev/errors/NG0318',
+      );
+    });
+
+    it('should continue binding when an invalid value has a throwing getter', () => {
+      const warnSpy = spyOn(console, 'warn');
+      const value = Object.assign(() => {}, {toString: () => 'applied-value'});
+      Object.defineProperty(value, 'overriddenName', {
+        get: () => {
+          throw new Error('Unexpected read of overriddenName');
+        },
+      });
+      const t = new ViewFixture({
+        create: createDiv,
+        update: () => ɵɵstyleProp('--test-value', value),
+        decls: 1,
+        vars: 2,
+      });
+
+      t.update();
+
+      expect((t.host.firstChild as HTMLElement).style.getPropertyValue('--test-value')).toBe(
+        'applied-value',
+      );
+      expect(warnSpy).toHaveBeenCalledOnceWith(
+        'NG0318: `[style.--test-value]` was bound to an invalid value. ' +
+          'Expected a string, number, SafeValue, null, or undefined, but received ' +
+          '`function` (`[unstringifiable value]`). ' +
+          'Find more at https://next.angular.dev/errors/NG0318',
+      );
+    });
+
+    it('should not warn when a style property is bound to a supported value', () => {
+      const warnSpy = spyOn(console, 'warn');
+      let value: string | number | SafeValue | null | undefined = 'block';
+      const t = new ViewFixture({
+        create: createDiv,
+        update: () => ɵɵstyleProp('display', value),
+        decls: 1,
+        vars: 2,
+      });
+
+      for (const supportedValue of [
+        'inline',
+        1,
+        bypassSanitizationTrustStyle('block'),
+        null,
+        undefined,
+      ]) {
+        value = supportedValue;
+        t.update();
+      }
+
+      expect(warnSpy).not.toHaveBeenCalled();
+    });
+
     it('should allow values even if a bypass operation is applied', () => {
       let backgroundImage: string | SafeValue = 'url("http://server")';
       const t = new ViewFixture({
@@ -244,6 +388,25 @@ describe('instructions', () => {
       fixture.update();
       expect(fixture.html).toEqual('<div style="background-color: red; height: 10px;"></div>');
     });
+
+    it('should warn when a style map property is bound to a boolean', () => {
+      const warnSpy = spyOn(console, 'warn');
+      const fixture = new ViewFixture({
+        create: createDivWithStyle,
+        update: () => ɵɵstyleMap({display: true}),
+        decls: 1,
+        vars: 2,
+        consts: attrs,
+      });
+
+      fixture.update();
+
+      expect(warnSpy).toHaveBeenCalledOnceWith(
+        'NG0318: `[style.display]` was bound to an invalid value. ' +
+          'Expected a string, number, SafeValue, null, or undefined, but received `boolean` (`true`). ' +
+          'Find more at https://next.angular.dev/errors/NG0318',
+      );
+    });
   });
 
   describe('elementClass', () => {