Problem/Motivation

The xssFilter() method has been removed. Instead there is an additional attribute in a render array to list tags which are allowed in the markup.

Proposed resolution

Remove the call to SafeMarkup::xssFilter() and add an attribute with allowed tags instead.

Remaining tasks

Review patch, commit.

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#10 catch_up_remove-2554463-10-interdiff.txt2.02 KBberdir
#10 catch_up_remove-2554463-10.patch10.97 KBberdir
#8 catch_up_remove-2554463-8.patch11.22 KBberdir
#7 interdiff_5-7.txt5.52 KBLKS90
#7 catch_up_remove-2554463-7.patch8.13 KBLKS90
#5 interdiff_3-5.txt1.29 KBLKS90
#5 catch_up_remove-2554463-5.patch2.94 KBLKS90
#3 catch_up_remove-2554463-3.patch2.95 KBLKS90

Comments

LKS90 created an issue. See original summary.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Priority: Normal » Critical

This breaks admin/reports/monitoring
Fatal error: Call to undefined method Drupal\Component\Utility\SafeMarkup::xssFilter()...

LKS90’s picture

LKS90 commented
Status: Active » Needs review
StatusFileSize
new catch_up_remove-2554463-3.patch2.95 KB

Here is the patch that removes the calls to SafeMarkup::xssFilter(). For the setMessage() call, I have to cleanup the message so the html markup is dropped.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Needs work 
+++ b/src/Plugin/monitoring/SensorPlugin/PhpNoticesSensorPlugin.php
@@ -42,7 +42,13 @@ class PhpNoticesSensorPlugin extends WatchdogAggregatorSensorPlugin {
+        $variables['!' . substr($key, 1)] = $variables[$key];
+        if (substr($key, 0, 1) === '%') {
+          unset($variables[$key]);

No no no.
List the ones that are relevant. You can name them: type, message, function, line, file. Nothing else.

LKS90’s picture

LKS90 commented
Status: Needs work » Needs review
StatusFileSize
new catch_up_remove-2554463-5.patch2.94 KB
new interdiff_3-5.txt1.29 KB

I simplified the removal of the sanitation characters. Now the message is formatted correctly.

Status: Needs review » Needs work

The last submitted patch, 5: catch_up_remove-2554463-5.patch, failed testing.

LKS90’s picture

LKS90 commented
Status: Needs work » Needs review
StatusFileSize
new catch_up_remove-2554463-7.patch8.13 KB
new interdiff_5-7.txt5.52 KB

Also fixed the fails in the CoreKernelTest related to the Sensor message and the replacement tags. Now we are still waiting for Captcha and Search API (which are both green in the tests, they only need a new release)

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
StatusFileSize
new catch_up_remove-2554463-8.patch11.22 KB

It's @message, not %message. And you missed a few.

Status: Needs review » Needs work

The last submitted patch, 8: catch_up_remove-2554463-8.patch, failed testing.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review
StatusFileSize
new catch_up_remove-2554463-10.patch10.97 KB
new catch_up_remove-2554463-10-interdiff.txt2.02 KB

Fixed the tests, we don't need that replacement stuff when we use @, which we have to since that's what core is doing.

Status: Needs review » Needs work

The last submitted patch, 10: catch_up_remove-2554463-10.patch, failed testing.

  • Berdir committed aa97b5c on 8.x-1.x 
    Issue #2554463 by LKS90, Berdir: Remove SafeMarkup::xssFilter() function...
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Fixed

Committed yesterday, forgot to update the status.

Status: Fixed » Closed (fixed)

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

The last submitted patch, 3: catch_up_remove-2554463-3.patch, failed testing.

The last submitted patch, 7: catch_up_remove-2554463-7.patch, failed testing.

The last submitted patch, 3: catch_up_remove-2554463-3.patch, failed testing.

The last submitted patch, 5: catch_up_remove-2554463-5.patch, failed testing.

The last submitted patch, 7: catch_up_remove-2554463-7.patch, failed testing.

The last submitted patch, 8: catch_up_remove-2554463-8.patch, failed testing.

Status: Closed (fixed) » Needs work

The last submitted patch, 10: catch_up_remove-2554463-10.patch, failed testing.

giancarlosotelo’s picture

giancarlosotelo commented
Status: Needs work » Closed (fixed)