Currently internal css mutation plugin assumes that there is css to be added via drupal_add_css but it might not always be the case (such as A/B test control variant)

CommentFileSizeAuthor
#3 multivariate_internal_css_check_2088773_3.patch1.21 KBnonsie
#1 multivariate_internal_css_check_2088773_1.patch1.22 KBnonsie

Comments

nonsie’s picture

nonsie
she/her
Primary language Estonian
commented
Status: Active » Needs review
StatusFileSize
new multivariate_internal_css_check_2088773_1.patch1.22 KB

And a patch

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented

Generally agree, but:

+++ b/plugins/mutations/multivariate_internal_css/multivariate_internal_css.module
@@ -10,8 +10,10 @@ function multivariate_internal_css_boot() {
+            drupal_add_css(check_plain($active_mutations[$mutation_id]['internal_css']), array('type' => 'inline', 'weight' => 1000, 'group' => CSS_THEME));

Is a check_plain really right here? I would expect it is intended to protect HTML, not CSS.

nonsie’s picture

nonsie
she/her
Primary language Estonian
commented
StatusFileSize
new multivariate_internal_css_check_2088773_3.patch1.21 KB

Here's a patch with check_plain() removed. I checked other modules that provide similar functionality and none of them sanitize user input.

pivica’s picture

pivica commented
Status: Needs review » Fixed

Thx, commited.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.