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

feat: adds :targetblank scrubber #275

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

stefannibrasil
Copy link
Contributor

[fixes #272]

New scrubber :targetblank - adds a target="_blank" attribute to all links. If there is a target already set, replaces it with target="_blank".

  link_farmers_markup = "ohai! <a href='http://www.myswarmysite.com/'>I like your blog post</a>"
  Loofah.html5_fragment(link_farmers_markup).scrub!(:targetblank)
  => "ohai! <a href='http://www.myswarmysite.com/' target="_blank">I like your blog post</a>"

(co-authored by @thdaraujo)

`:targetblank` adds a `target="_blank"` attribute to all links.
If there is a target already set, replaces it with `target="_blank"`.

```
  link_farmers_markup = "ohai! <a href='http://www.myswarmysite.com/'>I like your blog post</a>"
  Loofah.html5_fragment(link_farmers_markup).scrub!(:targetblank)
  => "ohai! <a href='http://www.myswarmysite.com/' target="_blank">I like your blog post</a>"
```

Co-authored-by: Thiago Araujo <thd.araujo@gmail.com>
@stefannibrasil
Copy link
Contributor Author

CI errors seem to be related to rubygems/rubygems#7064

@thdaraujo
Copy link
Contributor

thdaraujo commented Oct 15, 2023

seems related to jruby/jruby#7962

@stefannibrasil
Copy link
Contributor Author

@flavorjones this one is passing now :)

@flavorjones
Copy link
Owner

Looks great! Thank you so much. Merging.

@flavorjones flavorjones merged commit 5345bb7 into flavorjones:main Nov 13, 2023
12 checks passed
@flavorjones
Copy link
Owner

Just released https://github.com/flavorjones/loofah/releases/tag/v2.22.0 with this in it! Thank you again!

@n00dle
Copy link

n00dle commented Nov 14, 2023

Does this introduce a security issue without the rel attribute?

https://perishablepress.com/fix-blank-target-vulnerability/

@flavorjones
Copy link
Owner

@n00dle Thanks for asking this question. The assumption is that people who are using the :targetblank scrubber know what they're doing. But it might be worth updating the documentation to suggest that it not be used on untrusted content ... would you be willing to submit a PR updating the docstring and the README?

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.

Add scrub to append target=_blank to all links
4 participants