Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I'm getting "NOT Check an order's state" (note the double-encoded html entities) on "admin/config/workflow/rules/reaction/manage/" page when I negate a condition that reads properly "Check an order's state" without negation.
Comment | File | Size | Author |
---|---|---|---|
#8 | 1945006-8-htmlescape.patch | 4.22 KB | TR |
| |||
#8 | 1945006-8-htmlescape-test-only.patch | 3.71 KB | TR |
#7 | 1063108-7-htmlescape.patch | 515 bytes | TR |
| |||
#3 | 1945006-3-htmlescape.patch | 516 bytes | TR |
Comments
Comment #1
ptmkenny CreditAttribution: ptmkenny commentedAre you using Commerce or Ubercart? If so, you should report this to that module's issue queue, not Rules.
Comment #2
TR CreditAttribution: TR commentedConfirmed that this still happens in the latest 7.x-2.x release, but it is absolutely NOT a problem with Ubercart - this is a problem with Rules (or, more specifically, the '#type' => 'link' render element) improperly double-encoding the condition name when the the negate text is added. The only reason it shows up with an Ubercart condition and not any of the built-in core Rules conditions is because the core Rules conditions don't use special characters like ' which can be HTML encoded.
To see that this really is a Rules problem, you can simply edit one of the core Rules conditions (such as 'Data comparison' in modules/data.rules.inc) and change the name to include an apostrophe. Then clear your caches and use that condition (negated) in a rule - you will see the exact same behavior as described in the original post.
The problem does not exist in Rules 8.x-3.x, just in 7.x-2.x.
Comment #3
TR CreditAttribution: TR commentedHere's a patch that fixes this problem. I would appreciate it if someone would apply this patch and confirm that.
Patch looks like:
The problem was that using @ here prematurely sanitizes $label when the NOT is prepended. Note that in the original code $label is not sanitized here except in the case where NOT is prepended. Thus, this is not introducing a security problem - the patch is just treating NOT $label the same as we were previously treating $label. The security problem would only be if $label wasn't sanitized before output, which occurs elsewhere in the code. Drupal policy is to sanitize before output, and Rules DOES do that. And we know Rules does that, because that's why we were getting a DOUBLE escape when we sanitized $label here.
Comment #4
TR CreditAttribution: TR commentedComment #5
TR CreditAttribution: TR commentedComment #7
TR CreditAttribution: TR commentedTestbot does not like fuzz. Re-roll against current HEAD.
(EDIT: It seems I named the patch incorrectly. But the contents are as intended - it's just the name that's wrong)
Comment #8
TR CreditAttribution: TR commentedOkay, here's a test-only patch that demonstrates and reproduces the error described by the original poster. This patch should FAIL because of the bug.
Also, here's a patch with the same test but also with the fix posted in #7. This patch should PASS because the fix solves the problem.
Comment #10
TR CreditAttribution: TR commentedTests results in #8 are as expected - the test-only patch adds tests to demonstrate the problem in the current -dev, and the other patch fixes the problem and proves the fix with the new tests.
Comment #12
TR CreditAttribution: TR commentedCommitted.