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

Allow components to use unsafe image urls #218

Closed
stasusov opened this issue Oct 12, 2018 · 3 comments
Closed

Allow components to use unsafe image urls #218

stasusov opened this issue Oct 12, 2018 · 3 comments

Comments

@stasusov
Copy link
Contributor

stasusov commented Oct 12, 2018

What is the expected behavior?

Allow components to use image urls that are not safe

What is the current behavior?

Images accessed by unsafe url are not displayed when used in <gallery-image><gallery-image>

What are the steps to reproduce?

https://stackblitz.com/edit/ngx-gallery-avkn7t

  1. Add the "security bypassed" (with bypassSecurityTrustUrl()) "unsafe" url to [src]
  2. use gallerize directive for the container of <img> tags
  3. Click on one of the images. You can see Angular warnings in console, because url is not safe

What is the use-case or motivation for changing an existing behavior?

If developer passed image url through bypassSecurityTrustUrl(), it should be shown.

Which versions are you using for the following packages?

Angular: 6
Angular CDK: 6.4.7
Angular CLI: 6.0.0
Typescript: 2.7.2
Gallery: 3.3.1

@MurhafSousli
Copy link
Owner

Hi @stasusov It is not about the sanitizing I think, it could be caused of URL.createObjectURL I guess, I am not sure! is there another way to create a URL from the http response or store t as inline data instead.

@stasusov
Copy link
Contributor Author

stasusov commented Oct 13, 2018

@MurhafSousli, thank you.

You are right, the createObjectUrl() generates an "unsafe" url, that is not inserted to the template by default. That's why in a demo (home.component.ts), I am explicitly make the url "trusted". But there is no such "trust" procedure in the ngx-gallery components, so any "unsafe" urls are removed. That's what the messages you can see in the console warning about and why no image is displayed on click. But, if we add this procedure, image is shown, because the url is safe for Angular now.

For example,
If we change the line
<ng-container [lazyImage]="src" (loaded)="loadedImage = $event" (error)="error.emit($event)">
to
<ng-container [lazyImage]="src" (loaded)="loadedImage = trustCss($event)" (error)="error.emit($event)">
and update the component with

trustCss(imageUrl: string) {
    return this._sanitizer.bypassSecurityTrustStyle(imageUrl);
}

images are shown correctly.

This is acceptable in terms of security in my case (using gallerize directive), because if url is unsafe, I'll receive a warning from Angular on the step of adding it (to the <img/> or background) to one of my templates. But I am not aware of the other cases of library usage (again, in terms of security), so I can't just make a PR with these changes.

Does this explanation make any sense?

UPD: I've just noticed that the same component (gallery-image) already uses sanitizer to bypass check for custom HTML, so the library is responsible for a safety issue. Maybe this can help you somehow.

stasusov pushed a commit to stasusov/ngx-gallery that referenced this issue Oct 24, 2018
stasusov added a commit to stasusov/ngx-gallery that referenced this issue Oct 24, 2018
…-to-use-unsafe-urls

MurhafSousli#218: Allow gallery-image component to use unsafe urls
stasusov pushed a commit to stasusov/ngx-gallery that referenced this issue Oct 24, 2018
stasusov added a commit to stasusov/ngx-gallery that referenced this issue Oct 24, 2018
MurhafSousli added a commit that referenced this issue Oct 27, 2018
 #218: Allow gallery-image component to use unsafe urls
@MurhafSousli
Copy link
Owner

Fixed in 4.0.0 beta

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants