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

Modernize Youtube embeds and exempt from Bleach cleaning, abandon Imgur embeds #384

Merged
merged 3 commits into from
Aug 8, 2021

Conversation

HebaruSan
Copy link
Contributor

@HebaruSan HebaruSan commented Aug 7, 2021

Problem

https://spacedock.info/markdown says:

You can also embed YouTube videos and Imgur albums by wrapping the URL in brackets (specific to SpaceDock):

[[https://www.youtube.com/watch?v=T4rfHAqs9EI]]

However, if you try to use this currently, it just renders as an escaped inline <iframe>:

https://spacedock.info/mod/141/scatterer

image

Cause

#336 added a Bleach sanitization pass to our Markdown rendering, which only allows certain tags and attributes to be included in the rendered HTML. Currently iframe is not on the allowed list, and the above embedding functionality works by generating an iframe element.

Changes

Now iframe is allowed, but we apply extra validation to it. Only the attributes generated by SD's embed code are allowed, and for src specifically, only values beginning with the URL prefixes we use are allowed. If someone tries to use an iframe with an unauthorized URL, it will just show up as empty:

image

Fixes #383.
Fixes #178.

Side fixes

The embedding functionality was using a deprecated API (assigning a Pattern directly to a member of md.inlinePatterns) and is now updated to the newer register API. As part of this the KerbDown-related classes are refactored somewhat to put certain values and functions in the classes where they belong.

If you click "SHARE" on Youtube, it presents you with a link in the format https://youtu.be/video_id. Now we support embedding this format.

Imgur's embedding hasn't worked in a long time, and shows no signs of ever being fixed. Now it's removed from the code and from the markdown help page.

@HebaruSan HebaruSan added Type: Bug Area: Backend Related to the Python code that runs inside gunicorn Priority: Normal Status: Ready labels Aug 7, 2021
KerbalStuff/kerbdown.py Outdated Show resolved Hide resolved
@HebaruSan HebaruSan changed the title Modernize Youtube and Imgur embeds and exempt from Bleach cleaning Modernize Youtube embeds and exempt from Bleach cleaning, abandon Imgur embeds Aug 8, 2021
Copy link
Member

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Works marvelously. And doesn't compromise security, for what I can say with my very limited knowledge in that area.

@HebaruSan HebaruSan merged commit b06bb7b into KSP-SpaceDock:alpha Aug 8, 2021
@HebaruSan HebaruSan deleted the fix/embeds branch August 8, 2021 16:56
This was referenced Oct 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Backend Related to the Python code that runs inside gunicorn Priority: Normal Status: Ready Type: Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants