Support from Acquia helps fund testing for Drupal Acquia logo

Comments

David_Rothstein created an issue. See original summary.

Jill L’s picture

Status: Patch (to be ported) » Needs review
FileSize
1.29 KB

I've ported the patch to D7, and added the test to filter.test.

Not sure about the placement/description of the test, but it seems to function fine.

poker10’s picture

Status: Needs review » Needs work

Thanks for the patch. I have reviewed it and have one question - why in D10 we have this:

  if (preg_match('/^([-a-zA-Z][-a-zA-Z0-9]*)/', $attributes, $match)) {

And in D7 patch this?

@@ -1595,11 +1595,11 @@ function _filter_xss_attributes($attr) {
-  if (preg_match('/^([-a-zA-Z]+)/', $attr, $match)) {
+  if (preg_match('/^([-a-zA-Z]+[-a-zA-Z0-9]*)/', $attr, $match)) {

It seems to me that the plus sign should not be there. The same applies for the second change in the patch.

The patch also does not apply anymore, so the reroll is needed. Please check the problem mentioned above while doing rerolls.

The last submitted patch, 4: 2847553-4_test-only.patch, failed testing. View results

poker10’s picture

Issue tags: +Pending Drupal 7 commit

Adding a tag for the review from another D7 maintainer before commit.

  • mcdruid committed 6b7e2b1a on 7.x
    Issue #2847553 by poker10, Jill L, David_Rothstein: XSS attribute...
mcdruid’s picture

Status: Needs review » Fixed
Issue tags: -Pending Drupal 7 commit

Thanks; great that we added a test.

Status: Fixed » Closed (fixed)

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