-
Notifications
You must be signed in to change notification settings - Fork 38
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
fix(showcase): prevent arbitrary url evaluation #1955
Conversation
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 604eba0. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
c99dc31
to
4431f53
Compare
encapsulation: ViewEncapsulation.None, | ||
changeDetection: ChangeDetectionStrategy.OnPush | ||
}) | ||
export class OtterIconPresComponent { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why a component more that a pipe?
Here you just want to calculate the realUrl
of an icon (I also think that the alt
should not be the url of the icon but a meaningful short description)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree in general that the alt
should be a meaningful description, but in that specific case we are displaying the image that has been uploaded to create a pet. There is no description for that image and I thought in that specific case the url was relevant
As for component vs pipe, I used a component with the idea that we could want to change some html/css on these images but at the end it might not be the case, so I'm ok to only use a pipe
4431f53
to
7b9cffd
Compare
apps/showcase/src/components/utilities/otter-picker/otter-icon-path.pipe.ts
Outdated
Show resolved
Hide resolved
apps/showcase/src/components/utilities/otter-picker/otter-icon-path.pipe.ts
Show resolved
Hide resolved
7b9cffd
to
af84ab5
Compare
af84ab5
to
604eba0
Compare
Proposed change
Related issues