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

I9421 v-pkp-allowed-html #320

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jardakotesovec
Copy link
Collaborator

No description provided.

Comment on lines 3 to 4
const allowed_html =
'a[href|target|title],em,strong,cite,code,ul,ol,li[class],dl,dt,dd,b,i,u,img[src|alt],sup,sub,br,p';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this will be fed from the back end later, but just flagging to not be forgotten 😁

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonasraoni I was wondering whether it is worth it... but its probably quite easy.. will add it..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually.. now I realised that it has to be passed as its something people might want customise..

Comment on lines +20 to +21
// DOMPurify does not support defining tags along with attributes, its separate lists
// https://github.com/cure53/DOMPurify/issues/272
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a sample implementation here (I just skimmed the code, and it seems to be ok):
https://www.linkedin.com/feed/update/urn:li:activity:7034210540885282816/

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jonasraoni Thanks, I have seen it. But I don't think it makes that much of the difference as its meant to be for security rather than for validation. And customising it also requires more in depth testing (ideally creating some test coverage) and also check performance characteristics.

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

Successfully merging this pull request may close these issues.

2 participants