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

test of poorly nested or complex youtube markup in a comment #3630

Closed
wants to merge 3 commits into from

Conversation

jywarren
Copy link
Member

@jywarren jywarren commented Oct 5, 2018

This demonstrates an issue with improperly parsed HTML markup in a comment, which can cut off the rest of a webpage by not closing the <iframe> tag.

It uses this text: https://gist.github.com/jywarren/dcee2c01d2dcfb969845d89374848fa1

Which gets displayed with the iframe tag like:

| <p>\<iframe width="560" height="315" src="https://www.youtube.com/embed/Kt\_MSMpxy7Y" frameborder="0" allow="autoplay; encrypted-media" allowfullscreen\>\&lt;/iframe></p>

@ghost ghost assigned jywarren Oct 5, 2018
@ghost ghost added the in progress label Oct 5, 2018
@jywarren
Copy link
Member Author

jywarren commented Oct 5, 2018

This should be solvable with https://stackoverflow.com/questions/9933333/sanitize-html-and-close-incomplete-tags

Nokogiri::HTML::DocumentFragment.parse(html).to_html

@plotsbot
Copy link
Collaborator

plotsbot commented Oct 5, 2018

2 Warnings
⚠️ It looks like you merged from master in this pull request. Please rebase to get rid of the merge commits – you may want to rewind the master branch and rebase instead of merging in from master, which can cause problems when accepting new code!
⚠️ This pull request cannot be merged yet due to merge conflicts. Please resolve them – probably by rebasing – and ask for help (in the comments, or in the chatroom if you get into trouble!.
2 Messages
📖 @jywarren Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.
📖 This pull request doesn’t link to a issue number. Please refer to the issue it fixes (if any) in the body of your PR, in the format: Fixes #123.

Generated by 🚫 Danger

@grvsachdeva
Copy link
Member

@jywarren please update the status of this PR. Thanks!

@SidharthBansal
Copy link
Member

@jywarren we can include this in GCI if you want
cc\ @VladimirMikulic @Uzay-G

@jywarren
Copy link
Member Author

jywarren commented Jan 17, 2020 via email

@SidharthBansal
Copy link
Member

Awesome!

jywarren pushed a commit that referenced this pull request Jan 18, 2020
Unit tests:
 - "sanitizing comment body for XSS"
 - "should close incomplete tags"

Resolves #3630 #3553
NitinBhasneria pushed a commit to NitinBhasneria/plots2 that referenced this pull request Jan 21, 2020
Unit tests:
 - "sanitizing comment body for XSS"
 - "should close incomplete tags"

Resolves publiclab#3630 publiclab#3553
vinitshahdeo pushed a commit to vinitshahdeo/plots2 that referenced this pull request Feb 1, 2020
Unit tests:
 - "sanitizing comment body for XSS"
 - "should close incomplete tags"

Resolves publiclab#3630 publiclab#3553
NitinBhasneria pushed a commit to NitinBhasneria/plots2 that referenced this pull request Feb 5, 2020
Unit tests:
 - "sanitizing comment body for XSS"
 - "should close incomplete tags"

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

Successfully merging this pull request may close these issues.

None yet

4 participants