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

[4.0] Allow form elements in a subform template (sanitisation) #34514

Merged
merged 6 commits into from
Jun 15, 2021

Conversation

dgrammatiko
Copy link
Contributor

@dgrammatiko dgrammatiko commented Jun 14, 2021

Pull Request for Issue #34512 (the subform part) .

Summary of Changes

  • Revert the sanitisation here

Testing Instructions

Actual result BEFORE applying this Pull Request

Expected result AFTER applying this Pull Request

Documentation Changes Required

@wilsonge can you push the button here?

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Jun 14, 2021
@dgrammatiko dgrammatiko marked this pull request as draft June 14, 2021 11:49
@Fedik
Copy link
Member

Fedik commented Jun 14, 2021

hm, do we really need cleaning for subform "template"?
it can be pretty complex for a forms, there may be fields with custom element, and fields with onclick attributes

@dgrammatiko
Copy link
Contributor Author

and fields with onclick attributes

This is exactly what's not allowed and what the sanitizer is supposed to clean 😉

@Fedik
Copy link
Member

Fedik commented Jun 14, 2021

then subfrom will be broken for these fields,
there may be fields that use it

@dgrammatiko
Copy link
Contributor Author

then subfrom will be broken for these fields, there may be fields that use it

Hard call (thankfully not my call). Allowing inline on-events is XSS prone. Disallowing on-events is stricter but probably all elements will be required to be defined custom-elements (own constructor etc). I'm ok reverting this. @wilsonge your call...

@Fedik
Copy link
Member

Fedik commented Jun 14, 2021

I understood the intention, and it good one.
but it pretty hard.

@richard67
Copy link
Member

@dgrammatiko
Copy link
Contributor Author

I understood the intention, and it good one.
but it pretty hard.

Till there's a viable solution for sanitization of the subform templates the changes have been reverted

@dgrammatiko dgrammatiko marked this pull request as ready for review June 14, 2021 19:16
@joomdonation
Copy link
Contributor

I have tested this item ✅ successfully on 934208e


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

1 similar comment
@alikon
Copy link
Contributor

alikon commented Jun 15, 2021

I have tested this item ✅ successfully on 934208e


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

@alikon
Copy link
Contributor

alikon commented Jun 15, 2021

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jun 15, 2021
@richard67 richard67 added this to the Joomla 4.0 milestone Jun 15, 2021
@richard67 richard67 merged commit 48187e8 into joomla:4.0-dev Jun 15, 2021
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 15, 2021
@richard67
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NPM Resource Changed This Pull Request can't be tested by Patchtester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants