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

Text filter "Default Blacklist" removes HTML comments - no whitelisting possible #21248

Open
maggus opened this issue Jul 24, 2018 · 19 comments
Open

Comments

@maggus
Copy link

maggus commented Jul 24, 2018

Is your feature request related to a problem? Please describe.

Currently the "Default Blacklist" text filter removes YOOtheme Pro JSON data from articles as this data is masked by an HTML comment - probably in order to not be visible inside search results.

This issue can also be viewed here: https://yootheme.com/support/question/120930

The YOOtheme Pro developers don't see any possibility to whitelist HTML comments inside the current text filters.

Describe the solution you'd like

Add a possibility to whitelist HTML comments inside Joomla!'s text filters.

Additional context

Thank you for looking into it!

@maggus maggus changed the title Text filter "Default Blacklist" removes HTML comments Text filter "Default Blacklist" removes HTML comments - no whitelisting possible Jul 24, 2018
@zero-24
Copy link
Member

zero-24 commented Jul 25, 2018

What is YOOtheme Pro JSON data how does it look like and how can this be reproduced? Thanks

@brianteeman
Copy link
Contributor

You might be able to do it using the tinymce filters

@maggus
Copy link
Author

maggus commented Jul 25, 2018

YOOtheme Pro is a theme and page builder for Joomla!.

The data stored in Joomla! articles could look like this:
<p>Kurse im Überblick</p>
<p>In unseren Erste-Hilfe-Kursen lernen Sie, wie Sie sich im Notfall richtig verhalten und Leben retten können.</p>
<p>Von Gymnastik bis Yoga: Stärken Sie Ihr Wohlbefinden und Ihre Fitness– mit unseren Gesundheitskursen!</p>
<p>In den Familienkursen genießen junge Eltern mit ihren Kindern gemeinsame Zeit und können sich Anregungen holen.</p>
<hr id="system-readmore" />
<!-- {"name":"","type":"layout","children":[{"name":"Beitrag","type":"section","props":{"style":"default","width":"default","image_position":"center-center","id":"tm-main","class":"tm-main"},"children":[{"name":"","type":"row","props":{"fixed_width":"large","breakpoint":"m","layout":"1-1"},"children":[{"name":"","type":"column","props":{"image_position":"center-center","widths":["1-1"]},"children":[{"name":"","type":"joomla_position","props":{"layout":"stack","breakpoint":"m","content":"breadcrumb"}},{"name":"","type":"headline","props":{"title_element":"h1","content":"Kurse im \u00dcberblick"}}]}]},{"name":"","type":"row","props":{"fixed_width":"large","breakpoint":"s","layout":",,"},"children":[{"name":"","type":"column","props":{"image_position":"center-center","widths":[""]},"children":[{"name":"","type":"panel","props":{"link_text":"Weiterlesen","title_element":"h3","meta_style":"meta","meta_align":"bottom","icon_ratio":4,"image_align":"top","image_grid_width":"1-2","image_breakpoint":"m","link_style":"","margin":"default","title":"Erste Hilfe","content":"In unseren Erste-Hilfe-Kursen lernen Sie, wie Sie sich im Notfall richtig verhalten und Leben retten k\u00f6nnen.","link":"index.php?option=com_content&view=article&id=13&Itemid=114","image":"images\/articles\/predefined\/kurs_erste_hilfe_wb18299.jpg","image_alt":"Foto: Eine DRK-Ausbilderin f\u00fchrt den Teilnehmern die Herzdruckmassage an einem Dummy vor."}}]},{"name":"","type":"column","props":{"image_position":"center-center","widths":[""]},"children":[{"name":"","type":"panel","props":{"link_text":"Weiterlesen","title_element":"h3","meta_style":"meta","meta_align":"bottom","icon_ratio":4,"image_align":"top","image_grid_width":"1-2","image_breakpoint":"m","link_style":"","margin":"default","title":"Gesundheitskurse","content":"Von Gymnastik bis Yoga: St\u00e4rken Sie Ihr Wohlbefinden und Ihre Fitness\u2013 mit unseren Gesundheitskursen! ","link":"index.php?option=com_content&view=article&id=14&Itemid=115","image":"images\/articles\/predefined\/kurs_gymnastik_wb18391.jpg","image_alt":"Foto: Bei strahlendem Sonnenschein macht eine Frau \u00dcbungen mit einem Gymnastikband auf einer Wiese. Die Kursleiterin korrigiert behutsam die Haltung der Frau bei dieser \u00dcbung."}}]},{"name":"","type":"column","props":{"image_position":"center-center","widths":[""]},"children":[{"name":"","type":"panel","props":{"link_text":"Weiterlesen","title_element":"h3","meta_style":"meta","meta_align":"bottom","icon_ratio":4,"image_align":"top","image_grid_width":"1-2","image_breakpoint":"m","link_style":"","margin":"default","title":"Kurse f\u00fcr Familien","content":"In den Familienkursen genie\u00dfen junge Eltern mit ihren Kindern gemeinsame Zeit und k\u00f6nnen sich Anregungen holen.","link":"index.php?option=com_content&view=article&id=15&Itemid=116","image":"images\/articles\/predefined\/kurse_fuer_familien_wb26306.jpg","image_alt":"Foto: Ein Kleinstkind liegt - auf seine Unterarme gest\u00fctzt - auf einer blauen \u00dcbungsmatte. Es h\u00e4lt eine rote DRK-Badeente in den H\u00e4nden und blickt fragend in die Kamera."}}]}]}]}]} -->

With text filter "Default Blacklist" enabled and after clicking the save button of an article on the frontend the html code looks like this:
<p>Kurse im Überblick</p>
<p>In unseren Erste-Hilfe-Kursen lernen Sie, wie Sie sich im Notfall richtig verhalten und Leben retten können.</p>
<p>Von Gymnastik bis Yoga: Stärken Sie Ihr Wohlbefinden und Ihre Fitness– mit unseren Gesundheitskursen!</p>
<p>In den Familienkursen genießen junge Eltern mit ihren Kindern gemeinsame Zeit und können sich Anregungen holen.</p>

The HTML comment is completely removed and so is the readmore element - probably as there is nothing left after the readmore element.

The problem is that there seems to be no kind of 'tag name' that can be used to whitelist HTML comments. So the only solution to keep the HTML comment seems to be disabling Joomla!'s text filters at all for now (i.e. set to 'no filtering')

UPDATE[2018-07-26 00:43]: Maybe !-- could be used as a 'tag name' but unfortunately there is no option to whitelist this special tag within the 'Default Blacklist'.

@mbabker
Copy link
Contributor

mbabker commented Jul 25, 2018

No HTML comment makes it past this check, no matter the configuration given to the InputFilter class constructor. More specifically, the regex that's the first part of that if condition.

@brianteeman
Copy link
Contributor

Joomla is working as designed and expected
Your code is trying to do something that joomla was not designed to do
There really is no option for you (unless you change your code)

@maggus
Copy link
Author

maggus commented Jul 26, 2018

@brianteeman You are completely right! Joomla! works as expected! Therefore this is not a bug report but a feature request.

I am looking for a way to whitelist HTML comments without changing Joomla! Core files. Since it is recommended to use the Default Blacklist for Administrators this is on my mind.

Unfortunately I am just some power user of YOOtheme Pro and not its developer. But I have passed this thread to the developers so that they may join that conversation with some suggestions.

Thank you for looking into it anyway!

@steffans
Copy link

@mbabker Would you consider the possibility to whitelist HTML comments inside Joomla!'s text filters? Since they are valid HTML tags, there is no reason to blacklist them.

@brianteeman
Copy link
Contributor

it is a potential security issue to allow html comments from untrusted users. Hence the restrictions. I am sure you will understand why we wont publicly disclose anything further

@maggus
Copy link
Author

maggus commented Jul 26, 2018

@brianteeman So, it is not about the JSON object that is potential risk but the html comment itself? What in your opinion would be a "safe" tag to place an additional JSON object?

I am pretty sure YOOtheme could implement equivalent text filters inside the page builder itself so that this JSON object does not contain unwanted or risky tags.

@mbabker
Copy link
Contributor

mbabker commented Jul 26, 2018

If there is some security issue with HTML comments, to be frank, there needs to be a regression test for that in the filter API. There is no commentary or test covering HTML comments to the best of my knowledge. So either it is purposefully excluded by people who are no longer contributing to the project (who would be the only ones to understand the reasoning) or it was never considered in the API's design and adding support is a valid request.

@Hackwar
Copy link
Member

Hackwar commented Mar 23, 2019

From my perspective there are several ways to store such data. You could use an attribute of a span, you could store it in a custom field, in a separate table or in an XML structure instead of JSON. If you enable the blacklist, I don't see why we should allow HTML comments in there and thus I would propose to deny this feature request. Admittedly, I'm also requesting this to clean up the tracker.

@steffans
Copy link

@Hackwar HTML comments are valid tags and intended for any metadata (notes, etc.) within the markup. Comment tags are not visually shown and do not interfere with the actual markup. For example other editors like Gutenberg use the comment tags for metadata as well.

So why blacklist them at all?

@Skullbock
Copy link
Contributor

I agree with @steffans here. Power users would also like to leave their comments in html pages without having them removed.
This doesn't go against what @brianteeman said regarding "untrusted users". We just need to allow for comments to be whitelisted. @steffans maybe try with a PR for this?

@maggus
Copy link
Author

maggus commented Mar 26, 2019

I have very little GitHub experience. Thus, if a pull request is something that has to be done by the original author please tell me how to do this.

Thank you for discussing this issue anyway!

@joomla-cms-bot
Copy link

Set to "closed" on behalf of @jwaisner by The JTracker Application at issues.joomla.org/joomla-cms/21248

@jwaisner
Copy link
Member

Closing this feature request as J3 is not available for feature adds. Please review J4 and request for J4 if this feature is wanted.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21248.

@brianteeman
Copy link
Contributor

re-opened as its a valid feature request and flagged re-evaluate J4

@bato3
Copy link

bato3 commented Sep 15, 2020

OT: Isn't good idea use comments to store data. Eg. Cloudflare (Proxy + DDOS protection) removes comments. Better use data- attribute. Or even html tag defined yourself.

Also "as pro" i don't need comments, because I have prevous versions

@steffans
Copy link

@bato3 In this case the comments store data which is then processed on the server and transformed into a HTML layout. As mentioned before other page/block editors use the same technique for storing layout data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

No branches or pull requests