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

[Ivy] ExpressionChangedAfterItHasBeenCheckedError when using bypassSecurityTrustStyle in a property getter #33448

Closed
fr0 opened this issue Oct 28, 2019 · 4 comments
Assignees
Labels
area: core Issues related to the framework runtime freq2: medium regression Indicates than the issue relates to something that worked in a previous version state: has PR type: bug/fix
Milestone

Comments

@fr0
Copy link

fr0 commented Oct 28, 2019

🐞 bug report

Affected Package

The issue is caused by package @angular/core

Is this a regression?

Yes, it works in non-ivy mode

Description

A clear and concise description of the problem...
  <div [style.background-image]="iconSafe"></div>

   ...

  get iconSafe() {
    return this.sanitizer.bypassSecurityTrustStyle(`url(${this.icon})`);
  }
core.js:6047 ERROR Error: ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: 'SafeValue must use [property]=binding: url(https://i.imgur.com/4AiXzf8.jpg) (see http://g.co/ng/security#xss)'. Current value: 'SafeValue must use [property]=binding: url(https://i.imgur.com/4AiXzf8.jpg) (see http://g.co/ng/security#xss)'.

🔬 Minimal Reproduction

https://github.com/fr0/angular-ivy-test2

  1. npm install
  2. ng serve
  3. open http://localhost:4200 and press F12, observe error in console

🔥 Exception or Error


core.js:6047 ERROR Error: ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: 'SafeValue must use [property]=binding: url(https://i.imgur.com/4AiXzf8.jpg) (see http://g.co/ng/security#xss)'. Current value: 'SafeValue must use [property]=binding: url(https://i.imgur.com/4AiXzf8.jpg) (see http://g.co/ng/security#xss)'.

🌍 Your Environment

Angular Version:


Angular CLI: 9.0.0-next.16
Node: 12.12.0
OS: darwin x64
Angular: 9.0.0-next.14
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.900.0-next.16
@angular-devkit/build-angular     0.900.0-next.16
@angular-devkit/build-optimizer   0.900.0-next.16
@angular-devkit/build-webpack     0.900.0-next.16
@angular-devkit/core              9.0.0-next.16
@angular-devkit/schematics        9.0.0-next.16
@angular/cli                      9.0.0-next.16
@ngtools/webpack                  9.0.0-next.16
@schematics/angular               9.0.0-next.16
@schematics/update                0.900.0-next.16
rxjs                              6.5.3
typescript                        3.6.4
webpack                           4.41.2

@AndrewKushnir AndrewKushnir added the area: core Issues related to the framework runtime label Oct 28, 2019
@ngbot ngbot bot added this to the needsTriage milestone Oct 28, 2019
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Oct 29, 2019
@kara kara modified the milestones: Backlog, v9-candidates, v10-candidates Nov 5, 2019
@IgorMinar
Copy link
Contributor

IgorMinar commented Nov 8, 2019

I was able to repro this with rc.1.

Oddly enough the error occurs with:

export class AppComponent {
  icon = 'https://i.imgur.com/4AiXzf8.jpg';

  get iconSafe() {
    return this.sanitizer.bypassSecurityTrustStyle(`url(${this.icon})`);
  }
  ...
}

But not with:

export class AppComponent {
  icon = 'https://i.imgur.com/4AiXzf8.jpg';
  iconSafe = this.sanitizer.bypassSecurityTrustStyle(`url(${this.icon})`);

  ...
}

This makes sense because the getter creates a new Safe value every time it's called.

With VE (ivyEnabled: false) the issue doesn't repro at all.

@IgorMinar IgorMinar removed this from the v9-candidates milestone Nov 8, 2019
@ngbot ngbot bot added this to the Backlog milestone Nov 8, 2019
@IgorMinar IgorMinar modified the milestones: Backlog, v9-blockers Nov 8, 2019
@IgorMinar IgorMinar added regression Indicates than the issue relates to something that worked in a previous version and removed regression Indicates than the issue relates to something that worked in a previous version labels Nov 8, 2019
@IgorMinar
Copy link
Contributor

IgorMinar commented Nov 9, 2019

I looked into this more and the behavior of VE is that if a bound value is of SafeStyle type we don't throw ExpressionChangedAfterItHasBeenCheckedError regardless of the contents of the SafeStyle values.

I don't think that this is ideal (we should really compare the wrapped value), but a good first step should be to match VE in this behavior and treat all SafeStyle values as equal for the purposes of ExpressionChangedAfterItHasBeenCheckedError.

@waterplea
Copy link
Contributor

Isn't this expected though? This getter generates new object each change detection cycle. So when dev mode performs extra check — it does get a new object. Why wasn't it present with VE?

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 16, 2019
@kara kara added regression Indicates than the issue relates to something that worked in a previous version and removed severity5: ivy-compat labels Feb 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: core Issues related to the framework runtime freq2: medium regression Indicates than the issue relates to something that worked in a previous version state: has PR type: bug/fix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants