Problem/Motivation

Currently after #2549791: Remove SafeMarkup::xssFilter() and provide ability to define #markup escaping strategy and what tags to filter got commited, TMGMT test are broken on current core. Need to do some research to find the issue which is related to SafeMarkup.

Proposed resolution

look for the bug and fix it.
delete SafeMarkup::xssFilter() from the module and test to see if everything works, might happen in other modules as well.

Remaining tasks

Update description giving more details, create a patch review and commit.

User interface changes

Data model changes

Original report by [username]

Comments

juanse254 created an issue. See original summary.

juanse254’s picture

Assigned: Unassigned » juanse254
juanse254’s picture

Issue summary: View changes
juanse254’s picture

Status: Active » Needs review
StatusFileSize
new452 bytes

This will fix the tests.

Status: Needs review » Needs work

The last submitted patch, 4: Safe_Markups_XSS_Filter-2553801-4.patch, failed testing.

juanse254’s picture

Status: Needs work » Needs review
StatusFileSize
new509 bytes
new394 bytes

okay, this might do it. Its weird but working, feedback apreciated.

juanse254’s picture

Priority: Normal » Critical
LKS90’s picture

Status: Needs review » Needs work

I'd suggest only removing the xssFilter() method call.
The test fails that will occur are because the FileTranslator has raw HTML in it's message, which isn't filtered anymore. Fix the broken message in FileTranslator instead of filtering the message you want to use in drupal_set_message().

      $job->submitted('Exported file can be downloaded <a href="!link">here</a>.', array('!link' => file_create_url($path)));

Edit: Drupal::l() and @link, like in Collect.

juanse254’s picture

Status: Needs work » Needs review
StatusFileSize
new1.4 KB
new1.61 KB
new1.4 KB

Okay this way then!.

Status: Needs review » Needs work

The last submitted patch, 9: Safe_Markups_XSS_Filter-2553801-9.patch, failed testing.

berdir’s picture

Status: Needs work » Needs review
StatusFileSize
new490 bytes

This seems to do the trick for me.

Status: Needs review » Needs work

The last submitted patch, 11: Safe_Markups_XSS_Filter-2553801-11.patch, failed testing.

berdir’s picture

Status: Needs work » Needs review
StatusFileSize
new10.68 KB

Discussed this with @alexpott, turns out the actual problem were !placeholder's, so lets stop using that. Also, we should call t() even if we have no placeholders, no idea why we don't do that atm.

berdir’s picture

StatusFileSize
new10.74 KB

Wrong patch, I think.

berdir’s picture

And, forgot to rename test methods again.

Status: Needs review » Needs work

The last submitted patch, 15: Safe_Markups_XSS_Filter-2553801-15.patch, failed testing.

juanse254’s picture

Status: Needs work » Needs review
StatusFileSize
new9.82 KB
new1.59 KB

Some minor changes you missed, now it should pass.

berdir’s picture

Status: Needs review » Fixed

Thanks. Committed.

  • Berdir committed f5be274 on 8.x-1.x authored by juanse254
    Issue #2553801 by juanse254, Berdir: !placeholders break safe strings,...

The last submitted patch, 13: Safe_Markups_XSS_Filter-2553801-13.patch, failed testing.

The last submitted patch, 14: Safe_Markups_XSS_Filter-2553801-14.patch, failed testing.

Status: Fixed » Closed (fixed)

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