In the sample batch operation, strings stored in $context['results'] and $context['message'] should be check_plain()'ed.

CommentFileSizeAuthor
#5 852120-5-batch-example-sanitize.patch1.91 KBcygri
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Title: Documentation problem with Batch operations » Batch operations example doesn't do proper sanitizing
Version: 6.x-dev » 7.x-dev

Good catch!

This issue relates to http://api.drupal.org/api/group/batch/6

That page already has a note:

Note: if the batch 'title', 'init_message', 'progress_message', or 'error_message' could contain any user input, it is the responsibility of the code calling batch_set() to sanitize them first with a function like check_plain() or filter_xss().

But then the example code doesn't actually do that.

I'm not sure whether the 'results' have to be check_plain()'ed though... Someone should verify that.

The D7 version of this documentation is nearly (or completely?) identical, and suffers from the same problem. So it should be fixed there first and then ported back to D6.

pfournier’s picture

I can confirm that the results have to be check_plain()'ed in D6.

jhodgdon’s picture

If that's the case, then that should be added to the note about sanitizing the other stuff.

Anonymous’s picture

Assigned: Unassigned » linclark

Tagging as mine for now, I'm going to be showing 3 people how to roll patches tomorrow and will use this as an example.

cygri’s picture

Status: Active » Needs review
FileSize
1.91 KB

This patch adds check_plain() around user input in both examples. Also extended the note that lists the fields that require sanitizing.

jhodgdon’s picture

Version: 7.x-dev » 8.x-dev
Status: Needs review » Reviewed & tested by the community

Looks good -- thanks for the patch!

One thing: we need to leave the version at 8.x, commit to 8.x first, and then to 7.x. The reason being that we don't want older versions of Drupal to have fixes that the newer versions don't already have. So I am setting the version to 8.x and clicking "re-test" so that the test bot can verify that the patch applies and doesn't break anything in the 8.x code base.

jhodgdon’s picture

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 7.x and 8.x. Thanks!

Status: Fixed » Closed (fixed)

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